* [PATCH] Lazy wdired preprocessing
@ 2021-03-25 16:06 Arthur Miller
2021-03-25 23:09 ` Michael Heerdegen
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Arthur Miller @ 2021-03-25 16:06 UTC (permalink / raw)
To: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 381 bytes --]
Haven't got any repsonses, but for me it seems to work fine. Maybe I
haven't tested some use-case though.
I have tied-up just a bit: removed unnecessary commented out line I
added while testing and unnecessary argument passing to pre-processing
routines.
If it is still interesting.
I attach also my working file if someone wishes to just eval and test it
without rebuilding.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Lazy-wdired-preprocessing.patch --]
[-- Type: text/x-patch, Size: 11673 bytes --]
From 6ee3ce6cb3c2ed442ecc32d59ff47f0ff4e5a4d1 Mon Sep 17 00:00:00 2001
From: Arthur Miller <arthur.miller@live.com>
Date: Thu, 25 Mar 2021 16:57:18 +0100
Subject: [PATCH] Lazy wdired preprocessing
---
lisp/wdired.el | 190 +++++++++++++++++++++++++++++--------------------
1 file changed, 112 insertions(+), 78 deletions(-)
diff --git a/lisp/wdired.el b/lisp/wdired.el
index 43026d4bb7..8c997dc340 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -189,6 +189,8 @@ wdired-mode-hook
;; Local variables (put here to avoid compilation gripes)
(defvar wdired-col-perm) ;; Column where the permission bits start
+(defvar wdired-perm-beg) ;; Column where the permission bits start
+(defvar wdired-perm-end) ;; Column where the permission bits stop
(defvar wdired-old-content)
(defvar wdired-old-point)
(defvar wdired-old-marks)
@@ -235,6 +237,8 @@ wdired-change-to-wdired-mode
(setq-local wdired-old-marks
(dired-remember-marks (point-min) (point-max)))
(setq-local wdired-old-point (point))
+ (setq-local wdired-perm-beg nil)
+ (setq-local wdired-perm-end nil)
(setq-local query-replace-skip-read-only t)
(add-function :after-while (local 'isearch-filter-predicate)
#'wdired-isearch-filter-read-only)
@@ -243,22 +247,24 @@ wdired-change-to-wdired-mode
(setq buffer-read-only nil)
(dired-unadvertise default-directory)
(add-hook 'kill-buffer-hook #'wdired-check-kill-buffer nil t)
+ (add-hook 'before-change-functions #'wdired--before-change-fn nil t)
(add-hook 'after-change-functions #'wdired--restore-properties nil t)
(setq major-mode 'wdired-mode)
(setq mode-name "Editable Dired")
- (add-function :override (local 'revert-buffer-function) #'wdired-revert)
- ;; I temp disable undo for performance: since I'm going to clear the
- ;; undo list, it can save more than a 9% of time with big
- ;; directories because setting properties modify the undo-list.
- (buffer-disable-undo)
- (wdired-preprocess-files)
- (if wdired-allow-to-change-permissions
- (wdired-preprocess-perms))
- (if (fboundp 'make-symbolic-link)
- (wdired-preprocess-symlinks))
- (buffer-enable-undo) ; Performance hack. See above.
+ (setq revert-buffer-function 'wdired-revert)
(set-buffer-modified-p nil)
(setq buffer-undo-list nil)
+ ;; find one column with permissions and set permision text boundaries
+ (save-excursion
+ (goto-char (point-min))
+ (unless (re-search-forward dired-re-perms nil t 1)
+ (wdired-abort-changes)
+ (error "No files to be renamed - Exiting to Dired mode."))
+ (goto-char (match-beginning 0))
+ (setq-local wdired-perm-beg (current-column))
+ (goto-char (match-end 0))
+ (setq-local wdired-perm-end (current-column)))
+ (define-key wdired-mode-map [remap self-insert-command] #'wdired--self-insert)
(run-mode-hooks 'wdired-mode-hook)
(message "%s" (substitute-command-keys
"Press \\[wdired-finish-edit] when finished \
@@ -269,16 +275,49 @@ wdired-isearch-filter-read-only
(not (text-property-not-all (min beg end) (max beg end)
'read-only nil)))
+(defun wdired--point-at-perms-p ()
+ (and (>= (current-column) wdired-perm-beg)
+ (<= (current-column) wdired-perm-end)))
+
+(defun wdired--self-insert ()
+ (interactive)
+ (if (wdired--point-at-perms-p)
+ (when (not (get-text-property (line-beginning-position) 'front-sticky))
+ (wdired--before-change-fn (line-beginning-position) (line-end-position))
+ (setq unread-command-events (nconc (listify-key-sequence
+ (this-command-keys))
+ unread-command-events)))
+ (call-interactively 'self-insert-command)))
+
+(defun wdired--before-change-fn (beg end)
+ (save-excursion
+ ;; make sure to process entire lines
+ (goto-char beg)
+ (setq beg (line-beginning-position))
+ (goto-char end)
+ (setq end (line-end-position))
+
+ (while (< beg end)
+ (unless (get-text-property beg 'front-sticky)
+ (put-text-property beg (1+ beg) 'front-sticky t)
+ (wdired--preprocess-files)
+ (when wdired-allow-to-change-permissions
+ (wdired--preprocess-perms))
+ (when (fboundp 'make-symbolic-link)
+ (wdired--preprocess-symlinks)))
+ (forward-line)
+ (setq beg (point)))
+ ;; is this good enough? assumes no extra white lines from dired
+ (put-text-property (1- (point-max)) (point-max) 'read-only t)))
+
;; Protect the buffer so only the filenames can be changed, and put
;; properties so filenames (old and new) can be easily found.
-(defun wdired-preprocess-files ()
- (put-text-property (point-min) (1+ (point-min))'front-sticky t)
+(defun wdired--preprocess-files ()
(save-excursion
- (goto-char (point-min))
- (let ((b-protection (point))
- (used-F (dired-check-switches dired-actual-switches "F" "classify"))
- filename)
- (while (not (eobp))
+ (with-silent-modifications
+ (beginning-of-line)
+ (let ((used-F (dired-check-switches dired-actual-switches "F" "classify"))
+ filename)
(setq filename (dired-get-filename nil t))
(when (and filename
(not (member (file-name-nondirectory filename) '("." ".."))))
@@ -287,19 +326,16 @@ wdired-preprocess-files
;; the filename can't be modified.
(add-text-properties
(1- (point)) (point) `(old-name ,filename rear-nonsticky (read-only)))
- (put-text-property b-protection (point) 'read-only t)
+ (put-text-property (- (point) 1) (point) 'read-only t)
(dired-move-to-end-of-filename t)
(put-text-property (point) (1+ (point)) 'end-name t))
- (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
- (when (save-excursion
- (and (re-search-backward
- dired-permission-flags-regexp nil t)
- (looking-at "l")
- (search-forward " -> " (line-end-position) t)))
- (goto-char (line-end-position)))
- (setq b-protection (point))
- (forward-line))
- (put-text-property b-protection (point-max) 'read-only t))))
+ (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
+ (when (save-excursion
+ (and (re-search-backward
+ dired-permission-flags-regexp nil t)
+ (looking-at "l")
+ (search-forward " -> " (line-end-position) t)))
+ (goto-char (line-end-position)))))))
;; This code is a copy of some dired-get-filename lines.
(defsubst wdired-normalize-filename (file unquotep)
@@ -362,7 +398,6 @@ wdired-get-filename
(and file (> (length file) 0)
(concat (dired-current-directory) file))))))
-
(defun wdired-change-to-dired-mode ()
"Change the mode back to dired."
(or (eq major-mode 'wdired-mode)
@@ -379,14 +414,16 @@ wdired-change-to-dired-mode
(setq major-mode 'dired-mode)
(setq mode-name "Dired")
(dired-advertise)
- (remove-hook 'kill-buffer-hook #'wdired-check-kill-buffer t)
- (remove-hook 'after-change-functions #'wdired--restore-properties t)
- (remove-function (local 'revert-buffer-function) #'wdired-revert))
+ (remove-hook 'kill-buffer-hook 'wdired-check-kill-buffer t)
+ (remove-hook 'before-change-functions 'wdired--before-change-fn t)
+ (remove-hook 'after-change-functions 'wdired--restore-properties t)
+ (setq-local revert-buffer-function 'dired-revert))
(defun wdired-abort-changes ()
- "Abort changes and return to dired mode."
+ "Abort changes and return to dired mode. "
(interactive)
- (let ((inhibit-read-only t))
+ (remove-hook 'before-change-functions 'wdired--before-change-fn t)
+ (with-silent-modifications
(erase-buffer)
(insert wdired-old-content)
(goto-char wdired-old-point))
@@ -702,21 +739,19 @@ wdired-previous-line
(dired-move-to-filename)))
;; Put the needed properties to allow the user to change links' targets
-(defun wdired-preprocess-symlinks ()
- (let ((inhibit-read-only t))
- (save-excursion
- (goto-char (point-min))
- (while (not (eobp))
- (when (looking-at dired-re-sym)
- (re-search-forward " -> \\(.*\\)$")
- (put-text-property (1- (match-beginning 1))
- (match-beginning 1) 'old-link
- (match-string-no-properties 1))
- (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
- (unless wdired-allow-to-redirect-links
- (put-text-property (match-beginning 0)
- (match-end 1) 'read-only t)))
- (forward-line)))))
+(defun wdired--preprocess-symlinks ()
+ (save-excursion
+ (with-silent-modifications
+ (beginning-of-line)
+ (when (looking-at dired-re-sym)
+ (re-search-forward " -> \\(.*\\)$")
+ (put-text-property (1- (match-beginning 1))
+ (match-beginning 1) 'old-link
+ (match-string-no-properties 1))
+ (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
+ (unless wdired-allow-to-redirect-links
+ (put-text-property (match-beginning 0)
+ (match-end 1) 'read-only t))))))
(defun wdired-get-previous-link (&optional old move)
"Return the next symlink target.
@@ -822,34 +857,33 @@ wdired-perm-mode-map
;; Put a keymap property to the permission bits of the files, and store the
;; original name and permissions as a property
-(defun wdired-preprocess-perms ()
- (let ((inhibit-read-only t))
- (setq-local wdired-col-perm nil)
- (save-excursion
- (goto-char (point-min))
- (while (not (eobp))
- (when (and (not (looking-at dired-re-sym))
- (wdired-get-filename)
- (re-search-forward dired-re-perms (line-end-position) 'eol))
- (let ((begin (match-beginning 0))
- (end (match-end 0)))
- (unless wdired-col-perm
- (setq wdired-col-perm (- (current-column) 9)))
- (if (eq wdired-allow-to-change-permissions 'advanced)
- (progn
- (put-text-property begin end 'read-only nil)
- ;; make first permission bit writable
- (put-text-property
- (1- begin) begin 'rear-nonsticky '(read-only)))
- ;; avoid that keymap applies to text following permissions
- (add-text-properties
- (1+ begin) end
- `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
- (put-text-property end (1+ end) 'end-perm t)
- (put-text-property
- begin (1+ begin) 'old-perm (match-string-no-properties 0))))
- (forward-line)
- (beginning-of-line)))))
+(defun wdired--preprocess-perms ()
+ (save-excursion
+ (with-silent-modifications
+ (setq-local wdired-col-perm nil)
+ (beginning-of-line)
+ (when (and (not (looking-at dired-re-sym))
+ (wdired-get-filename)
+ (re-search-forward dired-re-perms
+ (line-end-position) 'eol))
+ (let ((begin (match-beginning 0))
+ (end (match-end 0)))
+ (unless wdired-col-perm
+ (setq wdired-col-perm (- (current-column) 9)))
+ (if (eq wdired-allow-to-change-permissions 'advanced)
+ (progn
+ (put-text-property begin end 'read-only nil)
+ ;; make first permission bit writable
+ (put-text-property
+ (1- begin) begin 'rear-nonsticky '(read-only)))
+ ;; avoid that keymap applies to text following permissions
+ (add-text-properties
+ (1+ begin) end
+ `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
+ (put-text-property end (1+ end) 'end-perm t)
+ (put-text-property
+ begin (1+ begin)
+ 'old-perm (match-string-no-properties 0)))))))
(defun wdired-perm-allowed-in-pos (char pos)
(cond
--
2.31.0
[-- Attachment #3: lazy-wdired.el --]
[-- Type: text/plain, Size: 8695 bytes --]
;;; lazy-wdired.el --- -*- lexical-binding: t; -*-
;; Copyright (C) 2020 Arthur Miller
;; Author: Arthur Miller <arthur.miller@live.com>
;; Keywords:
;; This program is free software; you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.
;; This program is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
;; GNU General Public License for more details.
;; You should have received a copy of the GNU General Public License
;; along with this program. If not, see <https://www.gnu.org/licenses/>.
;;; Commentary:
;;; Enable editing of file name and properties only at the point.
;;; Code:
(require 'wdired)
(defvar wdired-perm-beg) ;; Column where the permission bits start
(defvar wdired-perm-end) ;; Column where the permission bits stop
;;;###autoload
(defun wdired-change-to-wdired-mode ()
"Put a Dired buffer in Writable Dired (WDired) mode.
\\<wdired-mode-map>
In WDired mode, you can edit the names of the files in the
buffer, the target of the links, and the permission bits of the
files. After typing \\[wdired-finish-edit], Emacs modifies the files and
directories to reflect your edits.
See `wdired-mode'."
(interactive)
(unless (derived-mode-p 'dired-mode)
(error "Not a Dired buffer"))
(setq-local wdired-old-content
(buffer-substring (point-min) (point-max)))
(setq-local wdired-old-marks
(dired-remember-marks (point-min) (point-max)))
(setq-local wdired-old-point (point))
(setq-local wdired-perm-beg nil)
(setq-local wdired-perm-end nil)
(setq-local query-replace-skip-read-only t)
(add-function :after-while (local 'isearch-filter-predicate)
#'wdired-isearch-filter-read-only)
(use-local-map wdired-mode-map)
(force-mode-line-update)
(setq buffer-read-only nil)
(dired-unadvertise default-directory)
(add-hook 'kill-buffer-hook #'wdired-check-kill-buffer nil t)
(add-hook 'before-change-functions #'wdired--before-change-fn nil t)
(add-hook 'after-change-functions #'wdired--restore-properties nil t)
(setq major-mode 'wdired-mode)
(setq mode-name "Editable Dired")
(setq revert-buffer-function 'wdired-revert)
(set-buffer-modified-p nil)
(setq buffer-undo-list nil)
;; find one column with permissions and set permision text boundaries
(save-excursion
(goto-char (point-min))
(unless (re-search-forward dired-re-perms nil t 1)
(wdired-abort-changes)
(error "No files to be renamed - Exiting to Dired mode."))
(goto-char (match-beginning 0))
(setq-local wdired-perm-beg (current-column))
(goto-char (match-end 0))
(setq-local wdired-perm-end (current-column)))
(define-key wdired-mode-map [remap self-insert-command] #'wdired--self-insert)
(run-mode-hooks 'wdired-mode-hook)
(message "%s" (substitute-command-keys
"Press \\[wdired-finish-edit] when finished \
or \\[wdired-abort-changes] to abort changes")))
(defun wdired--point-at-perms-p ()
(and (>= (current-column) wdired-perm-beg)
(<= (current-column) wdired-perm-end)))
(defun wdired--self-insert ()
(interactive)
(if (wdired--point-at-perms-p)
(when (not (get-text-property (line-beginning-position) 'front-sticky))
(wdired--before-change-fn (line-beginning-position) (line-end-position))
(setq unread-command-events (nconc (listify-key-sequence
(this-command-keys))
unread-command-events)))
(call-interactively 'self-insert-command)))
(defun wdired--before-change-fn (beg end)
(save-excursion
;; make sure to process entire lines
(goto-char beg)
(setq beg (line-beginning-position))
(goto-char end)
(setq end (line-end-position))
(while (< beg end)
(unless (get-text-property beg 'front-sticky)
(put-text-property beg (1+ beg) 'front-sticky t)
(wdired--preprocess-files)
(when wdired-allow-to-change-permissions
(wdired--preprocess-perms))
(when (fboundp 'make-symbolic-link)
(wdired--preprocess-symlinks)))
(forward-line)
(setq beg (point)))
;; is this good enough? assumes no extra white lines from dired
(put-text-property (1- (point-max)) (point-max) 'read-only t)))
;; Protect the buffer so only the filenames can be changed, and put
;; properties so filenames (old and new) can be easily found.
(defun wdired--preprocess-files ()
(save-excursion
(with-silent-modifications
(beginning-of-line)
(let ((used-F (dired-check-switches dired-actual-switches "F" "classify"))
filename)
(setq filename (dired-get-filename nil t))
(when (and filename
(not (member (file-name-nondirectory filename) '("." ".."))))
(dired-move-to-filename)
;; The rear-nonsticky property below shall ensure that text preceding
;; the filename can't be modified.
(add-text-properties
(1- (point)) (point) `(old-name ,filename rear-nonsticky (read-only)))
(put-text-property (- (point) 1) (point) 'read-only t)
(dired-move-to-end-of-filename t)
(put-text-property (point) (1+ (point)) 'end-name t))
(when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
(when (save-excursion
(and (re-search-backward
dired-permission-flags-regexp nil t)
(looking-at "l")
(search-forward " -> " (line-end-position) t)))
(goto-char (line-end-position)))))))
(defun wdired-change-to-dired-mode ()
"Change the mode back to dired."
(or (eq major-mode 'wdired-mode)
(error "Not a Wdired buffer"))
(let ((inhibit-read-only t))
(remove-text-properties
(point-min) (point-max)
'(front-sticky nil rear-nonsticky nil read-only nil keymap nil)))
(remove-function (local 'isearch-filter-predicate)
#'wdired-isearch-filter-read-only)
(use-local-map dired-mode-map)
(force-mode-line-update)
(setq buffer-read-only t)
(setq major-mode 'dired-mode)
(setq mode-name "Dired")
(dired-advertise)
(remove-hook 'kill-buffer-hook 'wdired-check-kill-buffer t)
(remove-hook 'before-change-functions 'wdired--before-change-fn t)
(remove-hook 'after-change-functions 'wdired--restore-properties t)
(setq-local revert-buffer-function 'dired-revert))
(defun wdired-abort-changes ()
"Abort changes and return to dired mode. "
(interactive)
(remove-hook 'before-change-functions 'wdired--before-change-fn t)
(with-silent-modifications
(erase-buffer)
(insert wdired-old-content)
(goto-char wdired-old-point))
(wdired-change-to-dired-mode)
(set-buffer-modified-p nil)
(setq buffer-undo-list nil)
(message "Changes aborted"))
;; Put the needed properties to allow the user to change links' targets
(defun wdired--preprocess-symlinks ()
(save-excursion
(with-silent-modifications
(beginning-of-line)
(when (looking-at dired-re-sym)
(re-search-forward " -> \\(.*\\)$")
(put-text-property (1- (match-beginning 1))
(match-beginning 1) 'old-link
(match-string-no-properties 1))
(put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
(unless wdired-allow-to-redirect-links
(put-text-property (match-beginning 0)
(match-end 1) 'read-only t))))))
(defun wdired--preprocess-perms ()
(save-excursion
(with-silent-modifications
(setq-local wdired-col-perm nil)
(beginning-of-line)
(when (and (not (looking-at dired-re-sym))
(wdired-get-filename)
(re-search-forward dired-re-perms
(line-end-position) 'eol))
(let ((begin (match-beginning 0))
(end (match-end 0)))
(unless wdired-col-perm
(setq wdired-col-perm (- (current-column) 9)))
(if (eq wdired-allow-to-change-permissions 'advanced)
(progn
(put-text-property begin end 'read-only nil)
;; make first permission bit writable
(put-text-property
(1- begin) begin 'rear-nonsticky '(read-only)))
;; avoid that keymap applies to text following permissions
(add-text-properties
(1+ begin) end
`(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
(put-text-property end (1+ end) 'end-perm t)
(put-text-property
begin (1+ begin)
'old-perm (match-string-no-properties 0)))))))
(provide 'lazy-wdired)
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-25 16:06 [PATCH] Lazy wdired preprocessing Arthur Miller
@ 2021-03-25 23:09 ` Michael Heerdegen
2021-03-26 1:00 ` Arthur Miller
2021-03-26 10:18 ` Stefan Kangas
2021-03-26 19:37 ` Stefan Monnier
2 siblings, 1 reply; 24+ messages in thread
From: Michael Heerdegen @ 2021-03-25 23:09 UTC (permalink / raw)
To: emacs-devel
Arthur Miller <arthur.miller@live.com> writes:
> Haven't got any repsonses, but for me it seems to work fine. Maybe I
> haven't tested some use-case though.
I have understood what you want to achieve. Could you please say one or
two sentences about what the purpose of the changes in wdired.el is -
are they necessary to make lazy-wdired.el work, or are there user
visible changes (improvements) already for wdired.el alone?
TIA,
Michael.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-25 23:09 ` Michael Heerdegen
@ 2021-03-26 1:00 ` Arthur Miller
2021-03-26 3:27 ` Michael Heerdegen
0 siblings, 1 reply; 24+ messages in thread
From: Arthur Miller @ 2021-03-26 1:00 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: emacs-devel
Michael Heerdegen <michael_heerdegen@web.de> writes:
> Arthur Miller <arthur.miller@live.com> writes:
>
>> Haven't got any repsonses, but for me it seems to work fine. Maybe I
>> haven't tested some use-case though.
>
> I have understood what you want to achieve. Could you please say one or
> two sentences about what the purpose of the changes in wdired.el is -
> are they necessary to make lazy-wdired.el work, or are there user
> visible changes (improvements) already for wdired.el alone?
Lazy-wdired.el was just my working file, I sent it for the convenience:
if anyone wishes to easily test without need to apply patch to
wdired.el, but patch is what it is about; i.e. you can throw away
lazy-wdired.el :).
This was re-implementation of parts of wdired to make it more efficient
and faster to enter the wdired mode.
This patch makes wdired process only those lines in a region where
changes are requested, in interactive cases usually just one line at a
time. That makes it always same cost to enter wdired and possibly also
saves some battery life when user does not mean to edit all file names.
Price is somewhat slower editing per line, but I can't notice any
slowdowns or delays on my computer.
If you check original code, it will pre-process entire dired buffer to
make some part of dired non-editable. On directories with large number
of files (~500+) that results in a noticable delay when entering wdired
mode.
There are no user-visible changes, other then a refusing to enter wdired
when there are no files in a directory. I could remove that error, but I
don't think there is purpose to enter writable dired for empty
directory. I hope it is correct assumption.
So it is just a little optimization.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-26 1:00 ` Arthur Miller
@ 2021-03-26 3:27 ` Michael Heerdegen
2021-03-26 12:15 ` Arthur Miller
2021-03-26 12:21 ` Arthur Miller
0 siblings, 2 replies; 24+ messages in thread
From: Michael Heerdegen @ 2021-03-26 3:27 UTC (permalink / raw)
To: emacs-devel
Arthur Miller <arthur.miller@live.com> writes:
> This was re-implementation of parts of wdired to make it more efficient
> and faster to enter the wdired mode.
Ok, thanks for the elaboration. Didn't try it yet, but I welcome this
change. So far I have one question:
+ ;; find one column with permissions and set permision text boundaries
+ (save-excursion
+ (goto-char (point-min))
+ (unless (re-search-forward dired-re-perms nil t 1)
+ (wdired-abort-changes)
+ (error "No files to be renamed - Exiting to Dired mode."))
+ (goto-char (match-beginning 0))
+ (setq-local wdired-perm-beg (current-column))
+ (goto-char (match-end 0))
+ (setq-local wdired-perm-end (current-column)))
Did you check that this works when `dired-hide-details-mode' is enabled?
I ask because AFAIK `current-column' doesn't count invisible characters.
[It might be better to just count characters from the line's beginning,
but I see that the existing code also uses `current-column'.]
Thanks,
Michael.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-25 16:06 [PATCH] Lazy wdired preprocessing Arthur Miller
2021-03-25 23:09 ` Michael Heerdegen
@ 2021-03-26 10:18 ` Stefan Kangas
2021-03-26 19:37 ` Stefan Monnier
2 siblings, 0 replies; 24+ messages in thread
From: Stefan Kangas @ 2021-03-26 10:18 UTC (permalink / raw)
To: Arthur Miller, emacs-devel
Arthur Miller <arthur.miller@live.com> writes:
> Haven't got any repsonses, but for me it seems to work fine. Maybe I
> haven't tested some use-case though.
>
> I have tied-up just a bit: removed unnecessary commented out line I
> added while testing and unnecessary argument passing to pre-processing
> routines.
>
> If it is still interesting.
This feature is very welcome, thank you for working on it.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-26 3:27 ` Michael Heerdegen
@ 2021-03-26 12:15 ` Arthur Miller
2021-03-26 12:21 ` Arthur Miller
1 sibling, 0 replies; 24+ messages in thread
From: Arthur Miller @ 2021-03-26 12:15 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: emacs-devel
Michael Heerdegen <michael_heerdegen@web.de> writes:
> Arthur Miller <arthur.miller@live.com> writes:
>
>> This was re-implementation of parts of wdired to make it more efficient
>> and faster to enter the wdired mode.
>
> Ok, thanks for the elaboration. Didn't try it yet, but I welcome this
> change. So far I have one question:
>
> + ;; find one column with permissions and set permision text boundaries
> + (save-excursion
> + (goto-char (point-min))
> + (unless (re-search-forward dired-re-perms nil t 1)
> + (wdired-abort-changes)
> + (error "No files to be renamed - Exiting to Dired mode."))
> + (goto-char (match-beginning 0))
> + (setq-local wdired-perm-beg (current-column))
> + (goto-char (match-end 0))
> + (setq-local wdired-perm-end (current-column)))
>
> Did you check that this works when `dired-hide-details-mode' is enabled?
Yes, it works. I that option always on, and display details only when I
need them.
> I ask because AFAIK `current-column' doesn't count invisible characters.
> [It might be better to just count characters from the line's beginning,
> but I see that the existing code also uses `current-column'.]
As optimization, I pre-calculate start and end of permission part of
text in `wdired-change-to-widred-mode, so I don't need to do this when
processing every line, but I could do this later on, I don't think it
would be a problem. When details are hidden, one can not enter the
permission part of text, so the `current-column wont be called either. I
use it only to check that point is in permission part of text, so when
permissions are hidden user won't enter it and `current-column will not
be called.
I had version where I calculated column on my own, but I thought it is
better to use something that already is there than to add more code :).
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-26 3:27 ` Michael Heerdegen
2021-03-26 12:15 ` Arthur Miller
@ 2021-03-26 12:21 ` Arthur Miller
2021-03-27 23:49 ` Michael Heerdegen
1 sibling, 1 reply; 24+ messages in thread
From: Arthur Miller @ 2021-03-26 12:21 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: emacs-devel
Michael Heerdegen <michael_heerdegen@web.de> writes:
> Arthur Miller <arthur.miller@live.com> writes:
>
>> This was re-implementation of parts of wdired to make it more efficient
>> and faster to enter the wdired mode.
>
> Ok, thanks for the elaboration. Didn't try it yet, but I welcome this
> change. So far I have one question:
>
> + ;; find one column with permissions and set permision text boundaries
> + (save-excursion
> + (goto-char (point-min))
> + (unless (re-search-forward dired-re-perms nil t 1)
> + (wdired-abort-changes)
> + (error "No files to be renamed - Exiting to Dired mode."))
> + (goto-char (match-beginning 0))
> + (setq-local wdired-perm-beg (current-column))
> + (goto-char (match-end 0))
> + (setq-local wdired-perm-end (current-column)))
>
> Did you check that this works when `dired-hide-details-mode' is enabled?
> I ask because AFAIK `current-column' doesn't count invisible characters.
> [It might be better to just count characters from the line's beginning,
> but I see that the existing code also uses `current-column'.]
I just tested bit more. Once wdired is entered with hidden details, one
can not switch deatils on normally since dired-hide-details-mode checks
if it is in dired-mode.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-25 16:06 [PATCH] Lazy wdired preprocessing Arthur Miller
2021-03-25 23:09 ` Michael Heerdegen
2021-03-26 10:18 ` Stefan Kangas
@ 2021-03-26 19:37 ` Stefan Monnier
2021-03-27 7:39 ` Arthur Miller
2 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2021-03-26 19:37 UTC (permalink / raw)
To: Arthur Miller; +Cc: emacs-devel
> +(defvar wdired-perm-beg) ;; Column where the permission bits start
> +(defvar wdired-perm-end) ;; Column where the permission bits stop
I think this should use "--" in the names since they are internal variables.
> (setq mode-name "Editable Dired")
> - (add-function :override (local 'revert-buffer-function) #'wdired-revert)
> - ;; I temp disable undo for performance: since I'm going to clear the
> - ;; undo list, it can save more than a 9% of time with big
> - ;; directories because setting properties modify the undo-list.
> - (buffer-disable-undo)
> - (wdired-preprocess-files)
> - (if wdired-allow-to-change-permissions
> - (wdired-preprocess-perms))
> - (if (fboundp 'make-symbolic-link)
> - (wdired-preprocess-symlinks))
> - (buffer-enable-undo) ; Performance hack. See above.
> + (setq revert-buffer-function 'wdired-revert)
> (set-buffer-modified-p nil)
This reverts part of my recent change to the way we set
`revert-buffer-function` (most likely an oversight while merging my changes).
> (setq buffer-undo-list nil)
> + ;; find one column with permissions and set permision text boundaries
> + (save-excursion
> + (goto-char (point-min))
> + (unless (re-search-forward dired-re-perms nil t 1)
> + (wdired-abort-changes)
> + (error "No files to be renamed - Exiting to Dired mode."))
> + (goto-char (match-beginning 0))
> + (setq-local wdired-perm-beg (current-column))
> + (goto-char (match-end 0))
> + (setq-local wdired-perm-end (current-column)))
I'd recommend you put this in a separate function.
> + (define-key wdired-mode-map [remap self-insert-command] #'wdired--self-insert)
Why is this done here instead of in the definition of `wdired-mode-map`?
> +(defun wdired--point-at-perms-p ()
> + (and (>= (current-column) wdired-perm-beg)
> + (<= (current-column) wdired-perm-end)))
`current-column` can be somewhat costly, so we should refrain from
calling it twice gratuitously. And here we can even take advantage of
the (rarely used and rarely applicable) multi-arg form of `<=` to fix
that "for free":
(<= wdired-perm-beg (current-column) wdired-perm-end)
> +(defun wdired--self-insert ()
> + (interactive)
> + (if (wdired--point-at-perms-p)
> + (when (not (get-text-property (line-beginning-position) 'front-sticky))
> + (wdired--before-change-fn (line-beginning-position) (line-end-position))
> + (setq unread-command-events (nconc (listify-key-sequence
> + (this-command-keys))
> + unread-command-events)))
> + (call-interactively 'self-insert-command)))
I think this deserves a comment about why we look at `front-sticky`.
Better yet: move this test to a helper function to which you can give
a meaningful name (like `wdired--processed-p`).
Also, instead of using `unread-command-events`, you can just call the
appropriate command directly, no?
> - (remove-hook 'kill-buffer-hook #'wdired-check-kill-buffer t)
> - (remove-hook 'after-change-functions #'wdired--restore-properties t)
> - (remove-function (local 'revert-buffer-function) #'wdired-revert))
> + (remove-hook 'kill-buffer-hook 'wdired-check-kill-buffer t)
> + (remove-hook 'before-change-functions 'wdired--before-change-fn t)
> + (remove-hook 'after-change-functions 'wdired--restore-properties t)
> + (setq-local revert-buffer-function 'dired-revert))
This also look like the merge wasn't done right.
> (defun wdired-abort-changes ()
> - "Abort changes and return to dired mode."
> + "Abort changes and return to dired mode. "
What happened here?
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-26 19:37 ` Stefan Monnier
@ 2021-03-27 7:39 ` Arthur Miller
2021-03-27 14:56 ` Stefan Monnier
0 siblings, 1 reply; 24+ messages in thread
From: Arthur Miller @ 2021-03-27 7:39 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 5426 bytes --]
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> +(defvar wdired-perm-beg) ;; Column where the permission bits start
>> +(defvar wdired-perm-end) ;; Column where the permission bits stop
>
> I think this should use "--" in the names since they are internal variables.
I just followed naming as already was in wdired. In general I tried to
change as little as possible, so I didn't re-factor any existing code
unless needed. Partly to save my time, partly to not mess up
unnecessary. I have now changed all global vars to use '--' in name. I
was thinking about "private" functions too, but I'll leave that for some
other time or as exercise to someone else.
>> (setq mode-name "Editable Dired")
>> - (add-function :override (local 'revert-buffer-function) #'wdired-revert)
>> - ;; I temp disable undo for performance: since I'm going to clear the
>> - ;; undo list, it can save more than a 9% of time with big
>> - ;; directories because setting properties modify the undo-list.
>> - (buffer-disable-undo)
>> - (wdired-preprocess-files)
>> - (if wdired-allow-to-change-permissions
>> - (wdired-preprocess-perms))
>> - (if (fboundp 'make-symbolic-link)
>> - (wdired-preprocess-symlinks))
>> - (buffer-enable-undo) ; Performance hack. See above.
>> + (setq revert-buffer-function 'wdired-revert)
>> (set-buffer-modified-p nil)
>
> This reverts part of my recent change to the way we set
> `revert-buffer-function` (most likely an oversight while merging my changes).
You commited changes to wdired? :-)
Yeah, I copy-pasted entire function from my working file, so yes indeed,
it was an oversight on my part, fixed.
>> (setq buffer-undo-list nil)
>> + ;; find one column with permissions and set permision text boundaries
>> + (save-excursion
>> + (goto-char (point-min))
>> + (unless (re-search-forward dired-re-perms nil t 1)
>> + (wdired-abort-changes)
>> + (error "No files to be renamed - Exiting to Dired mode."))
>> + (goto-char (match-beginning 0))
>> + (setq-local wdired-perm-beg (current-column))
>> + (goto-char (match-end 0))
>> + (setq-local wdired-perm-end (current-column)))
>
> I'd recommend you put this in a separate function.
I have refactored that a bit more and am using directory-empty-p to exit
earlier. Hope we can use default-directory there. Check the code.
>> + (define-key wdired-mode-map [remap self-insert-command] #'wdired--self-insert)
>
> Why is this done here instead of in the definition of `wdired-mode-map`?
Was typing as I was thinking ... :-). Fixed.
>> +(defun wdired--point-at-perms-p ()
>> + (and (>= (current-column) wdired-perm-beg)
>> + (<= (current-column) wdired-perm-end)))
>
> `current-column` can be somewhat costly, so we should refrain from
> calling it twice gratuitously. And here we can even take advantage of
> the (rarely used and rarely applicable) multi-arg form of `<=` to fix
> that "for free":
Ok. Didn't know that (current-column) was expensive. I use now a good 'nuff
implementation for this purpose (wdired--current-column).
> (<= wdired-perm-beg (current-column) wdired-perm-end)
Cool; thnks, didn't know we can write so.
>> +(defun wdired--self-insert ()
>> + (interactive)
>> + (if (wdired--point-at-perms-p)
>> + (when (not (get-text-property (line-beginning-position) 'front-sticky))
>> + (wdired--before-change-fn (line-beginning-position) (line-end-position))
>> + (setq unread-command-events (nconc (listify-key-sequence
>> + (this-command-keys))
>> + unread-command-events)))
>> + (call-interactively 'self-insert-command)))
>
> I think this deserves a comment about why we look at `front-sticky`.
> Better yet: move this test to a helper function to which you can give
> a meaningful name (like `wdired--processed-p`).
Sure, it makes code more self-documented.
> Also, instead of using `unread-command-events`, you can just call the
> appropriate command directly, no?
As I stated about refactoring above, I tried to change as
little as possible. Partly because I was lazy to look into the old code
and partly because I tried to "plug" into already existing (and
hopefully debugged) code. But sure toggle-bit is the one I think.
>> - (remove-hook 'kill-buffer-hook #'wdired-check-kill-buffer t)
>> - (remove-hook 'after-change-functions #'wdired--restore-properties t)
>> - (remove-function (local 'revert-buffer-function) #'wdired-revert))
>> + (remove-hook 'kill-buffer-hook 'wdired-check-kill-buffer t)
>> + (remove-hook 'before-change-functions 'wdired--before-change-fn t)
>> + (remove-hook 'after-change-functions 'wdired--restore-properties t)
>> + (setq-local revert-buffer-function 'dired-revert))
>
> This also look like the merge wasn't done right.
No idea; mr. git format-patch seems to shuffle lines around, and I trust
it to do what it has to do.
>> (defun wdired-abort-changes ()
>> - "Abort changes and return to dired mode."
>> + "Abort changes and return to dired mode. "
>
> What happened here?
No idea. :-).
It's all yours now. I don't think I will have more time nor possibility
to work on this, so if this one is not good enough, you will to finnish
it on your own, or someone else could help. We are waiting a kid any
day now, so no hobby programming for quite some time over for me :-).
Thnks for the guidance.
Cheers
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lazy-wdired.patch --]
[-- Type: text/x-patch, Size: 15410 bytes --]
From 7e6a6f2d7c958228882a7cdd34533d2c322093b4 Mon Sep 17 00:00:00 2001
From: Arthur Miller <arthur.miller@live.com>
Date: Sat, 27 Mar 2021 08:29:44 +0100
Subject: [PATCH] lazy wdired
---
lisp/wdired.el | 223 ++++++++++++++++++++++++++++---------------------
1 file changed, 129 insertions(+), 94 deletions(-)
diff --git a/lisp/wdired.el b/lisp/wdired.el
index 43026d4bb7..df78082596 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -172,6 +172,7 @@ wdired-mode-map
(define-key map [remap upcase-word] #'wdired-upcase-word)
(define-key map [remap capitalize-word] #'wdired-capitalize-word)
(define-key map [remap downcase-word] #'wdired-downcase-word)
+ (define-key map [remap self-insert-command] #'wdired--self-insert)
map)
"Keymap used in `wdired-mode'.")
@@ -188,10 +189,12 @@ wdired-mode-hook
"Hooks run when changing to WDired mode.")
;; Local variables (put here to avoid compilation gripes)
-(defvar wdired-col-perm) ;; Column where the permission bits start
-(defvar wdired-old-content)
-(defvar wdired-old-point)
-(defvar wdired-old-marks)
+(defvar wdired--col-perm) ;; Column where the permission bits start
+(defvar wdired--perm-beg) ;; Column where the permission bits start
+(defvar wdired--perm-end) ;; Column where the permission bits stop
+(defvar wdired--old-content)
+(defvar wdired--old-point)
+(defvar wdired--old-marks)
(defun wdired-mode ()
"Writable Dired (WDired) mode.
@@ -230,11 +233,14 @@ wdired-change-to-wdired-mode
(interactive)
(unless (derived-mode-p 'dired-mode)
(error "Not a Dired buffer"))
+ (when (directory-empty-p (expand-file-name default-directory))
+ (error "No files to be renamed"))
(setq-local wdired-old-content
(buffer-substring (point-min) (point-max)))
- (setq-local wdired-old-marks
+ (setq-local wdired--old-marks
(dired-remember-marks (point-min) (point-max)))
- (setq-local wdired-old-point (point))
+ (setq-local wdired--old-point (point))
+ (wdired--set-permission-bounds)
(setq-local query-replace-skip-read-only t)
(add-function :after-while (local 'isearch-filter-predicate)
#'wdired-isearch-filter-read-only)
@@ -243,20 +249,11 @@ wdired-change-to-wdired-mode
(setq buffer-read-only nil)
(dired-unadvertise default-directory)
(add-hook 'kill-buffer-hook #'wdired-check-kill-buffer nil t)
+ (add-hook 'before-change-functions #'wdired--before-change-fn nil t)
(add-hook 'after-change-functions #'wdired--restore-properties nil t)
(setq major-mode 'wdired-mode)
(setq mode-name "Editable Dired")
(add-function :override (local 'revert-buffer-function) #'wdired-revert)
- ;; I temp disable undo for performance: since I'm going to clear the
- ;; undo list, it can save more than a 9% of time with big
- ;; directories because setting properties modify the undo-list.
- (buffer-disable-undo)
- (wdired-preprocess-files)
- (if wdired-allow-to-change-permissions
- (wdired-preprocess-perms))
- (if (fboundp 'make-symbolic-link)
- (wdired-preprocess-symlinks))
- (buffer-enable-undo) ; Performance hack. See above.
(set-buffer-modified-p nil)
(setq buffer-undo-list nil)
(run-mode-hooks 'wdired-mode-hook)
@@ -264,6 +261,53 @@ wdired-change-to-wdired-mode
"Press \\[wdired-finish-edit] when finished \
or \\[wdired-abort-changes] to abort changes")))
+(defun wdired--set-permission-bounds ()
+ (save-excursion
+ (goto-char (point-min))
+ (re-search-forward dired-re-perms nil t 1)
+ (goto-char (match-beginning 0))
+ (setq-local wdired--perm-beg (current-column))
+ (goto-char (match-end 0))
+ (setq-local wdired--perm-end (current-column))))
+
+(defun wdired--current-column ()
+ (- (point) (line-beginning-position)))
+
+(defun wdired--point-at-perms-p ()
+ (<= wdired-perm-beg (wdired--current-column) wdired-perm-end))
+
+(defun wdired--line-preprocessed-p ()
+ (get-text-property (line-beginning-position) 'front-sticky))
+
+(defun wdired--self-insert ()
+ (interactive)
+ (if (wdired--point-at-perms-p)
+ (unless (wdired--line-preprocessed-p)
+ (wdired--before-change-fn (line-beginning-position) (line-end-position))
+ (wdired-toggle-bit))
+ (call-interactively 'self-insert-command)))
+
+(defun wdired--before-change-fn (beg end)
+ (save-excursion
+ ;; make sure to process entire lines
+ (goto-char beg)
+ (setq beg (line-beginning-position))
+ (goto-char end)
+ (setq end (line-end-position))
+
+ (while (< beg end)
+ (unless (wdired--line-preprocessed-p)
+ (put-text-property beg (1+ beg) 'front-sticky t)
+ (wdired--preprocess-files)
+ (when wdired-allow-to-change-permissions
+ (wdired--preprocess-perms))
+ (when (fboundp 'make-symbolic-link)
+ (wdired--preprocess-symlinks)))
+ (forward-line)
+ (setq beg (point)))
+ ;; is this good enough? assumes no extra white lines from dired
+ (put-text-property (1- (point-max)) (point-max) 'read-only t)))
+
(defun wdired-isearch-filter-read-only (beg end)
"Skip matches that have a read-only property."
(not (text-property-not-all (min beg end) (max beg end)
@@ -271,14 +315,12 @@ wdired-isearch-filter-read-only
;; Protect the buffer so only the filenames can be changed, and put
;; properties so filenames (old and new) can be easily found.
-(defun wdired-preprocess-files ()
- (put-text-property (point-min) (1+ (point-min))'front-sticky t)
+(defun wdired--preprocess-files ()
(save-excursion
- (goto-char (point-min))
- (let ((b-protection (point))
- (used-F (dired-check-switches dired-actual-switches "F" "classify"))
- filename)
- (while (not (eobp))
+ (with-silent-modifications
+ (beginning-of-line)
+ (let ((used-F (dired-check-switches dired-actual-switches "F" "classify"))
+ filename)
(setq filename (dired-get-filename nil t))
(when (and filename
(not (member (file-name-nondirectory filename) '("." ".."))))
@@ -287,19 +329,16 @@ wdired-preprocess-files
;; the filename can't be modified.
(add-text-properties
(1- (point)) (point) `(old-name ,filename rear-nonsticky (read-only)))
- (put-text-property b-protection (point) 'read-only t)
+ (put-text-property (- (point) 1) (point) 'read-only t)
(dired-move-to-end-of-filename t)
(put-text-property (point) (1+ (point)) 'end-name t))
- (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
- (when (save-excursion
- (and (re-search-backward
- dired-permission-flags-regexp nil t)
- (looking-at "l")
- (search-forward " -> " (line-end-position) t)))
- (goto-char (line-end-position)))
- (setq b-protection (point))
- (forward-line))
- (put-text-property b-protection (point-max) 'read-only t))))
+ (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
+ (when (save-excursion
+ (and (re-search-backward
+ dired-permission-flags-regexp nil t)
+ (looking-at "l")
+ (search-forward " -> " (line-end-position) t)))
+ (goto-char (line-end-position)))))))
;; This code is a copy of some dired-get-filename lines.
(defsubst wdired-normalize-filename (file unquotep)
@@ -362,7 +401,6 @@ wdired-get-filename
(and file (> (length file) 0)
(concat (dired-current-directory) file))))))
-
(defun wdired-change-to-dired-mode ()
"Change the mode back to dired."
(or (eq major-mode 'wdired-mode)
@@ -379,17 +417,19 @@ wdired-change-to-dired-mode
(setq major-mode 'dired-mode)
(setq mode-name "Dired")
(dired-advertise)
- (remove-hook 'kill-buffer-hook #'wdired-check-kill-buffer t)
- (remove-hook 'after-change-functions #'wdired--restore-properties t)
- (remove-function (local 'revert-buffer-function) #'wdired-revert))
+ (remove-hook 'kill-buffer-hook 'wdired-check-kill-buffer t)
+ (remove-hook 'before-change-functions 'wdired--before-change-fn t)
+ (remove-hook 'after-change-functions 'wdired--restore-properties t)
+ (setq-local revert-buffer-function 'dired-revert))
(defun wdired-abort-changes ()
"Abort changes and return to dired mode."
(interactive)
- (let ((inhibit-read-only t))
+ (remove-hook 'before-change-functions 'wdired--before-change-fn t)
+ (with-silent-modifications
(erase-buffer)
(insert wdired-old-content)
- (goto-char wdired-old-point))
+ (goto-char wdired--old-point))
(wdired-change-to-dired-mode)
(set-buffer-modified-p nil)
(setq buffer-undo-list nil)
@@ -411,7 +451,7 @@ wdired-finish-edit
(setq errors (cdr tmp-value))
(setq changes (car tmp-value)))
(when (and wdired-allow-to-change-permissions
- (boundp 'wdired-col-perm)) ; could have been changed
+ (boundp 'wdired--col-perm)) ; could have been changed
(setq tmp-value (wdired-do-perm-changes))
(setq errors (+ errors (cdr tmp-value)))
(setq changes (or changes (car tmp-value))))
@@ -429,11 +469,11 @@ wdired-finish-edit
(let ((mark (cond ((integerp wdired-keep-marker-rename)
wdired-keep-marker-rename)
(wdired-keep-marker-rename
- (cdr (assoc file-old wdired-old-marks)))
+ (cdr (assoc file-old wdired--old-marks)))
(t nil))))
(when mark
(push (cons (substitute-in-file-name file-new) mark)
- wdired-old-marks))))
+ wdired--old-marks))))
(push (cons file-old (substitute-in-file-name file-new))
files-renamed))))
(forward-line -1)))
@@ -458,7 +498,7 @@ wdired-finish-edit
;; Re-sort the buffer.
(revert-buffer)
(let ((inhibit-read-only t))
- (dired-mark-remembered wdired-old-marks)))
+ (dired-mark-remembered wdired--old-marks)))
(let ((inhibit-read-only t))
(remove-text-properties (point-min) (point-max)
'(old-name nil end-name nil old-link nil
@@ -702,21 +742,19 @@ wdired-previous-line
(dired-move-to-filename)))
;; Put the needed properties to allow the user to change links' targets
-(defun wdired-preprocess-symlinks ()
- (let ((inhibit-read-only t))
- (save-excursion
- (goto-char (point-min))
- (while (not (eobp))
- (when (looking-at dired-re-sym)
- (re-search-forward " -> \\(.*\\)$")
- (put-text-property (1- (match-beginning 1))
- (match-beginning 1) 'old-link
- (match-string-no-properties 1))
- (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
- (unless wdired-allow-to-redirect-links
- (put-text-property (match-beginning 0)
- (match-end 1) 'read-only t)))
- (forward-line)))))
+(defun wdired--preprocess-symlinks ()
+ (save-excursion
+ (with-silent-modifications
+ (beginning-of-line)
+ (when (looking-at dired-re-sym)
+ (re-search-forward " -> \\(.*\\)$")
+ (put-text-property (1- (match-beginning 1))
+ (match-beginning 1) 'old-link
+ (match-string-no-properties 1))
+ (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
+ (unless wdired-allow-to-redirect-links
+ (put-text-property (match-beginning 0)
+ (match-end 1) 'read-only t))))))
(defun wdired-get-previous-link (&optional old move)
"Return the next symlink target.
@@ -800,10 +838,8 @@ wdired-capitalize-word
(interactive "p")
(wdired-xcase-word 'capitalize-word arg))
-
;; The following code deals with changing the access bits (or
;; permissions) of the files.
-
(defvar wdired-perm-mode-map
(let ((map (make-sparse-keymap)))
(define-key map " " #'wdired-toggle-bit)
@@ -822,34 +858,33 @@ wdired-perm-mode-map
;; Put a keymap property to the permission bits of the files, and store the
;; original name and permissions as a property
-(defun wdired-preprocess-perms ()
- (let ((inhibit-read-only t))
- (setq-local wdired-col-perm nil)
- (save-excursion
- (goto-char (point-min))
- (while (not (eobp))
- (when (and (not (looking-at dired-re-sym))
- (wdired-get-filename)
- (re-search-forward dired-re-perms (line-end-position) 'eol))
- (let ((begin (match-beginning 0))
- (end (match-end 0)))
- (unless wdired-col-perm
- (setq wdired-col-perm (- (current-column) 9)))
- (if (eq wdired-allow-to-change-permissions 'advanced)
- (progn
- (put-text-property begin end 'read-only nil)
- ;; make first permission bit writable
- (put-text-property
- (1- begin) begin 'rear-nonsticky '(read-only)))
- ;; avoid that keymap applies to text following permissions
- (add-text-properties
- (1+ begin) end
- `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
- (put-text-property end (1+ end) 'end-perm t)
- (put-text-property
- begin (1+ begin) 'old-perm (match-string-no-properties 0))))
- (forward-line)
- (beginning-of-line)))))
+(defun wdired--preprocess-perms ()
+ (save-excursion
+ (with-silent-modifications
+ (setq-local wdired--col-perm nil)
+ (beginning-of-line)
+ (when (and (not (looking-at dired-re-sym))
+ (wdired-get-filename)
+ (re-search-forward dired-re-perms
+ (line-end-position) 'eol))
+ (let ((begin (match-beginning 0))
+ (end (match-end 0)))
+ (unless wdired--col-perm
+ (setq wdired--col-perm (- (current-column) 9)))
+ (if (eq wdired-allow-to-change-permissions 'advanced)
+ (progn
+ (put-text-property begin end 'read-only nil)
+ ;; make first permission bit writable
+ (put-text-property
+ (1- begin) begin 'rear-nonsticky '(read-only)))
+ ;; avoid that keymap applies to text following permissions
+ (add-text-properties
+ (1+ begin) end
+ `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
+ (put-text-property end (1+ end) 'end-perm t)
+ (put-text-property
+ begin (1+ begin)
+ 'old-perm (match-string-no-properties 0)))))))
(defun wdired-perm-allowed-in-pos (char pos)
(cond
@@ -865,10 +900,10 @@ wdired-set-bit
"Set a permission bit character."
(interactive)
(if (wdired-perm-allowed-in-pos last-command-event
- (- (current-column) wdired-col-perm))
+ (- (current-column) wdired--col-perm))
(let ((new-bit (char-to-string last-command-event))
(inhibit-read-only t)
- (pos-prop (- (point) (- (current-column) wdired-col-perm))))
+ (pos-prop (- (point) (- (current-column) wdired--col-perm))))
(put-text-property 0 1 'keymap wdired-perm-mode-map new-bit)
(put-text-property 0 1 'read-only t new-bit)
(insert new-bit)
@@ -882,11 +917,11 @@ wdired-toggle-bit
(interactive)
(let ((inhibit-read-only t)
(new-bit "-")
- (pos-prop (- (point) (- (current-column) wdired-col-perm))))
+ (pos-prop (- (point) (- (current-column) wdired--col-perm))))
(if (eq (char-after (point)) ?-)
(setq new-bit
- (if (= (% (- (current-column) wdired-col-perm) 3) 0) "r"
- (if (= (% (- (current-column) wdired-col-perm) 3) 1) "w"
+ (if (= (% (- (current-column) wdired--col-perm) 3) 0) "r"
+ (if (= (% (- (current-column) wdired--col-perm) 3) 1) "w"
"x"))))
(put-text-property 0 1 'keymap wdired-perm-mode-map new-bit)
(put-text-property 0 1 'read-only t new-bit)
--
2.31.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-27 7:39 ` Arthur Miller
@ 2021-03-27 14:56 ` Stefan Monnier
2021-03-27 15:17 ` Arthur Miller
2021-03-27 18:20 ` [PATCH] Lazy wdired preprocessing - BUG Arthur Miller
0 siblings, 2 replies; 24+ messages in thread
From: Stefan Monnier @ 2021-03-27 14:56 UTC (permalink / raw)
To: Arthur Miller; +Cc: emacs-devel
>>> +(defvar wdired-perm-beg) ;; Column where the permission bits start
>>> +(defvar wdired-perm-end) ;; Column where the permission bits stop
>> I think this should use "--" in the names since they are internal variables.
> I just followed naming as already was in wdired.
I know; fixing all the old code to use such conventions is hard, but
I try and make sure that new code at least follows those conventions ;-)
>> `current-column` can be somewhat costly, so we should refrain from
>> calling it twice gratuitously. And here we can even take advantage of
>> the (rarely used and rarely applicable) multi-arg form of `<=` to fix
>> that "for free":
> Ok. Didn't know that (current-column) was expensive. I use now a good 'nuff
> implementation for this purpose (wdired--current-column).
Oh, not *that* costly. Just in the sense that it's
better not to feel free to call it redundantly.
But, I think `wdired--current-column` looks fine: it completely
side-steps the question of what happens if some of the text is
currently invisible.
> It's all yours now. I don't think I will have more time nor possibility
> to work on this, so if this one is not good enough, you will to finnish
> it on your own, or someone else could help. We are waiting a kid any
> day now, so no hobby programming for quite some time over for me :-).
Would it be time to plug in Richard's endorsement of reproduction here?
I pushed your change with the following additional patch on top,
Stefan
commit ed6b9586d74795605debf614bd4328611e1f1c22
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Sat Mar 27 10:54:10 2021 -0400
* lisp/wdired.el: Fix minor regressions and simplify a bit
Use `wdired--current-column` more consistently to avoid mayhem when it
doesn't return the same result as `current-column`.
(wdired--col-perm): Remove, redundant with `wdired--perm-beg`.
(wdired-change-to-wdired-mode): Don't error in empty directory.
(wdired--set-permission-bounds): Set `wdired--perm-beg` when we can't
find permissions. Move `wdired--perm-beg` 1 char further (like
`wdired--col-perm`). Use `wdired--current-column`.
(wdired--point-at-perms-p): Fix when `wdired--perm-beg` is nil.
(wdired--self-insert): Lookup the keymap to know command to call.
(wdired--before-change-fn): Just use `point` instead of `beg`.
Use `with-silent-modifications` here rather than in each of the
`wdired--preprocess-*` functions.
(wdired--preprocess-files): Presume we're at BOL and within
`with-silent-modifications`. Fix application of `read-only`.
(wdired-abort-changes): Don't use `with-silent-modifications` since
we're really modifying the buffer.
(wdired--preprocess-symlinks): Presume we're at BOL and within
`with-silent-modifications`.
(wdired--preprocess-perms): Presume we're at BOL and within
`with-silent-modifications`.
(wdired-set-bit): Add `char` argument. Use `wdired--current-column`.
Copy previous text properties rather than duplicating the code of
`wdired--preprocess-perms`.
(wdired-toggle-bit): Delegate to `wdired-set-bit`.
diff --git a/lisp/wdired.el b/lisp/wdired.el
index 61272d947f..97861a4474 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -189,7 +189,6 @@ wdired-mode-hook
"Hooks run when changing to WDired mode.")
;; Local variables (put here to avoid compilation gripes)
-(defvar wdired--col-perm) ;; Column where the permission bits start
(defvar wdired--perm-beg) ;; Column where the permission bits start
(defvar wdired--perm-end) ;; Column where the permission bits stop
(defvar wdired--old-content)
@@ -233,8 +232,6 @@ wdired-change-to-wdired-mode
(interactive)
(unless (derived-mode-p 'dired-mode)
(error "Not a Dired buffer"))
- (when (directory-empty-p (expand-file-name default-directory))
- (error "No files to be renamed"))
(setq-local wdired--old-content
(buffer-substring (point-min) (point-max)))
(setq-local wdired--old-marks
@@ -264,49 +261,60 @@ wdired-change-to-wdired-mode
(defun wdired--set-permission-bounds ()
(save-excursion
(goto-char (point-min))
- (re-search-forward dired-re-perms nil t 1)
- (goto-char (match-beginning 0))
- (setq-local wdired--perm-beg (current-column))
- (goto-char (match-end 0))
- (setq-local wdired--perm-end (current-column))))
+ (if (not (re-search-forward dired-re-perms nil t 1))
+ (progn
+ (setq-local wdired--perm-beg nil)
+ (setq-local wdired--perm-end nil))
+ (goto-char (match-beginning 0))
+ ;; Add 1 since the first char matched by `dired-re-perms' is the
+ ;; one describing the nature of the entry (dir/symlink/...) rather
+ ;; than its permissions.
+ (setq-local wdired--perm-beg (1+ (wdired--current-column)))
+ (goto-char (match-end 0))
+ (setq-local wdired--perm-end (wdired--current-column)))))
(defun wdired--current-column ()
(- (point) (line-beginning-position)))
(defun wdired--point-at-perms-p ()
- (<= wdired--perm-beg (wdired--current-column) wdired--perm-end))
+ (and wdired--perm-beg
+ (<= wdired--perm-beg (wdired--current-column) wdired--perm-end)))
(defun wdired--line-preprocessed-p ()
(get-text-property (line-beginning-position) 'front-sticky))
(defun wdired--self-insert ()
(interactive)
- (if (wdired--point-at-perms-p)
- (unless (wdired--line-preprocessed-p)
- (wdired--before-change-fn (line-beginning-position) (line-end-position))
- (wdired-toggle-bit))
- (call-interactively 'self-insert-command)))
+ (if (wdired--line-preprocessed-p)
+ (call-interactively 'self-insert-command)
+ (wdired--before-change-fn (line-beginning-position) (line-end-position))
+ (let ((map (get-text-property (point) 'keymap)))
+ (when map
+ (let ((cmd (lookup-key map (this-command-keys))))
+ (call-interactively (or cmd 'self-insert-command)))))))
(defun wdired--before-change-fn (beg end)
(save-excursion
- ;; make sure to process entire lines
- (goto-char beg)
- (setq beg (line-beginning-position))
+ ;; Make sure to process entire lines.
(goto-char end)
(setq end (line-end-position))
+ (goto-char beg)
+ (forward-line 0)
- (while (< beg end)
+ (while (< (point) end)
(unless (wdired--line-preprocessed-p)
- (put-text-property beg (1+ beg) 'front-sticky t)
- (wdired--preprocess-files)
- (when wdired-allow-to-change-permissions
- (wdired--preprocess-perms))
- (when (fboundp 'make-symbolic-link)
- (wdired--preprocess-symlinks)))
- (forward-line)
- (setq beg (point)))
- ;; is this good enough? assumes no extra white lines from dired
- (put-text-property (1- (point-max)) (point-max) 'read-only t)))
+ (with-silent-modifications
+ (put-text-property (point) (1+ (point)) 'front-sticky t)
+ (wdired--preprocess-files)
+ (when wdired-allow-to-change-permissions
+ (wdired--preprocess-perms))
+ (when (fboundp 'make-symbolic-link)
+ (wdired--preprocess-symlinks))))
+ (forward-line))
+ (when (eobp)
+ (with-silent-modifications
+ ;; Is this good enough? Assumes no extra white lines from dired.
+ (put-text-property (1- (point-max)) (point-max) 'read-only t)))))
(defun wdired-isearch-filter-read-only (beg end)
"Skip matches that have a read-only property."
@@ -317,28 +325,26 @@ wdired-isearch-filter-read-only
;; properties so filenames (old and new) can be easily found.
(defun wdired--preprocess-files ()
(save-excursion
- (with-silent-modifications
- (beginning-of-line)
- (let ((used-F (dired-check-switches dired-actual-switches "F" "classify"))
- filename)
- (setq filename (dired-get-filename nil t))
- (when (and filename
- (not (member (file-name-nondirectory filename) '("." ".."))))
- (dired-move-to-filename)
- ;; The rear-nonsticky property below shall ensure that text preceding
- ;; the filename can't be modified.
- (add-text-properties
- (1- (point)) (point) `(old-name ,filename rear-nonsticky (read-only)))
- (put-text-property (- (point) 1) (point) 'read-only t)
- (dired-move-to-end-of-filename t)
- (put-text-property (point) (1+ (point)) 'end-name t))
- (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
- (when (save-excursion
- (and (re-search-backward
- dired-permission-flags-regexp nil t)
- (looking-at "l")
- (search-forward " -> " (line-end-position) t)))
- (goto-char (line-end-position)))))))
+ (let ((used-F (dired-check-switches dired-actual-switches "F" "classify"))
+ (beg (point))
+ (filename (dired-get-filename nil t)))
+ (when (and filename
+ (not (member (file-name-nondirectory filename) '("." ".."))))
+ (dired-move-to-filename)
+ ;; The rear-nonsticky property below shall ensure that text preceding
+ ;; the filename can't be modified.
+ (add-text-properties
+ (1- (point)) (point) `(old-name ,filename rear-nonsticky (read-only)))
+ (put-text-property beg (point) 'read-only t)
+ (dired-move-to-end-of-filename t)
+ (put-text-property (point) (1+ (point)) 'end-name t))
+ (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
+ (when (save-excursion
+ (and (re-search-backward
+ dired-permission-flags-regexp nil t)
+ (looking-at "l")
+ (search-forward " -> " (line-end-position) t)))
+ (goto-char (line-end-position))))))
;; This code is a copy of some dired-get-filename lines.
(defsubst wdired-normalize-filename (file unquotep)
@@ -425,8 +431,8 @@ wdired-change-to-dired-mode
(defun wdired-abort-changes ()
"Abort changes and return to dired mode."
(interactive)
- (remove-hook 'before-change-functions 'wdired--before-change-fn t)
- (with-silent-modifications
+ (remove-hook 'before-change-functions #'wdired--before-change-fn t)
+ (let ((inhibit-read-only t))
(erase-buffer)
(insert wdired--old-content)
(goto-char wdired--old-point))
@@ -451,7 +457,7 @@ wdired-finish-edit
(setq errors (cdr tmp-value))
(setq changes (car tmp-value)))
(when (and wdired-allow-to-change-permissions
- (boundp 'wdired--col-perm)) ; could have been changed
+ wdired--perm-beg) ; could have been changed
(setq tmp-value (wdired-do-perm-changes))
(setq errors (+ errors (cdr tmp-value)))
(setq changes (or changes (car tmp-value))))
@@ -744,17 +750,15 @@ wdired-previous-line
;; Put the needed properties to allow the user to change links' targets
(defun wdired--preprocess-symlinks ()
(save-excursion
- (with-silent-modifications
- (beginning-of-line)
- (when (looking-at dired-re-sym)
- (re-search-forward " -> \\(.*\\)$")
- (put-text-property (1- (match-beginning 1))
- (match-beginning 1) 'old-link
- (match-string-no-properties 1))
- (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
- (unless wdired-allow-to-redirect-links
- (put-text-property (match-beginning 0)
- (match-end 1) 'read-only t))))))
+ (when (looking-at dired-re-sym)
+ (re-search-forward " -> \\(.*\\)$")
+ (put-text-property (1- (match-beginning 1))
+ (match-beginning 1) 'old-link
+ (match-string-no-properties 1))
+ (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
+ (unless wdired-allow-to-redirect-links
+ (put-text-property (match-beginning 0)
+ (match-end 1) 'read-only t)))))
(defun wdired-get-previous-link (&optional old move)
"Return the next symlink target.
@@ -861,31 +865,26 @@ wdired-perm-mode-map
;; original name and permissions as a property
(defun wdired--preprocess-perms ()
(save-excursion
- (with-silent-modifications
- (setq-local wdired--col-perm nil)
- (beginning-of-line)
- (when (and (not (looking-at dired-re-sym))
- (wdired-get-filename)
- (re-search-forward dired-re-perms
- (line-end-position) 'eol))
- (let ((begin (match-beginning 0))
- (end (match-end 0)))
- (unless wdired--col-perm
- (setq wdired--col-perm (- (current-column) 9)))
- (if (eq wdired-allow-to-change-permissions 'advanced)
- (progn
- (put-text-property begin end 'read-only nil)
- ;; make first permission bit writable
- (put-text-property
- (1- begin) begin 'rear-nonsticky '(read-only)))
- ;; avoid that keymap applies to text following permissions
- (add-text-properties
- (1+ begin) end
- `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
- (put-text-property end (1+ end) 'end-perm t)
- (put-text-property
- begin (1+ begin)
- 'old-perm (match-string-no-properties 0)))))))
+ (when (and (not (looking-at dired-re-sym))
+ (wdired-get-filename)
+ (re-search-forward dired-re-perms
+ (line-end-position) 'eol))
+ (let ((begin (match-beginning 0))
+ (end (match-end 0)))
+ (if (eq wdired-allow-to-change-permissions 'advanced)
+ (progn
+ (put-text-property begin end 'read-only nil)
+ ;; make first permission bit writable
+ (put-text-property
+ (1- begin) begin 'rear-nonsticky '(read-only)))
+ ;; avoid that keymap applies to text following permissions
+ (add-text-properties
+ (1+ begin) end
+ `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
+ (put-text-property end (1+ end) 'end-perm t)
+ (put-text-property
+ begin (1+ begin)
+ 'old-perm (match-string-no-properties 0))))))
(defun wdired-perm-allowed-in-pos (char pos)
(cond
@@ -897,39 +896,30 @@ wdired-perm-allowed-in-pos
((memq char '(?t ?T)) (= pos 8))
((= char ?l) (= pos 5))))
-(defun wdired-set-bit ()
+(defun wdired-set-bit (&optional char)
"Set a permission bit character."
- (interactive)
- (if (wdired-perm-allowed-in-pos last-command-event
- (- (current-column) wdired--col-perm))
- (let ((new-bit (char-to-string last-command-event))
+ (interactive (list last-command-event))
+ (unless char (setq char last-command-event))
+ (if (wdired-perm-allowed-in-pos char
+ (- (wdired--current-column) wdired--perm-beg))
+ (let ((new-bit (char-to-string char))
(inhibit-read-only t)
- (pos-prop (- (point) (- (current-column) wdired--col-perm))))
- (put-text-property 0 1 'keymap wdired-perm-mode-map new-bit)
- (put-text-property 0 1 'read-only t new-bit)
+ (pos-prop (+ (line-beginning-position) wdired--perm-beg)))
+ (set-text-properties 0 1 (text-properties-at (point)) new-bit)
(insert new-bit)
(delete-char 1)
- (put-text-property (1- pos-prop) pos-prop 'perm-changed t)
- (put-text-property (1- (point)) (point) 'rear-nonsticky '(keymap)))
+ (put-text-property (1- pos-prop) pos-prop 'perm-changed t))
(forward-char 1)))
(defun wdired-toggle-bit ()
"Toggle the permission bit at point."
(interactive)
- (let ((inhibit-read-only t)
- (new-bit "-")
- (pos-prop (- (point) (- (current-column) wdired--col-perm))))
- (if (eq (char-after (point)) ?-)
- (setq new-bit
- (if (= (% (- (current-column) wdired--col-perm) 3) 0) "r"
- (if (= (% (- (current-column) wdired--col-perm) 3) 1) "w"
- "x"))))
- (put-text-property 0 1 'keymap wdired-perm-mode-map new-bit)
- (put-text-property 0 1 'read-only t new-bit)
- (insert new-bit)
- (delete-char 1)
- (put-text-property (1- pos-prop) pos-prop 'perm-changed t)
- (put-text-property (1- (point)) (point) 'rear-nonsticky '(keymap))))
+ (wdired-set-bit
+ (cond
+ ((not (eq (char-after (point)) ?-)) ?-)
+ ((= (% (- (wdired--current-column) wdired--perm-beg) 3) 0) ?r)
+ ((= (% (- (wdired--current-column) wdired--perm-beg) 3) 1) ?w)
+ (t ?x))))
(defun wdired-mouse-toggle-bit (event)
"Toggle the permission bit that was left clicked."
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-27 14:56 ` Stefan Monnier
@ 2021-03-27 15:17 ` Arthur Miller
2021-03-27 15:56 ` Stefan Monnier
2021-03-27 18:20 ` [PATCH] Lazy wdired preprocessing - BUG Arthur Miller
1 sibling, 1 reply; 24+ messages in thread
From: Arthur Miller @ 2021-03-27 15:17 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>>> +(defvar wdired-perm-beg) ;; Column where the permission bits start
>>>> +(defvar wdired-perm-end) ;; Column where the permission bits stop
>>> I think this should use "--" in the names since they are internal variables.
>> I just followed naming as already was in wdired.
>
> I know; fixing all the old code to use such conventions is hard, but
> I try and make sure that new code at least follows those conventions ;-)
>
>>> `current-column` can be somewhat costly, so we should refrain from
>>> calling it twice gratuitously. And here we can even take advantage of
>>> the (rarely used and rarely applicable) multi-arg form of `<=` to fix
>>> that "for free":
>> Ok. Didn't know that (current-column) was expensive. I use now a good 'nuff
>> implementation for this purpose (wdired--current-column).
>
> Oh, not *that* costly. Just in the sense that it's
> better not to feel free to call it redundantly.
> But, I think `wdired--current-column` looks fine: it completely
> side-steps the question of what happens if some of the text is
> currently invisible.
>
>> It's all yours now. I don't think I will have more time nor possibility
>> to work on this, so if this one is not good enough, you will to finnish
>> it on your own, or someone else could help. We are waiting a kid any
>> day now, so no hobby programming for quite some time over for me :-).
>
> Would it be time to plug in Richard's endorsement of reproduction here?
Hmm, I don't think I know what it's about; but I trust your and RMS
judgement! :-)
> I pushed your change with the following additional patch on top,
Cool, thnks!
>
> Stefan
>
>
> commit ed6b9586d74795605debf614bd4328611e1f1c22
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat Mar 27 10:54:10 2021 -0400
>
> * lisp/wdired.el: Fix minor regressions and simplify a bit
>
> Use `wdired--current-column` more consistently to avoid mayhem when it
> doesn't return the same result as `current-column`.
>
> (wdired--col-perm): Remove, redundant with `wdired--perm-beg`.
> (wdired-change-to-wdired-mode): Don't error in empty directory.
> (wdired--set-permission-bounds): Set `wdired--perm-beg` when we can't
> find permissions. Move `wdired--perm-beg` 1 char further (like
> `wdired--col-perm`). Use `wdired--current-column`.
> (wdired--point-at-perms-p): Fix when `wdired--perm-beg` is nil.
> (wdired--self-insert): Lookup the keymap to know command to call.
> (wdired--before-change-fn): Just use `point` instead of `beg`.
> Use `with-silent-modifications` here rather than in each of the
> `wdired--preprocess-*` functions.
> (wdired--preprocess-files): Presume we're at BOL and within
> `with-silent-modifications`. Fix application of `read-only`.
> (wdired-abort-changes): Don't use `with-silent-modifications` since
> we're really modifying the buffer.
> (wdired--preprocess-symlinks): Presume we're at BOL and within
> `with-silent-modifications`.
> (wdired--preprocess-perms): Presume we're at BOL and within
> `with-silent-modifications`.
> (wdired-set-bit): Add `char` argument. Use `wdired--current-column`.
> Copy previous text properties rather than duplicating the code of
> `wdired--preprocess-perms`.
> (wdired-toggle-bit): Delegate to `wdired-set-bit`.
>
> diff --git a/lisp/wdired.el b/lisp/wdired.el
> index 61272d947f..97861a4474 100644
> --- a/lisp/wdired.el
> +++ b/lisp/wdired.el
> @@ -189,7 +189,6 @@ wdired-mode-hook
> "Hooks run when changing to WDired mode.")
>
> ;; Local variables (put here to avoid compilation gripes)
> -(defvar wdired--col-perm) ;; Column where the permission bits start
> (defvar wdired--perm-beg) ;; Column where the permission bits start
> (defvar wdired--perm-end) ;; Column where the permission bits stop
> (defvar wdired--old-content)
> @@ -233,8 +232,6 @@ wdired-change-to-wdired-mode
> (interactive)
> (unless (derived-mode-p 'dired-mode)
> (error "Not a Dired buffer"))
> - (when (directory-empty-p (expand-file-name default-directory))
> - (error "No files to be renamed"))
> (setq-local wdired--old-content
> (buffer-substring (point-min) (point-max)))
> (setq-local wdired--old-marks
> @@ -264,49 +261,60 @@ wdired-change-to-wdired-mode
> (defun wdired--set-permission-bounds ()
> (save-excursion
> (goto-char (point-min))
> - (re-search-forward dired-re-perms nil t 1)
> - (goto-char (match-beginning 0))
> - (setq-local wdired--perm-beg (current-column))
> - (goto-char (match-end 0))
> - (setq-local wdired--perm-end (current-column))))
> + (if (not (re-search-forward dired-re-perms nil t 1))
> + (progn
> + (setq-local wdired--perm-beg nil)
> + (setq-local wdired--perm-end nil))
> + (goto-char (match-beginning 0))
> + ;; Add 1 since the first char matched by `dired-re-perms' is the
> + ;; one describing the nature of the entry (dir/symlink/...) rather
> + ;; than its permissions.
> + (setq-local wdired--perm-beg (1+ (wdired--current-column)))
> + (goto-char (match-end 0))
> + (setq-local wdired--perm-end (wdired--current-column)))))
>
> (defun wdired--current-column ()
> (- (point) (line-beginning-position)))
>
> (defun wdired--point-at-perms-p ()
> - (<= wdired--perm-beg (wdired--current-column) wdired--perm-end))
> + (and wdired--perm-beg
> + (<= wdired--perm-beg (wdired--current-column) wdired--perm-end)))
>
> (defun wdired--line-preprocessed-p ()
> (get-text-property (line-beginning-position) 'front-sticky))
>
> (defun wdired--self-insert ()
> (interactive)
> - (if (wdired--point-at-perms-p)
> - (unless (wdired--line-preprocessed-p)
> - (wdired--before-change-fn (line-beginning-position) (line-end-position))
> - (wdired-toggle-bit))
> - (call-interactively 'self-insert-command)))
> + (if (wdired--line-preprocessed-p)
> + (call-interactively 'self-insert-command)
> + (wdired--before-change-fn (line-beginning-position) (line-end-position))
> + (let ((map (get-text-property (point) 'keymap)))
> + (when map
> + (let ((cmd (lookup-key map (this-command-keys))))
> + (call-interactively (or cmd 'self-insert-command)))))))
>
> (defun wdired--before-change-fn (beg end)
> (save-excursion
> - ;; make sure to process entire lines
> - (goto-char beg)
> - (setq beg (line-beginning-position))
> + ;; Make sure to process entire lines.
> (goto-char end)
> (setq end (line-end-position))
> + (goto-char beg)
> + (forward-line 0)
>
> - (while (< beg end)
> + (while (< (point) end)
> (unless (wdired--line-preprocessed-p)
> - (put-text-property beg (1+ beg) 'front-sticky t)
> - (wdired--preprocess-files)
> - (when wdired-allow-to-change-permissions
> - (wdired--preprocess-perms))
> - (when (fboundp 'make-symbolic-link)
> - (wdired--preprocess-symlinks)))
> - (forward-line)
> - (setq beg (point)))
> - ;; is this good enough? assumes no extra white lines from dired
> - (put-text-property (1- (point-max)) (point-max) 'read-only t)))
> + (with-silent-modifications
> + (put-text-property (point) (1+ (point)) 'front-sticky t)
> + (wdired--preprocess-files)
> + (when wdired-allow-to-change-permissions
> + (wdired--preprocess-perms))
> + (when (fboundp 'make-symbolic-link)
> + (wdired--preprocess-symlinks))))
> + (forward-line))
> + (when (eobp)
> + (with-silent-modifications
> + ;; Is this good enough? Assumes no extra white lines from dired.
> + (put-text-property (1- (point-max)) (point-max) 'read-only t)))))
>
> (defun wdired-isearch-filter-read-only (beg end)
> "Skip matches that have a read-only property."
> @@ -317,28 +325,26 @@ wdired-isearch-filter-read-only
> ;; properties so filenames (old and new) can be easily found.
> (defun wdired--preprocess-files ()
> (save-excursion
> - (with-silent-modifications
> - (beginning-of-line)
> - (let ((used-F (dired-check-switches dired-actual-switches "F" "classify"))
> - filename)
> - (setq filename (dired-get-filename nil t))
> - (when (and filename
> - (not (member (file-name-nondirectory filename) '("." ".."))))
> - (dired-move-to-filename)
> - ;; The rear-nonsticky property below shall ensure that text preceding
> - ;; the filename can't be modified.
> - (add-text-properties
> - (1- (point)) (point) `(old-name ,filename rear-nonsticky (read-only)))
> - (put-text-property (- (point) 1) (point) 'read-only t)
> - (dired-move-to-end-of-filename t)
> - (put-text-property (point) (1+ (point)) 'end-name t))
> - (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
> - (when (save-excursion
> - (and (re-search-backward
> - dired-permission-flags-regexp nil t)
> - (looking-at "l")
> - (search-forward " -> " (line-end-position) t)))
> - (goto-char (line-end-position)))))))
> + (let ((used-F (dired-check-switches dired-actual-switches "F" "classify"))
> + (beg (point))
> + (filename (dired-get-filename nil t)))
> + (when (and filename
> + (not (member (file-name-nondirectory filename) '("." ".."))))
> + (dired-move-to-filename)
> + ;; The rear-nonsticky property below shall ensure that text preceding
> + ;; the filename can't be modified.
> + (add-text-properties
> + (1- (point)) (point) `(old-name ,filename rear-nonsticky (read-only)))
> + (put-text-property beg (point) 'read-only t)
> + (dired-move-to-end-of-filename t)
> + (put-text-property (point) (1+ (point)) 'end-name t))
> + (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
> + (when (save-excursion
> + (and (re-search-backward
> + dired-permission-flags-regexp nil t)
> + (looking-at "l")
> + (search-forward " -> " (line-end-position) t)))
> + (goto-char (line-end-position))))))
>
> ;; This code is a copy of some dired-get-filename lines.
> (defsubst wdired-normalize-filename (file unquotep)
> @@ -425,8 +431,8 @@ wdired-change-to-dired-mode
> (defun wdired-abort-changes ()
> "Abort changes and return to dired mode."
> (interactive)
> - (remove-hook 'before-change-functions 'wdired--before-change-fn t)
> - (with-silent-modifications
> + (remove-hook 'before-change-functions #'wdired--before-change-fn t)
> + (let ((inhibit-read-only t))
> (erase-buffer)
> (insert wdired--old-content)
> (goto-char wdired--old-point))
> @@ -451,7 +457,7 @@ wdired-finish-edit
> (setq errors (cdr tmp-value))
> (setq changes (car tmp-value)))
> (when (and wdired-allow-to-change-permissions
> - (boundp 'wdired--col-perm)) ; could have been changed
> + wdired--perm-beg) ; could have been changed
> (setq tmp-value (wdired-do-perm-changes))
> (setq errors (+ errors (cdr tmp-value)))
> (setq changes (or changes (car tmp-value))))
> @@ -744,17 +750,15 @@ wdired-previous-line
> ;; Put the needed properties to allow the user to change links' targets
> (defun wdired--preprocess-symlinks ()
> (save-excursion
> - (with-silent-modifications
> - (beginning-of-line)
> - (when (looking-at dired-re-sym)
> - (re-search-forward " -> \\(.*\\)$")
> - (put-text-property (1- (match-beginning 1))
> - (match-beginning 1) 'old-link
> - (match-string-no-properties 1))
> - (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
> - (unless wdired-allow-to-redirect-links
> - (put-text-property (match-beginning 0)
> - (match-end 1) 'read-only t))))))
> + (when (looking-at dired-re-sym)
> + (re-search-forward " -> \\(.*\\)$")
> + (put-text-property (1- (match-beginning 1))
> + (match-beginning 1) 'old-link
> + (match-string-no-properties 1))
> + (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
> + (unless wdired-allow-to-redirect-links
> + (put-text-property (match-beginning 0)
> + (match-end 1) 'read-only t)))))
>
> (defun wdired-get-previous-link (&optional old move)
> "Return the next symlink target.
> @@ -861,31 +865,26 @@ wdired-perm-mode-map
> ;; original name and permissions as a property
> (defun wdired--preprocess-perms ()
> (save-excursion
> - (with-silent-modifications
> - (setq-local wdired--col-perm nil)
> - (beginning-of-line)
> - (when (and (not (looking-at dired-re-sym))
> - (wdired-get-filename)
> - (re-search-forward dired-re-perms
> - (line-end-position) 'eol))
> - (let ((begin (match-beginning 0))
> - (end (match-end 0)))
> - (unless wdired--col-perm
> - (setq wdired--col-perm (- (current-column) 9)))
> - (if (eq wdired-allow-to-change-permissions 'advanced)
> - (progn
> - (put-text-property begin end 'read-only nil)
> - ;; make first permission bit writable
> - (put-text-property
> - (1- begin) begin 'rear-nonsticky '(read-only)))
> - ;; avoid that keymap applies to text following permissions
> - (add-text-properties
> - (1+ begin) end
> - `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
> - (put-text-property end (1+ end) 'end-perm t)
> - (put-text-property
> - begin (1+ begin)
> - 'old-perm (match-string-no-properties 0)))))))
> + (when (and (not (looking-at dired-re-sym))
> + (wdired-get-filename)
> + (re-search-forward dired-re-perms
> + (line-end-position) 'eol))
> + (let ((begin (match-beginning 0))
> + (end (match-end 0)))
> + (if (eq wdired-allow-to-change-permissions 'advanced)
> + (progn
> + (put-text-property begin end 'read-only nil)
> + ;; make first permission bit writable
> + (put-text-property
> + (1- begin) begin 'rear-nonsticky '(read-only)))
> + ;; avoid that keymap applies to text following permissions
> + (add-text-properties
> + (1+ begin) end
> + `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
> + (put-text-property end (1+ end) 'end-perm t)
> + (put-text-property
> + begin (1+ begin)
> + 'old-perm (match-string-no-properties 0))))))
>
> (defun wdired-perm-allowed-in-pos (char pos)
> (cond
> @@ -897,39 +896,30 @@ wdired-perm-allowed-in-pos
> ((memq char '(?t ?T)) (= pos 8))
> ((= char ?l) (= pos 5))))
>
> -(defun wdired-set-bit ()
> +(defun wdired-set-bit (&optional char)
> "Set a permission bit character."
> - (interactive)
> - (if (wdired-perm-allowed-in-pos last-command-event
> - (- (current-column) wdired--col-perm))
> - (let ((new-bit (char-to-string last-command-event))
> + (interactive (list last-command-event))
> + (unless char (setq char last-command-event))
> + (if (wdired-perm-allowed-in-pos char
> + (- (wdired--current-column) wdired--perm-beg))
> + (let ((new-bit (char-to-string char))
> (inhibit-read-only t)
> - (pos-prop (- (point) (- (current-column) wdired--col-perm))))
> - (put-text-property 0 1 'keymap wdired-perm-mode-map new-bit)
> - (put-text-property 0 1 'read-only t new-bit)
> + (pos-prop (+ (line-beginning-position) wdired--perm-beg)))
> + (set-text-properties 0 1 (text-properties-at (point)) new-bit)
> (insert new-bit)
> (delete-char 1)
> - (put-text-property (1- pos-prop) pos-prop 'perm-changed t)
> - (put-text-property (1- (point)) (point) 'rear-nonsticky '(keymap)))
> + (put-text-property (1- pos-prop) pos-prop 'perm-changed t))
> (forward-char 1)))
>
> (defun wdired-toggle-bit ()
> "Toggle the permission bit at point."
> (interactive)
> - (let ((inhibit-read-only t)
> - (new-bit "-")
> - (pos-prop (- (point) (- (current-column) wdired--col-perm))))
> - (if (eq (char-after (point)) ?-)
> - (setq new-bit
> - (if (= (% (- (current-column) wdired--col-perm) 3) 0) "r"
> - (if (= (% (- (current-column) wdired--col-perm) 3) 1) "w"
> - "x"))))
> - (put-text-property 0 1 'keymap wdired-perm-mode-map new-bit)
> - (put-text-property 0 1 'read-only t new-bit)
> - (insert new-bit)
> - (delete-char 1)
> - (put-text-property (1- pos-prop) pos-prop 'perm-changed t)
> - (put-text-property (1- (point)) (point) 'rear-nonsticky '(keymap))))
> + (wdired-set-bit
> + (cond
> + ((not (eq (char-after (point)) ?-)) ?-)
> + ((= (% (- (wdired--current-column) wdired--perm-beg) 3) 0) ?r)
> + ((= (% (- (wdired--current-column) wdired--perm-beg) 3) 1) ?w)
> + (t ?x))))
>
> (defun wdired-mouse-toggle-bit (event)
> "Toggle the permission bit that was left clicked."
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-27 15:17 ` Arthur Miller
@ 2021-03-27 15:56 ` Stefan Monnier
2021-03-27 17:01 ` Arthur Miller
0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2021-03-27 15:56 UTC (permalink / raw)
To: Arthur Miller; +Cc: emacs-devel
>> Would it be time to plug in Richard's endorsement of reproduction here?
> Hmm, I don't think I know what it's about; but I trust your and RMS
> judgement! :-)
https://lists.gnu.org/r/emacs-devel/2005-04/msg01005.html
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-27 15:56 ` Stefan Monnier
@ 2021-03-27 17:01 ` Arthur Miller
0 siblings, 0 replies; 24+ messages in thread
From: Arthur Miller @ 2021-03-27 17:01 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> Would it be time to plug in Richard's endorsement of reproduction here?
>> Hmm, I don't think I know what it's about; but I trust your and RMS
>> judgement! :-)
>
> https://lists.gnu.org/r/emacs-devel/2005-04/msg01005.html
>
:-)
Inded! Though I am not sure I'll show that one to the best of the all
best ones ^^.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing - BUG
2021-03-27 14:56 ` Stefan Monnier
2021-03-27 15:17 ` Arthur Miller
@ 2021-03-27 18:20 ` Arthur Miller
2021-03-27 18:32 ` Stefan Monnier
1 sibling, 1 reply; 24+ messages in thread
From: Arthur Miller @ 2021-03-27 18:20 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>>> +(defvar wdired-perm-beg) ;; Column where the permission bits start
>>>> +(defvar wdired-perm-end) ;; Column where the permission bits stop
>>> I think this should use "--" in the names since they are internal variables.
>> I just followed naming as already was in wdired.
>
> I know; fixing all the old code to use such conventions is hard, but
> I try and make sure that new code at least follows those conventions ;-)
>
>>> `current-column` can be somewhat costly, so we should refrain from
>>> calling it twice gratuitously. And here we can even take advantage of
>>> the (rarely used and rarely applicable) multi-arg form of `<=` to fix
>>> that "for free":
>> Ok. Didn't know that (current-column) was expensive. I use now a good 'nuff
>> implementation for this purpose (wdired--current-column).
>
> Oh, not *that* costly. Just in the sense that it's
> better not to feel free to call it redundantly.
> But, I think `wdired--current-column` looks fine: it completely
> side-steps the question of what happens if some of the text is
> currently invisible.
>
>> It's all yours now. I don't think I will have more time nor possibility
>> to work on this, so if this one is not good enough, you will to finnish
>> it on your own, or someone else could help. We are waiting a kid any
>> day now, so no hobby programming for quite some time over for me :-).
>
> Would it be time to plug in Richard's endorsement of reproduction here?
>
> I pushed your change with the following additional patch on top,
I have just rebuild with the new patch. I see a small bug: when entered
wdired mode, and trying to do a very first change, it requires two
keypresses. The first key press, I think, gets consumed in first call to
wdired--self insert, so user is required to press again a key to do
actual edit.
I was struggling with that one so one of the reasons I pushed key event back
onto the queue was that one.
> + (let ((cmd (lookup-key map (this-command-keys))))
> + (call-interactively (or cmd 'self-insert-command)))))))
I am not sure how that part works, so I am not attempting to suggest a
change, not sure if it's that part, but I think the keypress is
consumed in call to preprocess function.
>
> Stefan
>
>
> commit ed6b9586d74795605debf614bd4328611e1f1c22
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat Mar 27 10:54:10 2021 -0400
>
> * lisp/wdired.el: Fix minor regressions and simplify a bit
>
> Use `wdired--current-column` more consistently to avoid mayhem when it
> doesn't return the same result as `current-column`.
>
> (wdired--col-perm): Remove, redundant with `wdired--perm-beg`.
> (wdired-change-to-wdired-mode): Don't error in empty directory.
> (wdired--set-permission-bounds): Set `wdired--perm-beg` when we can't
> find permissions. Move `wdired--perm-beg` 1 char further (like
> `wdired--col-perm`). Use `wdired--current-column`.
> (wdired--point-at-perms-p): Fix when `wdired--perm-beg` is nil.
> (wdired--self-insert): Lookup the keymap to know command to call.
> (wdired--before-change-fn): Just use `point` instead of `beg`.
> Use `with-silent-modifications` here rather than in each of the
> `wdired--preprocess-*` functions.
> (wdired--preprocess-files): Presume we're at BOL and within
> `with-silent-modifications`. Fix application of `read-only`.
> (wdired-abort-changes): Don't use `with-silent-modifications` since
> we're really modifying the buffer.
> (wdired--preprocess-symlinks): Presume we're at BOL and within
> `with-silent-modifications`.
> (wdired--preprocess-perms): Presume we're at BOL and within
> `with-silent-modifications`.
> (wdired-set-bit): Add `char` argument. Use `wdired--current-column`.
> Copy previous text properties rather than duplicating the code of
> `wdired--preprocess-perms`.
> (wdired-toggle-bit): Delegate to `wdired-set-bit`.
>
> diff --git a/lisp/wdired.el b/lisp/wdired.el
> index 61272d947f..97861a4474 100644
> --- a/lisp/wdired.el
> +++ b/lisp/wdired.el
> @@ -189,7 +189,6 @@ wdired-mode-hook
> "Hooks run when changing to WDired mode.")
>
> ;; Local variables (put here to avoid compilation gripes)
> -(defvar wdired--col-perm) ;; Column where the permission bits start
> (defvar wdired--perm-beg) ;; Column where the permission bits start
> (defvar wdired--perm-end) ;; Column where the permission bits stop
> (defvar wdired--old-content)
> @@ -233,8 +232,6 @@ wdired-change-to-wdired-mode
> (interactive)
> (unless (derived-mode-p 'dired-mode)
> (error "Not a Dired buffer"))
> - (when (directory-empty-p (expand-file-name default-directory))
> - (error "No files to be renamed"))
> (setq-local wdired--old-content
> (buffer-substring (point-min) (point-max)))
> (setq-local wdired--old-marks
> @@ -264,49 +261,60 @@ wdired-change-to-wdired-mode
> (defun wdired--set-permission-bounds ()
> (save-excursion
> (goto-char (point-min))
> - (re-search-forward dired-re-perms nil t 1)
> - (goto-char (match-beginning 0))
> - (setq-local wdired--perm-beg (current-column))
> - (goto-char (match-end 0))
> - (setq-local wdired--perm-end (current-column))))
> + (if (not (re-search-forward dired-re-perms nil t 1))
> + (progn
> + (setq-local wdired--perm-beg nil)
> + (setq-local wdired--perm-end nil))
> + (goto-char (match-beginning 0))
> + ;; Add 1 since the first char matched by `dired-re-perms' is the
> + ;; one describing the nature of the entry (dir/symlink/...) rather
> + ;; than its permissions.
> + (setq-local wdired--perm-beg (1+ (wdired--current-column)))
> + (goto-char (match-end 0))
> + (setq-local wdired--perm-end (wdired--current-column)))))
>
> (defun wdired--current-column ()
> (- (point) (line-beginning-position)))
>
> (defun wdired--point-at-perms-p ()
> - (<= wdired--perm-beg (wdired--current-column) wdired--perm-end))
> + (and wdired--perm-beg
> + (<= wdired--perm-beg (wdired--current-column) wdired--perm-end)))
>
> (defun wdired--line-preprocessed-p ()
> (get-text-property (line-beginning-position) 'front-sticky))
>
> (defun wdired--self-insert ()
> (interactive)
> - (if (wdired--point-at-perms-p)
> - (unless (wdired--line-preprocessed-p)
> - (wdired--before-change-fn (line-beginning-position) (line-end-position))
> - (wdired-toggle-bit))
> - (call-interactively 'self-insert-command)))
> + (if (wdired--line-preprocessed-p)
> + (call-interactively 'self-insert-command)
> + (wdired--before-change-fn (line-beginning-position) (line-end-position))
> + (let ((map (get-text-property (point) 'keymap)))
> + (when map
> + (let ((cmd (lookup-key map (this-command-keys))))
> + (call-interactively (or cmd 'self-insert-command)))))))
>
> (defun wdired--before-change-fn (beg end)
> (save-excursion
> - ;; make sure to process entire lines
> - (goto-char beg)
> - (setq beg (line-beginning-position))
> + ;; Make sure to process entire lines.
> (goto-char end)
> (setq end (line-end-position))
> + (goto-char beg)
> + (forward-line 0)
>
> - (while (< beg end)
> + (while (< (point) end)
> (unless (wdired--line-preprocessed-p)
> - (put-text-property beg (1+ beg) 'front-sticky t)
> - (wdired--preprocess-files)
> - (when wdired-allow-to-change-permissions
> - (wdired--preprocess-perms))
> - (when (fboundp 'make-symbolic-link)
> - (wdired--preprocess-symlinks)))
> - (forward-line)
> - (setq beg (point)))
> - ;; is this good enough? assumes no extra white lines from dired
> - (put-text-property (1- (point-max)) (point-max) 'read-only t)))
> + (with-silent-modifications
> + (put-text-property (point) (1+ (point)) 'front-sticky t)
> + (wdired--preprocess-files)
> + (when wdired-allow-to-change-permissions
> + (wdired--preprocess-perms))
> + (when (fboundp 'make-symbolic-link)
> + (wdired--preprocess-symlinks))))
> + (forward-line))
> + (when (eobp)
> + (with-silent-modifications
> + ;; Is this good enough? Assumes no extra white lines from dired.
> + (put-text-property (1- (point-max)) (point-max) 'read-only t)))))
>
> (defun wdired-isearch-filter-read-only (beg end)
> "Skip matches that have a read-only property."
> @@ -317,28 +325,26 @@ wdired-isearch-filter-read-only
> ;; properties so filenames (old and new) can be easily found.
> (defun wdired--preprocess-files ()
> (save-excursion
> - (with-silent-modifications
> - (beginning-of-line)
> - (let ((used-F (dired-check-switches dired-actual-switches "F" "classify"))
> - filename)
> - (setq filename (dired-get-filename nil t))
> - (when (and filename
> - (not (member (file-name-nondirectory filename) '("." ".."))))
> - (dired-move-to-filename)
> - ;; The rear-nonsticky property below shall ensure that text preceding
> - ;; the filename can't be modified.
> - (add-text-properties
> - (1- (point)) (point) `(old-name ,filename rear-nonsticky (read-only)))
> - (put-text-property (- (point) 1) (point) 'read-only t)
> - (dired-move-to-end-of-filename t)
> - (put-text-property (point) (1+ (point)) 'end-name t))
> - (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
> - (when (save-excursion
> - (and (re-search-backward
> - dired-permission-flags-regexp nil t)
> - (looking-at "l")
> - (search-forward " -> " (line-end-position) t)))
> - (goto-char (line-end-position)))))))
> + (let ((used-F (dired-check-switches dired-actual-switches "F" "classify"))
> + (beg (point))
> + (filename (dired-get-filename nil t)))
> + (when (and filename
> + (not (member (file-name-nondirectory filename) '("." ".."))))
> + (dired-move-to-filename)
> + ;; The rear-nonsticky property below shall ensure that text preceding
> + ;; the filename can't be modified.
> + (add-text-properties
> + (1- (point)) (point) `(old-name ,filename rear-nonsticky (read-only)))
> + (put-text-property beg (point) 'read-only t)
> + (dired-move-to-end-of-filename t)
> + (put-text-property (point) (1+ (point)) 'end-name t))
> + (when (and used-F (looking-at "[*/@|=>]$")) (forward-char))
> + (when (save-excursion
> + (and (re-search-backward
> + dired-permission-flags-regexp nil t)
> + (looking-at "l")
> + (search-forward " -> " (line-end-position) t)))
> + (goto-char (line-end-position))))))
>
> ;; This code is a copy of some dired-get-filename lines.
> (defsubst wdired-normalize-filename (file unquotep)
> @@ -425,8 +431,8 @@ wdired-change-to-dired-mode
> (defun wdired-abort-changes ()
> "Abort changes and return to dired mode."
> (interactive)
> - (remove-hook 'before-change-functions 'wdired--before-change-fn t)
> - (with-silent-modifications
> + (remove-hook 'before-change-functions #'wdired--before-change-fn t)
> + (let ((inhibit-read-only t))
> (erase-buffer)
> (insert wdired--old-content)
> (goto-char wdired--old-point))
> @@ -451,7 +457,7 @@ wdired-finish-edit
> (setq errors (cdr tmp-value))
> (setq changes (car tmp-value)))
> (when (and wdired-allow-to-change-permissions
> - (boundp 'wdired--col-perm)) ; could have been changed
> + wdired--perm-beg) ; could have been changed
> (setq tmp-value (wdired-do-perm-changes))
> (setq errors (+ errors (cdr tmp-value)))
> (setq changes (or changes (car tmp-value))))
> @@ -744,17 +750,15 @@ wdired-previous-line
> ;; Put the needed properties to allow the user to change links' targets
> (defun wdired--preprocess-symlinks ()
> (save-excursion
> - (with-silent-modifications
> - (beginning-of-line)
> - (when (looking-at dired-re-sym)
> - (re-search-forward " -> \\(.*\\)$")
> - (put-text-property (1- (match-beginning 1))
> - (match-beginning 1) 'old-link
> - (match-string-no-properties 1))
> - (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
> - (unless wdired-allow-to-redirect-links
> - (put-text-property (match-beginning 0)
> - (match-end 1) 'read-only t))))))
> + (when (looking-at dired-re-sym)
> + (re-search-forward " -> \\(.*\\)$")
> + (put-text-property (1- (match-beginning 1))
> + (match-beginning 1) 'old-link
> + (match-string-no-properties 1))
> + (put-text-property (match-end 1) (1+ (match-end 1)) 'end-link t)
> + (unless wdired-allow-to-redirect-links
> + (put-text-property (match-beginning 0)
> + (match-end 1) 'read-only t)))))
>
> (defun wdired-get-previous-link (&optional old move)
> "Return the next symlink target.
> @@ -861,31 +865,26 @@ wdired-perm-mode-map
> ;; original name and permissions as a property
> (defun wdired--preprocess-perms ()
> (save-excursion
> - (with-silent-modifications
> - (setq-local wdired--col-perm nil)
> - (beginning-of-line)
> - (when (and (not (looking-at dired-re-sym))
> - (wdired-get-filename)
> - (re-search-forward dired-re-perms
> - (line-end-position) 'eol))
> - (let ((begin (match-beginning 0))
> - (end (match-end 0)))
> - (unless wdired--col-perm
> - (setq wdired--col-perm (- (current-column) 9)))
> - (if (eq wdired-allow-to-change-permissions 'advanced)
> - (progn
> - (put-text-property begin end 'read-only nil)
> - ;; make first permission bit writable
> - (put-text-property
> - (1- begin) begin 'rear-nonsticky '(read-only)))
> - ;; avoid that keymap applies to text following permissions
> - (add-text-properties
> - (1+ begin) end
> - `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
> - (put-text-property end (1+ end) 'end-perm t)
> - (put-text-property
> - begin (1+ begin)
> - 'old-perm (match-string-no-properties 0)))))))
> + (when (and (not (looking-at dired-re-sym))
> + (wdired-get-filename)
> + (re-search-forward dired-re-perms
> + (line-end-position) 'eol))
> + (let ((begin (match-beginning 0))
> + (end (match-end 0)))
> + (if (eq wdired-allow-to-change-permissions 'advanced)
> + (progn
> + (put-text-property begin end 'read-only nil)
> + ;; make first permission bit writable
> + (put-text-property
> + (1- begin) begin 'rear-nonsticky '(read-only)))
> + ;; avoid that keymap applies to text following permissions
> + (add-text-properties
> + (1+ begin) end
> + `(keymap ,wdired-perm-mode-map rear-nonsticky (keymap))))
> + (put-text-property end (1+ end) 'end-perm t)
> + (put-text-property
> + begin (1+ begin)
> + 'old-perm (match-string-no-properties 0))))))
>
> (defun wdired-perm-allowed-in-pos (char pos)
> (cond
> @@ -897,39 +896,30 @@ wdired-perm-allowed-in-pos
> ((memq char '(?t ?T)) (= pos 8))
> ((= char ?l) (= pos 5))))
>
> -(defun wdired-set-bit ()
> +(defun wdired-set-bit (&optional char)
> "Set a permission bit character."
> - (interactive)
> - (if (wdired-perm-allowed-in-pos last-command-event
> - (- (current-column) wdired--col-perm))
> - (let ((new-bit (char-to-string last-command-event))
> + (interactive (list last-command-event))
> + (unless char (setq char last-command-event))
> + (if (wdired-perm-allowed-in-pos char
> + (- (wdired--current-column) wdired--perm-beg))
> + (let ((new-bit (char-to-string char))
> (inhibit-read-only t)
> - (pos-prop (- (point) (- (current-column) wdired--col-perm))))
> - (put-text-property 0 1 'keymap wdired-perm-mode-map new-bit)
> - (put-text-property 0 1 'read-only t new-bit)
> + (pos-prop (+ (line-beginning-position) wdired--perm-beg)))
> + (set-text-properties 0 1 (text-properties-at (point)) new-bit)
> (insert new-bit)
> (delete-char 1)
> - (put-text-property (1- pos-prop) pos-prop 'perm-changed t)
> - (put-text-property (1- (point)) (point) 'rear-nonsticky '(keymap)))
> + (put-text-property (1- pos-prop) pos-prop 'perm-changed t))
> (forward-char 1)))
>
> (defun wdired-toggle-bit ()
> "Toggle the permission bit at point."
> (interactive)
> - (let ((inhibit-read-only t)
> - (new-bit "-")
> - (pos-prop (- (point) (- (current-column) wdired--col-perm))))
> - (if (eq (char-after (point)) ?-)
> - (setq new-bit
> - (if (= (% (- (current-column) wdired--col-perm) 3) 0) "r"
> - (if (= (% (- (current-column) wdired--col-perm) 3) 1) "w"
> - "x"))))
> - (put-text-property 0 1 'keymap wdired-perm-mode-map new-bit)
> - (put-text-property 0 1 'read-only t new-bit)
> - (insert new-bit)
> - (delete-char 1)
> - (put-text-property (1- pos-prop) pos-prop 'perm-changed t)
> - (put-text-property (1- (point)) (point) 'rear-nonsticky '(keymap))))
> + (wdired-set-bit
> + (cond
> + ((not (eq (char-after (point)) ?-)) ?-)
> + ((= (% (- (wdired--current-column) wdired--perm-beg) 3) 0) ?r)
> + ((= (% (- (wdired--current-column) wdired--perm-beg) 3) 1) ?w)
> + (t ?x))))
>
> (defun wdired-mouse-toggle-bit (event)
> "Toggle the permission bit that was left clicked."
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing - BUG
2021-03-27 18:20 ` [PATCH] Lazy wdired preprocessing - BUG Arthur Miller
@ 2021-03-27 18:32 ` Stefan Monnier
2021-03-27 18:50 ` Arthur Miller
0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2021-03-27 18:32 UTC (permalink / raw)
To: Arthur Miller; +Cc: emacs-devel
> I have just rebuild with the new patch. I see a small bug: when entered
> wdired mode, and trying to do a very first change, it requires two
> keypresses. The first key press, I think, gets consumed in first call to
> wdired--self insert, so user is required to press again a key to do
> actual edit.
Duh, little brain fart, sorry.
Should be fixed now,
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing - BUG
2021-03-27 18:32 ` Stefan Monnier
@ 2021-03-27 18:50 ` Arthur Miller
0 siblings, 0 replies; 24+ messages in thread
From: Arthur Miller @ 2021-03-27 18:50 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> I have just rebuild with the new patch. I see a small bug: when entered
>> wdired mode, and trying to do a very first change, it requires two
>> keypresses. The first key press, I think, gets consumed in first call to
>> wdired--self insert, so user is required to press again a key to do
>> actual edit.
>
> Duh, little brain fart, sorry.
> Should be fixed now,
Confirmed, works perfectly now!
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-26 12:21 ` Arthur Miller
@ 2021-03-27 23:49 ` Michael Heerdegen
2021-03-28 1:51 ` Stefan Monnier
2021-03-28 7:50 ` Sv: " arthur miller
0 siblings, 2 replies; 24+ messages in thread
From: Michael Heerdegen @ 2021-03-27 23:49 UTC (permalink / raw)
To: Arthur Miller; +Cc: emacs-devel
Arthur Miller <arthur.miller@live.com> writes:
> I just tested bit more. Once wdired is entered with hidden details, one
> can not switch deatils on normally since dired-hide-details-mode checks
> if it is in dired-mode.
Yes, that's Bug#45127.
Ok, I tested your code a bit now that it has landed on master, and it
works very well so far.
One different, minor thing that is still a bit annoying with large dired
buffers is that when I abort wdired, the directory is completely read in
anew. I wonder if we could use buffer-undo-list to rewind any user
changes and just `wdired-change-to-dired-mode' instead. Would that work
ok?
TIA,
Michael.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-27 23:49 ` Michael Heerdegen
@ 2021-03-28 1:51 ` Stefan Monnier
2021-03-28 1:56 ` Michael Heerdegen
2021-03-28 7:50 ` Sv: " arthur miller
1 sibling, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2021-03-28 1:51 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: Arthur Miller, emacs-devel
> One different, minor thing that is still a bit annoying with large dired
> buffers is that when I abort wdired, the directory is completely read in
> anew.
That's odd. The way I read the code, if you `wdired-abort` then it just
restores the original contents of the buffer and doesn't look at the
actual files at all.
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-28 1:51 ` Stefan Monnier
@ 2021-03-28 1:56 ` Michael Heerdegen
2021-03-28 2:00 ` Stefan Monnier
0 siblings, 1 reply; 24+ messages in thread
From: Michael Heerdegen @ 2021-03-28 1:56 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Arthur Miller, emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
> That's odd. The way I read the code, if you `wdired-abort` then it just
> restores the original contents of the buffer and doesn't look at the
> actual files at all.
Ok, true. My problem is that this loses window points when the buffer
is displayed in several windows, but that might be a minor problem for
others.
Michael.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Lazy wdired preprocessing
2021-03-28 1:56 ` Michael Heerdegen
@ 2021-03-28 2:00 ` Stefan Monnier
0 siblings, 0 replies; 24+ messages in thread
From: Stefan Monnier @ 2021-03-28 2:00 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: Arthur Miller, emacs-devel
>> That's odd. The way I read the code, if you `wdired-abort` then it just
>> restores the original contents of the buffer and doesn't look at the
>> actual files at all.
> Ok, true. My problem is that this loses window points when the buffer
> is displayed in several windows, but that might be a minor problem for
> others.
That should be easy enough to fix. I'd recommend you `M-x report-emacs-bug`
(and then post a patch to fix it ;-)
E.g. you could use undo to recover the original contents, which should
also recover the original `window-point`s.
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Sv: [PATCH] Lazy wdired preprocessing
2021-03-27 23:49 ` Michael Heerdegen
2021-03-28 1:51 ` Stefan Monnier
@ 2021-03-28 7:50 ` arthur miller
2021-03-28 13:51 ` Stefan Monnier
[not found] ` <87y2e6242i.fsf@web.de>
1 sibling, 2 replies; 24+ messages in thread
From: arthur miller @ 2021-03-28 7:50 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: emacs-devel@gnu.org
[-- Attachment #1: Type: text/plain, Size: 850 bytes --]
>> I just tested bit more. Once wdired is entered with hidden details, one
>> can not switch deatils on normally since dired-hide-details-mode checks
>> if it is in dired-mode.
> Yes, that's Bug#45127.
I didn't know it was a bug. It would be easy to hack that check to check for
dired or wdired mode too. I am not sure how beneficial it would be though.
Honestly I think editing permissions is just a show off :-). I never sed wdired
for that. It is much easier for me to do it chmod when I need it, either from
the minibuffer or command line. Maybe we can just document behaviour in
manual and clode the bug? 🙂
> Ok, I tested your code a bit now that it has landed on master, and it
> works very well so far.
The hard part was Stefan's, I was mostly just complaining and did an
initial sketch, and some "easy parts " :-).
[-- Attachment #2: Type: text/html, Size: 3363 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Sv: [PATCH] Lazy wdired preprocessing
2021-03-28 7:50 ` Sv: " arthur miller
@ 2021-03-28 13:51 ` Stefan Monnier
2021-03-28 16:22 ` Sv: " arthur miller
[not found] ` <87y2e6242i.fsf@web.de>
1 sibling, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2021-03-28 13:51 UTC (permalink / raw)
To: arthur miller; +Cc: Michael Heerdegen, emacs-devel@gnu.org
> The hard part was Stefan's, I was mostly just complaining and did an
> initial sketch, and some "easy parts " :-).
Funny, that's exactly how I'd describe my involvement: "I was mostly just complaining and did an
initial [verbal] sketch, and some "easy parts " :-)."
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Sv: Sv: [PATCH] Lazy wdired preprocessing
2021-03-28 13:51 ` Stefan Monnier
@ 2021-03-28 16:22 ` arthur miller
0 siblings, 0 replies; 24+ messages in thread
From: arthur miller @ 2021-03-28 16:22 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Michael Heerdegen, emacs-devel@gnu.org
[-- Attachment #1: Type: text/plain, Size: 657 bytes --]
No, it is just to realize, RMS law implied 🙂.
________________________________
Från: Stefan Monnier <monnier@iro.umontreal.ca>
Skickat: den 28 mars 2021 15:51
Till: arthur miller <arthur.miller@live.com>
Kopia: Michael Heerdegen <michael_heerdegen@web.de>; emacs-devel@gnu.org <emacs-devel@gnu.org>
Ämne: Re: Sv: [PATCH] Lazy wdired preprocessing
> The hard part was Stefan's, I was mostly just complaining and did an
> initial sketch, and some "easy parts " :-).
Funny, that's exactly how I'd describe my involvement: "I was mostly just complaining and did an
initial [verbal] sketch, and some "easy parts " :-)."
Stefan
[-- Attachment #2: Type: text/html, Size: 1550 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Sv: Sv: [PATCH] Lazy wdired preprocessing
[not found] ` <87y2e6242i.fsf@web.de>
@ 2021-03-29 8:35 ` arthur miller
0 siblings, 0 replies; 24+ messages in thread
From: arthur miller @ 2021-03-29 8:35 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: emacs-devel@gnu.org
[-- Attachment #1: Type: text/plain, Size: 1244 bytes --]
Indeed. Sounds useful to see sizes and timestamps sometimes when editing names.
________________________________
Från: Michael Heerdegen <michael_heerdegen@web.de>
Skickat: den 29 mars 2021 02:31
Till: arthur miller <arthur.miller@live.com>
Kopia: emacs-devel@gnu.org; emacs-devel@gnu.org <emacs-devel@gnu.org>
Ämne: Re: Sv: [PATCH] Lazy wdired preprocessing
arthur miller <arthur.miller@live.com> writes:
> > Yes, that's Bug#45127.
>
> It would be easy to hack that check to check for dired or wdired mode
> too. I am not sure how beneficial it would be though. Honestly I
> think editing permissions is just a show off :-).
It's not about changing the permissions: most of the time I have
`dired-hide-details-mode' initially off (i.e. details shown) in normal
dired because of other useful information the listing shows (times,
number of hardlinks). Then I switch to wdired to change file names and
see that I rather wanted to hide details because some file names are
long and the end is out of view, but I have already made some changes.
Instead of aborting wdired, turn `dired-hide-details-mode' on, switch to
wdired again and restart editing I would rather just toggle details from
within wdired.
Michael.
[-- Attachment #2: Type: text/html, Size: 2116 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-03-29 8:35 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25 16:06 [PATCH] Lazy wdired preprocessing Arthur Miller
2021-03-25 23:09 ` Michael Heerdegen
2021-03-26 1:00 ` Arthur Miller
2021-03-26 3:27 ` Michael Heerdegen
2021-03-26 12:15 ` Arthur Miller
2021-03-26 12:21 ` Arthur Miller
2021-03-27 23:49 ` Michael Heerdegen
2021-03-28 1:51 ` Stefan Monnier
2021-03-28 1:56 ` Michael Heerdegen
2021-03-28 2:00 ` Stefan Monnier
2021-03-28 7:50 ` Sv: " arthur miller
2021-03-28 13:51 ` Stefan Monnier
2021-03-28 16:22 ` Sv: " arthur miller
[not found] ` <87y2e6242i.fsf@web.de>
2021-03-29 8:35 ` arthur miller
2021-03-26 10:18 ` Stefan Kangas
2021-03-26 19:37 ` Stefan Monnier
2021-03-27 7:39 ` Arthur Miller
2021-03-27 14:56 ` Stefan Monnier
2021-03-27 15:17 ` Arthur Miller
2021-03-27 15:56 ` Stefan Monnier
2021-03-27 17:01 ` Arthur Miller
2021-03-27 18:20 ` [PATCH] Lazy wdired preprocessing - BUG Arthur Miller
2021-03-27 18:32 ` Stefan Monnier
2021-03-27 18:50 ` Arthur Miller
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).