unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Partial wdired (edit just filename at the point)
@ 2021-03-16 18:23 Arthur Miller
  2021-03-17  1:18 ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: Arthur Miller @ 2021-03-16 18:23 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]


Wdired is really nice for batch renaming files, but I have noticed that
in case there are lots of files, starting wdired mode and finalizing
changes can take a while. Sometimes I switch to wdired mode just to rename
a single file. On my machine, a directory with ~700 - 800 files causes
a noticable delay. I haven't measured but it is in range of 2-3
seconds I would guess. I had one directory with almost 2000 files and
when tested it took quite a bit of time.

I traced it down to how dired/wdired use text properties to controll if
text is writable or not. When switching to, wdired goes three times
through entire buffer (filenames, perms and symlinks) and changes text
properties to writable text.

I have rewritten those few functions to drop to wdired mode to only work
on a line under the point; it is just quick copy/paste hack. It
works fine for my needs and wdired starts without noticable delay, but
there is one cosmetic detail: despite me changing properties only for
current line, wdired still let me edit other text in dired buffer
itself.

When saving changes, it properly saves only the intended line. I don't
seem to be able to find which property affect that behaviour, wonder if
someone can help in that regard?

Another question: is this interesting to add to wdired? It makes Emacs
behave a bit more like standard file managers; they usually bound F2
(for example Dolphing) to file-rename operation.

Error checking is needed to handle cases when point is not in a line
with a filename.


[-- Attachment #2: partial-wdired.el --]
[-- Type: text/plain, Size: 5412 bytes --]

(defun wdired-change-to-partial-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 (line-beginning-position) (line-end-position)))
  (setq-local wdired-old-marks
              (dired-remember-marks (line-beginning-position) (line-end-position)))
  (setq-local wdired-old-point (point))
  (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 'after-change-functions 'wdired--restore-properties nil t)
  (setq major-mode 'wdired-mode)
  (setq mode-name "Partially Editable Dired")
  (setq 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-partial-preprocess-files)
  (if wdired-allow-to-change-permissions
      (wdired-partial-preprocess-perms))
  (if (fboundp 'make-symbolic-link)
      (wdired-partial-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)
  (message "%s" (substitute-command-keys
		 "Press \\[wdired-finish-edit] when finished \
or \\[wdired-abort-changes] to abort changes")))

(defun wdired-abort-changes ()
  "Abort changes and return to dired mode."
  (interactive)
  (let ((inhibit-read-only t))
    (if (equal mode-name "Partially Editable Dired")
        (delete-region (line-beginning-position) (line-end-position))
      (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"))

;; Protect the buffer so only the filename can be changed, and put
;; properties so filename (old and new) can be easily found.
(defun wdired-partial-preprocess-files ()
  (put-text-property (line-beginning-position) (1+ (line-beginning-position))'front-sticky t)
  (save-excursion
    (goto-char (line-beginning-position))
    (let ((b-protection (point))
          (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 b-protection (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))
      (put-text-property b-protection (line-end-position) 'read-only t))))

(defun wdired-partial-preprocess-perms ()
  (let ((inhibit-read-only t))
    (setq-local wdired-col-perm nil)
    (save-excursion
      (goto-char (line-beginning-position))
      (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-partial-preprocess-symlinks ()
  (let ((inhibit-read-only t))
    (save-excursion
      (goto-char (line-beginning-position))
      (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))))))

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-16 18:23 Partial wdired (edit just filename at the point) Arthur Miller
@ 2021-03-17  1:18 ` Stefan Monnier
  2021-03-17  2:21   ` Arthur Miller
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2021-03-17  1:18 UTC (permalink / raw)
  To: Arthur Miller; +Cc: emacs-devel

> Wdired is really nice for batch renaming files, but I have noticed that
> in case there are lots of files, starting wdired mode and finalizing
> changes can take a while. Sometimes I switch to wdired mode just to rename
> a single file. On my machine, a directory with ~700 - 800 files causes
> a noticable delay. I haven't measured but it is in range of 2-3
> seconds I would guess. I had one directory with almost 2000 files and
> when tested it took quite a bit of time.

It seems to scale linearly, so it will indeed get slower with very large
directories, tho it should stay manageable (i.e. if it took 2-3s for
700 files, it should still take less than 6-9s for 2000 files).

But yes, it's a problem.

> I traced it down to how dired/wdired use text properties to controll if
> text is writable or not. When switching to, wdired goes three times
> through entire buffer (filenames, perms and symlinks) and changes text
> properties to writable text.

The profiler suggests that most of the time is spent in
`wdired-preprocess-files` which does one of those traversals.

> I have rewritten those few functions to drop to wdired mode to only work
> on a line under the point; it is just quick copy/paste hack.  It
> works fine for my needs and wdired starts without noticable delay, but
> there is one cosmetic detail: despite me changing properties only for
> current line, wdired still let me edit other text in dired buffer
> itself.

I'd rather try and speed up the general code.  How 'bout making
`wdired-preprocess-files` work one line at a time and do it lazily,
using a `before-change-function`?

This way entering `wdired` should be instantaneous but you can still
edit any line you like.  The actual edits would be slightly slowed down,
but that shouldn't be nearly as problematic.


        Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-17  1:18 ` Stefan Monnier
@ 2021-03-17  2:21   ` Arthur Miller
  2021-03-17  2:34     ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: Arthur Miller @ 2021-03-17  2:21 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Wdired is really nice for batch renaming files, but I have noticed that
>> in case there are lots of files, starting wdired mode and finalizing
>> changes can take a while. Sometimes I switch to wdired mode just to rename
>> a single file. On my machine, a directory with ~700 - 800 files causes
>> a noticable delay. I haven't measured but it is in range of 2-3
>> seconds I would guess. I had one directory with almost 2000 files and
>> when tested it took quite a bit of time.
>
> It seems to scale linearly, so it will indeed get slower with very large
> directories, tho it should stay manageable (i.e. if it took 2-3s for
> 700 files, it should still take less than 6-9s for 2000 files).

Something like that indeed. I haven't done measurements. Not that I
practice to have thousands of files in every directory, but those small
delays are still noticable and slightly annoying.

> But yes, it's a problem.

Even if there is just like few tens of files, it saves battery on
laptop if unnecessary processing is ommited :).

>> I traced it down to how dired/wdired use text properties to controll if
>> text is writable or not. When switching to, wdired goes three times
>> through entire buffer (filenames, perms and symlinks) and changes text
>> properties to writable text.
>
> The profiler suggests that most of the time is spent in
> `wdired-preprocess-files` which does one of those traversals.

Probably; file names are most of a dired buffer; usually there are not
so many symlinks, and permissions are not so much of text.

>> I have rewritten those few functions to drop to wdired mode to only work
>> on a line under the point; it is just quick copy/paste hack.  It
>> works fine for my needs and wdired starts without noticable delay, but
>> there is one cosmetic detail: despite me changing properties only for
>> current line, wdired still let me edit other text in dired buffer
>> itself.
>
> I'd rather try and speed up the general code.  How 'bout making
> `wdired-preprocess-files` work one line at a time and do it lazily,
> using a `before-change-function`?

Never used 'before-change-function, so I didn't think about it :).

Sure, as a strategy it sounds good indeed, but I am not sure I
understand how to apply that hook in this case. Docs says it is called
before any buffer mdification, but I am not sure what it means: before
every key-press in this case or what is ment with buffer modification here?

> This way entering `wdired` should be instantaneous but you can still
> edit any line you like.  The actual edits would be slightly slowed down,
> but that shouldn't be nearly as problematic.

I agree it shouldn't be noticable; it is how the implementation I
attached works; it is just that I do it explicitly with a "manual"
call to function that change properties just for the line at the point.

Thanks for looking at it.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-17  2:21   ` Arthur Miller
@ 2021-03-17  2:34     ` Stefan Monnier
  2021-03-17 13:58       ` Arthur Miller
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2021-03-17  2:34 UTC (permalink / raw)
  To: Arthur Miller; +Cc: emacs-devel

> Never used 'before-change-function, so I didn't think about it :).
>
> Sure, as a strategy it sounds good indeed, but I am not sure I
> understand how to apply that hook in this case. Docs says it is called
> before any buffer mdification, but I am not sure what it means: before
> every key-press in this case or what is ment with buffer modification here?

Not "before a key press" because it can't know you're about to press
a key, and even if it knows, it can't easily know what that command
will do.

So instead it's done at the beginning of those ELisp primitives that
modify a buffer, like `delete-region`, or `insert`.

It can be called several times in a single command, depending on what
the command does, so it's best to optimize function placed on this hook
such that they are usually very cheap.

>> This way entering `wdired` should be instantaneous but you can still
>> edit any line you like.  The actual edits would be slightly slowed down,
>> but that shouldn't be nearly as problematic.
>
> I agree it shouldn't be noticable; it is how the implementation I
> attached works; it is just that I do it explicitly with a "manual"
> call to function that change properties just for the line at the point.

Yes, I understand, but I think we can have our cake and eat it too: be
super-fast when editing only a single line yet without having to tell
Emacs in advance whether we plan to edit only a single line or all
the lines.


        Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-17  2:34     ` Stefan Monnier
@ 2021-03-17 13:58       ` Arthur Miller
  2021-03-17 14:09         ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: Arthur Miller @ 2021-03-17 13:58 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Never used 'before-change-function, so I didn't think about it :).
>>
>> Sure, as a strategy it sounds good indeed, but I am not sure I
>> understand how to apply that hook in this case. Docs says it is called
>> before any buffer mdification, but I am not sure what it means: before
>> every key-press in this case or what is ment with buffer modification here?
>
> Not "before a key press" because it can't know you're about to press
> a key, and even if it knows, it can't easily know what that command
> will do.

Well of course not; Emacs can't know when I am going to press a key, :-),
that is not what I ment.

I ment after a key is pressed Emacs will call it before the actuall
character is inserted into buffer. What I want to know is if it is going
to be called in context of every keypress, i.e. every insert, and from
your following statement it seems it will be.

The problem here is how aborting changes is implemented in wdired: undo
is disabled and original code just copies entire buffer and pastes it back
when changes are aborted. I do same strategy but just for the line in
question. Question is how to do this when multiple lines are edited. I
would like to skip copying entire buffer into buffer-string as wdired
does originally. I'll see if enabling undo stack can help; I think the
problem will be to keep away from editing certain part of lines
(timestamps and alike).

> Yes, I understand, but I think we can have our cake and eat it too: be
> super-fast when editing only a single line yet without having to tell
> Emacs in advance whether we plan to edit only a single line or all
> the lines.

I agree with you, but I am not sure how to implement it. My hack was
literally less than a 5 minute change, I just removed loops and changed
mode name so I can abort it properly.

It would be actually very trivial if wdired didn't manipulated text
properties for entire buffer; i.e. if we used only read-only property
for entire buffer, i.e. a "naive approach" where entire buffer is
guarded with just one variable. I understand original author choosed to
go via text properties for a good reason, I am not sure which one
though: just extra safety in case of accidental change, or is there some
other?







^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-17 13:58       ` Arthur Miller
@ 2021-03-17 14:09         ` Stefan Monnier
  2021-03-17 19:56           ` Arthur Miller
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2021-03-17 14:09 UTC (permalink / raw)
  To: Arthur Miller; +Cc: emacs-devel

> The problem here is how aborting changes is implemented in wdired: undo
> is disabled and original code just copies entire buffer and pastes it back
> when changes are aborted.

I think you should be able to leave this part of the code completely unchanged.

> I would like to skip copying entire buffer into buffer-string as
> wdired does originally.

Any particular reason for that?  It should be very fast, even for very
large directories.

> I agree with you, but I am not sure how to implement it. My hack was
> literally less than a 5 minute change, I just removed loops and changed
> mode name so I can abort it properly.

I'd start with the following:
When converting to wdired, instead of calling `wdired-preprocess-files`, use

    (add-hook 'before-change-functions #'wdired--preprocess-lines nil t)

and then turn `wdired-preprocess-files` into `wdired--preprocess-lines`,
which will `get-text-property` of the first char of each line in the
region to see if it's already been marked as `read-only`.  If yes,
do nothing and if not, do what the old code did on that line.


        Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-17 14:09         ` Stefan Monnier
@ 2021-03-17 19:56           ` Arthur Miller
  2021-03-17 22:40             ` Arthur Miller
  0 siblings, 1 reply; 33+ messages in thread
From: Arthur Miller @ 2021-03-17 19:56 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The problem here is how aborting changes is implemented in wdired: undo
>> is disabled and original code just copies entire buffer and pastes it back
>> when changes are aborted.
>
> I think you should be able to leave this part of the code completely unchanged.

Yes; and for the simplicity it will be so.

>> I would like to skip copying entire buffer into buffer-string as
>> wdired does originally.
>
> Any particular reason for that?  It should be very fast, even for very
> large directories.

Just for efficiency; it copies entire buffer which can be quite big
memory wise; but indeed it seems to be very fast, and I guess for the
simplicity of implementation it can be left as is :-).

>> I agree with you, but I am not sure how to implement it. My hack was
>> literally less than a 5 minute change, I just removed loops and changed
>> mode name so I can abort it properly.
>
> I'd start with the following:
> When converting to wdired, instead of calling `wdired-preprocess-files`, use
>
>     (add-hook 'before-change-functions #'wdired--preprocess-lines nil t)
>
> and then turn `wdired-preprocess-files` into `wdired--preprocess-lines`,
> which will `get-text-property` of the first char of each line in the
> region to see if it's already been marked as `read-only`.  If yes,
> do nothing and if not, do what the old code did on that line.

I tested, and currently I don't see any noticable slowdowns, even on
that large directory. Dropping into wdired seems to be quite reactive
and I can start editing any file name immidiately.

However I can't seem to be able to get it work with permissions; I am
not sure why. I have checked that wdired-allow-to-change-permissions is t.

I have attached code as a separate file (I worked so). I can make a
patch for wdired.el later, if you or someone can give me a tip why text
props for permissions are not changing as they should.


[-- Attachment #2: partial-wdired.el --]
[-- Type: text/plain, Size: 6420 bytes --]

;;; partial-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)

;;;###autoload
(defun wdired-change-to-partial-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 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--preprocess-line 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)
  (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--preprocess-line (beg end)
  "Preprocess current line under point to make it writable.  "
  (let ((inhibit-read-only t))
  (unless (get-text-property (line-beginning-position) 'front-sticky)
    (buffer-disable-undo)
    (put-text-property (line-beginning-position) (1+
                                                  (line-beginning-position))
                       'front-sticky t)
      (save-excursion
        (goto-char (line-beginning-position))
          (let ((b-protection (point))
                (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 b-protection (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))
            (put-text-property b-protection (line-end-position) 'read-only t))

          (when wdired-allow-to-change-permissions
            (goto-char (line-beginning-position))
            (setq-local wdired-col-perm nil)
            (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 (fboundp 'make-symbolic-link)
            (goto-char (line-beginning-position))
            (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))))))
      (buffer-enable-undo)))

  (defun wdired-abort-changes ()
    "Abort changes and return to dired mode."
    (interactive)
    (remove-hook 'before-change-functions 'wdired--preprocess-line t)
    (let ((inhibit-read-only t))
      (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"))

  (provide 'partial-wdired)

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-17 19:56           ` Arthur Miller
@ 2021-03-17 22:40             ` Arthur Miller
  2021-03-18  2:10               ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: Arthur Miller @ 2021-03-17 22:40 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 2327 bytes --]

Arthur Miller <arthur.miller@live.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> The problem here is how aborting changes is implemented in wdired: undo
>>> is disabled and original code just copies entire buffer and pastes it back
>>> when changes are aborted.
>>
>> I think you should be able to leave this part of the code completely unchanged.
>
> Yes; and for the simplicity it will be so.
>
>>> I would like to skip copying entire buffer into buffer-string as
>>> wdired does originally.
>>
>> Any particular reason for that?  It should be very fast, even for very
>> large directories.
>
> Just for efficiency; it copies entire buffer which can be quite big
> memory wise; but indeed it seems to be very fast, and I guess for the
> simplicity of implementation it can be left as is :-).
>
>>> I agree with you, but I am not sure how to implement it. My hack was
>>> literally less than a 5 minute change, I just removed loops and changed
>>> mode name so I can abort it properly.
>>
>> I'd start with the following:
>> When converting to wdired, instead of calling `wdired-preprocess-files`, use
>>
>>     (add-hook 'before-change-functions #'wdired--preprocess-lines nil t)
>>
>> and then turn `wdired-preprocess-files` into `wdired--preprocess-lines`,
>> which will `get-text-property` of the first char of each line in the
>> region to see if it's already been marked as `read-only`.  If yes,
>> do nothing and if not, do what the old code did on that line.
>
> I tested, and currently I don't see any noticable slowdowns, even on
> that large directory. Dropping into wdired seems to be quite reactive
> and I can start editing any file name immidiately.
>
> However I can't seem to be able to get it work with permissions; I am
> not sure why. I have checked that wdired-allow-to-change-permissions is t.
>
> I have attached code as a separate file (I worked so). I can make a
> patch for wdired.el later, if you or someone can give me a tip why text
> props for permissions are not changing as they should.

I change my mind :-) I have tested with emacs -Q option, and I see same
behaviour with permission properties with original wdired code, so I
guess it works properly (or as buggy as original). Someone please test
it though.

Attached is a patch for wdired based on current master.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lazy-wdired.patch --]
[-- Type: text/x-patch, Size: 9518 bytes --]

From cddace1dae6cb9e4367f1fad3d7bd3745282824d Mon Sep 17 00:00:00 2001
From: Arthur Miller <arthur.miller@live.com>
Date: Wed, 17 Mar 2021 23:24:10 +0100
Subject: [PATCH] Make wdired lazy

---
 lisp/wdired.el | 156 +++++++++++++++++++++++--------------------------
 1 file changed, 74 insertions(+), 82 deletions(-)

diff --git a/lisp/wdired.el b/lisp/wdired.el
index c495d8de34..ba71306e66 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -250,20 +250,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--preprocess-line 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)
-  ;; 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)
@@ -278,25 +269,33 @@ 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)
-  (save-excursion
-    (goto-char (point-min))
-    (let ((b-protection (point))
-          (used-F (dired-check-switches dired-actual-switches "F" "classify"))
-	  filename)
-      (while (not (eobp))
-	(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 b-protection (point) 'read-only t)
-          (dired-move-to-end-of-filename t)
-	  (put-text-property (point) (1+ (point)) 'end-name t))
+(defun wdired--preprocess-line (beg end)
+  "Preprocess current line under point to make it writable.  "
+  (let ((inhibit-read-only t))
+    (unless (get-text-property (line-beginning-position) 'front-sticky)
+      (buffer-disable-undo)
+      (put-text-property (line-beginning-position) (1+
+                                                    (line-beginning-position))
+                         'front-sticky t)
+      (save-excursion
+        (goto-char (line-beginning-position))
+        (let ((b-protection (point))
+              (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 b-protection (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
@@ -304,9 +303,51 @@ wdired-preprocess-files
                        (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))))
+          (setq b-protection (point))
+          (put-text-property b-protection (line-end-position)
+                             'read-only t))
+
+        ;; Put a keymap property to the permission bits of the files,
+        ;; and store the original name and permissions as a property
+        (when wdired-allow-to-change-permissions
+          (goto-char (line-beginning-position))
+          (setq-local wdired-col-perm nil)
+          (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)))))
+
+        ;; Put the needed properties to allow the user to change
+        ;; links' targets
+        (when (fboundp 'make-symbolic-link)
+          (goto-char (line-beginning-position))
+          (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))))))
+    (buffer-enable-undo)))
 
 ;; This code is a copy of some dired-get-filename lines.
 (defsubst wdired-normalize-filename (file unquotep)
@@ -369,7 +410,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)
@@ -390,10 +430,10 @@ wdired-change-to-dired-mode
   (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--preprocess-line t)
   (let ((inhibit-read-only t))
     (erase-buffer)
     (insert wdired-old-content)
@@ -709,23 +749,6 @@ wdired-previous-line
 				  (current-column)))))
       (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-get-previous-link (&optional old move)
   "Return the next symlink target.
 If OLD, return the old target.  If MOVE, move point before it."
@@ -828,37 +851,6 @@ wdired-perm-mode-map
     (define-key map [down-mouse-1] 'wdired-mouse-toggle-bit)
     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-perm-allowed-in-pos (char pos)
   (cond
    ((= char ?-)          t)
-- 
2.31.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-17 22:40             ` Arthur Miller
@ 2021-03-18  2:10               ` Stefan Monnier
  2021-03-18 10:26                 ` Arthur Miller
  2021-03-21 22:17                 ` Tomas Hlavaty
  0 siblings, 2 replies; 33+ messages in thread
From: Stefan Monnier @ 2021-03-18  2:10 UTC (permalink / raw)
  To: Arthur Miller; +Cc: emacs-devel

Hi,

These changes are looking quite good.  There are a few things to fix
before we can install it, tho:

> diff --git a/lisp/wdired.el b/lisp/wdired.el
> index c495d8de34..ba71306e66 100644
> --- a/lisp/wdired.el
> +++ b/lisp/wdired.el
> @@ -250,20 +250,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--preprocess-line nil t)
>    (add-hook 'after-change-functions 'wdired--restore-properties nil t)

[ Nitpick:
  Please use #' rather than ' to quote functions (I know the rest of the
  code here doesn't (yet), but we should move towards more systematic
  use of #' so as to better catch obsolete functions and other issues).  ]

> +(defun wdired--preprocess-line (beg end)
> +  "Preprocess current line under point to make it writable.  "

This is incorrect: what it *should* do is to preprocess an area that
covers BEG..END.  That's why I suggested to name it "...lines" rather
than "...line" ;-)
And the "current point" is not really relevant (it will *usually* be in
the vicinity, but not always).

> +  (let ((inhibit-read-only t))
> +    (unless (get-text-property (line-beginning-position) 'front-sticky)
> +      (buffer-disable-undo)

As you can guess from the previous comment, we should loop over BEG...END
to process all the lines involved.

Also, I recommend you use `with-silent-modifications` which will cover
both `inhibit-read-only` and the undo (and a few more potential
problems, which admittedly should hopefully not matter here).
[ That macro didn't exist back when wdired.el was written.  ]

> @@ -304,9 +303,51 @@ wdired-preprocess-files
>                         (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))))
> +          (setq b-protection (point))
> +          (put-text-property b-protection (line-end-position)
> +                             'read-only t))
> +
> +        ;; Put a keymap property to the permission bits of the files,
> +        ;; and store the original name and permissions as a property
> +        (when wdired-allow-to-change-permissions
> +          (goto-char (line-beginning-position))
> +          (setq-local wdired-col-perm nil)
> +          (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)))))
> +
> +        ;; Put the needed properties to allow the user to change
> +        ;; links' targets
> +        (when (fboundp 'make-symbolic-link)
> +          (goto-char (line-beginning-position))
> +          (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))))))
> +    (buffer-enable-undo)))

To minimize code changes (and to avoid making such a large function
(which are sadly frequent in Emacs's code base)), I think you can keep
the `wdired-preprocess-perms` and `wdired-preprocess-symlinks` (tho
you'll want to change them by making them take the beg..end arguments.)
and call them from here instead of moving their code into this new
function.


        Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-18  2:10               ` Stefan Monnier
@ 2021-03-18 10:26                 ` Arthur Miller
  2021-03-18 10:32                   ` Thierry Volpiatto
  2021-03-18 14:21                   ` Stefan Monnier
  2021-03-21 22:17                 ` Tomas Hlavaty
  1 sibling, 2 replies; 33+ messages in thread
From: Arthur Miller @ 2021-03-18 10:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Hi,
>
> These changes are looking quite good.  There are a few things to fix
> before we can install it, tho:
>
>> diff --git a/lisp/wdired.el b/lisp/wdired.el
>> index c495d8de34..ba71306e66 100644
>> --- a/lisp/wdired.el
>> +++ b/lisp/wdired.el
>> @@ -250,20 +250,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--preprocess-line nil t)
>>    (add-hook 'after-change-functions 'wdired--restore-properties nil t)
>
> [ Nitpick:
>   Please use #' rather than ' to quote functions (I know the rest of the
>   code here doesn't (yet), but we should move towards more systematic
>   use of #' so as to better catch obsolete functions and other issues).  ]

I skipped it I don't like to look at it in either CL nor Elisp; in general I
don't like much of special characters in syntax. I think it is pitty it
is introduced in Elisp. But if you want to have it, it is just to add
it, it's not something essential to argue about; I am just giving reason
why I didn't used it from the beginning :-).

>> +(defun wdired--preprocess-line (beg end)
>> +  "Preprocess current line under point to make it writable.  "
>
> This is incorrect: what it *should* do is to preprocess an area that
> covers BEG..END.  That's why I suggested to name it "...lines" rather
> than "...line" ;-)
> And the "current point" is not really relevant (it will *usually* be in
> the vicinity, but not always).
>
>> +  (let ((inhibit-read-only t))
>> +    (unless (get-text-property (line-beginning-position) 'front-sticky)
>> +      (buffer-disable-undo)
>
> As you can guess from the previous comment, we should loop over BEG...END
> to process all the lines involved.

Can you elaborate more on this please? Why is it wrong and why do you
want it to work with region? As I see it used in Dired, most of the time
beg and end points will be the same, which will effectively result in
lots of processing to change properties for one character. Usually, in
Dired buffer I would move to a line with name and change few characters
only.

Current code changes properties for entire line, if it worked on region
it would change only properties for one char at time. I think it turns
into very complicated processing for just changing one char properties.

I checked to do some editing and printed out region when normally
editing a file name:
(defun wdired--preprocess-line (beg end)
  "Preprocess current line under point to make it writable.  "
  (let ((inhibit-read-only t))
    (message "REG: %s %s" beg end)
    ( ... )

This is what I got for each char I typed in:

REG: 8397 8397
REG: 8398 8398
REG: 8399 8399
REG: 8400 8400
REG: 8401 8401
REG: 8402 8402
REG: 8402 8403
REG: 8401 8402
REG: 8400 8401
REG: 8399 8400
REG: 8398 8399
REG: 8397 8398

The way I did it, it works per line; it will process entire line at
time. I have also checked to change multiple file names with rectangle
commands and in it works well too. I guess due to how Emacs internally
anserts chars into buffer (sequentially).

> Also, I recommend you use `with-silent-modifications` which will cover
> both `inhibit-read-only` and the undo (and a few more potential
> problems, which admittedly should hopefully not matter here).
> [ That macro didn't exist back when wdired.el was written.  ]

Thanks; I wasn't aware of that macro either.

>> @@ -304,9 +303,51 @@ wdired-preprocess-files
>>                         (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))))
>> +          (setq b-protection (point))
>> +          (put-text-property b-protection (line-end-position)
>> +                             'read-only t))
>> +
>> +        ;; Put a keymap property to the permission bits of the files,
>> +        ;; and store the original name and permissions as a property
>> +        (when wdired-allow-to-change-permissions
>> +          (goto-char (line-beginning-position))
>> +          (setq-local wdired-col-perm nil)
>> +          (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)))))
>> +
>> +        ;; Put the needed properties to allow the user to change
>> +        ;; links' targets
>> +        (when (fboundp 'make-symbolic-link)
>> +          (goto-char (line-beginning-position))
>> +          (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))))))
>> +    (buffer-enable-undo)))
>
> To minimize code changes (and to avoid making such a large function
> (which are sadly frequent in Emacs's code base)), I think you can keep
> the `wdired-preprocess-perms` and `wdired-preprocess-symlinks` (tho
> you'll want to change them by making them take the beg..end arguments.)
> and call them from here instead of moving their code into this new
> function.

There are much bigger monstrousites I have seen in Emacs lisp folder
than this :-).

I agree with you that we should avoid large functions, I had that in
mind when I refactored out those two methods, but I didn't thought this
was so big (~60 lines without comments). I merged them into one to fit
everything relevant into one spot and one page on the screen. I can
change it back to three smaller defuns, but I will still though prefer
to have them following each other in the code rather than scattered
around in wdired.el for no reason as it is now. They are not refered
from any other code.

Anyway, sharps and refactoring are non-important issue; I can change it
back, just please check more on that with region, before I do changes.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-18 10:26                 ` Arthur Miller
@ 2021-03-18 10:32                   ` Thierry Volpiatto
  2021-03-18 11:00                     ` Arthur Miller
  2021-03-23 23:32                     ` Michael Heerdegen
  2021-03-18 14:21                   ` Stefan Monnier
  1 sibling, 2 replies; 33+ messages in thread
From: Thierry Volpiatto @ 2021-03-18 10:32 UTC (permalink / raw)
  To: Arthur Miller; +Cc: Stefan Monnier, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 8090 bytes --]


Hello, just taking this thread on the fly.

Normally one can pass a list of files with absolute path to dired to get
a dired buffer with only the needed file names to possibly edit with
wdired, however this is broken in Emacs since ages; Helm is fixing this
by advice, see
https://github.com/emacs-helm/helm/blob/master/helm-lib.el#L186.
This allow selecting files from helm-find-files and switching to a dired
buffer with only those files.

HTH.


Arthur Miller <arthur.miller@live.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> Hi,
>>
>> These changes are looking quite good.  There are a few things to fix
>> before we can install it, tho:
>>
>>> diff --git a/lisp/wdired.el b/lisp/wdired.el
>>> index c495d8de34..ba71306e66 100644
>>> --- a/lisp/wdired.el
>>> +++ b/lisp/wdired.el
>>> @@ -250,20 +250,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--preprocess-line nil t)
>>>    (add-hook 'after-change-functions 'wdired--restore-properties nil t)
>>
>> [ Nitpick:
>>   Please use #' rather than ' to quote functions (I know the rest of the
>>   code here doesn't (yet), but we should move towards more systematic
>>   use of #' so as to better catch obsolete functions and other issues).  ]
>
> I skipped it I don't like to look at it in either CL nor Elisp; in general I
> don't like much of special characters in syntax. I think it is pitty it
> is introduced in Elisp. But if you want to have it, it is just to add
> it, it's not something essential to argue about; I am just giving reason
> why I didn't used it from the beginning :-).
>
>>> +(defun wdired--preprocess-line (beg end)
>>> +  "Preprocess current line under point to make it writable.  "
>>
>> This is incorrect: what it *should* do is to preprocess an area that
>> covers BEG..END.  That's why I suggested to name it "...lines" rather
>> than "...line" ;-)
>> And the "current point" is not really relevant (it will *usually* be in
>> the vicinity, but not always).
>>
>>> +  (let ((inhibit-read-only t))
>>> +    (unless (get-text-property (line-beginning-position) 'front-sticky)
>>> +      (buffer-disable-undo)
>>
>> As you can guess from the previous comment, we should loop over BEG...END
>> to process all the lines involved.
>
> Can you elaborate more on this please? Why is it wrong and why do you
> want it to work with region? As I see it used in Dired, most of the time
> beg and end points will be the same, which will effectively result in
> lots of processing to change properties for one character. Usually, in
> Dired buffer I would move to a line with name and change few characters
> only.
>
> Current code changes properties for entire line, if it worked on region
> it would change only properties for one char at time. I think it turns
> into very complicated processing for just changing one char properties.
>
> I checked to do some editing and printed out region when normally
> editing a file name:
> (defun wdired--preprocess-line (beg end)
>   "Preprocess current line under point to make it writable.  "
>   (let ((inhibit-read-only t))
>     (message "REG: %s %s" beg end)
>     ( ... )
>
> This is what I got for each char I typed in:
>
> REG: 8397 8397
> REG: 8398 8398
> REG: 8399 8399
> REG: 8400 8400
> REG: 8401 8401
> REG: 8402 8402
> REG: 8402 8403
> REG: 8401 8402
> REG: 8400 8401
> REG: 8399 8400
> REG: 8398 8399
> REG: 8397 8398
>
> The way I did it, it works per line; it will process entire line at
> time. I have also checked to change multiple file names with rectangle
> commands and in it works well too. I guess due to how Emacs internally
> anserts chars into buffer (sequentially).
>
>> Also, I recommend you use `with-silent-modifications` which will cover
>> both `inhibit-read-only` and the undo (and a few more potential
>> problems, which admittedly should hopefully not matter here).
>> [ That macro didn't exist back when wdired.el was written.  ]
>
> Thanks; I wasn't aware of that macro either.
>
>>> @@ -304,9 +303,51 @@ wdired-preprocess-files
>>>                         (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))))
>>> +          (setq b-protection (point))
>>> +          (put-text-property b-protection (line-end-position)
>>> +                             'read-only t))
>>> +
>>> +        ;; Put a keymap property to the permission bits of the files,
>>> +        ;; and store the original name and permissions as a property
>>> +        (when wdired-allow-to-change-permissions
>>> +          (goto-char (line-beginning-position))
>>> +          (setq-local wdired-col-perm nil)
>>> +          (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)))))
>>> +
>>> +        ;; Put the needed properties to allow the user to change
>>> +        ;; links' targets
>>> +        (when (fboundp 'make-symbolic-link)
>>> +          (goto-char (line-beginning-position))
>>> +          (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))))))
>>> +    (buffer-enable-undo)))
>>
>> To minimize code changes (and to avoid making such a large function
>> (which are sadly frequent in Emacs's code base)), I think you can keep
>> the `wdired-preprocess-perms` and `wdired-preprocess-symlinks` (tho
>> you'll want to change them by making them take the beg..end arguments.)
>> and call them from here instead of moving their code into this new
>> function.
>
> There are much bigger monstrousites I have seen in Emacs lisp folder
> than this :-).
>
> I agree with you that we should avoid large functions, I had that in
> mind when I refactored out those two methods, but I didn't thought this
> was so big (~60 lines without comments). I merged them into one to fit
> everything relevant into one spot and one page on the screen. I can
> change it back to three smaller defuns, but I will still though prefer
> to have them following each other in the code rather than scattered
> around in wdired.el for no reason as it is now. They are not refered
> from any other code.
>
> Anyway, sharps and refactoring are non-important issue; I can change it
> back, just please check more on that with region, before I do changes.


-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-18 10:32                   ` Thierry Volpiatto
@ 2021-03-18 11:00                     ` Arthur Miller
  2021-03-18 11:11                       ` Thierry Volpiatto
  2021-03-23 23:32                     ` Michael Heerdegen
  1 sibling, 1 reply; 33+ messages in thread
From: Arthur Miller @ 2021-03-18 11:00 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

Thierry Volpiatto <thievol@posteo.net> writes:

> Hello, just taking this thread on the fly.
>

Hi Thierry! Thanks for the Helm, I can't really imagine my Emacs without
Helm!

> Normally one can pass a list of files with absolute path to dired to get
> a dired buffer with only the needed file names to possibly edit with
> wdired, however this is broken in Emacs since ages; Helm is fixing this
> by advice, see

I didn't know about this feature of Dired to be honest.

> https://github.com/emacs-helm/helm/blob/master/helm-lib.el#L186.
> This allow selecting files from helm-find-files and switching to a dired
> buffer with only those files.

I do use Helm when I wish to delete some files or move them around
etc; I am not always dropping into Dired. But sometimes it is more
convenient to do more "direct editing" and file management operations as
in a conventional file manager. In those cases I find dired + wdired
very nice. I can't remember when I last opened Dolphin (or some other fm).



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-18 11:00                     ` Arthur Miller
@ 2021-03-18 11:11                       ` Thierry Volpiatto
  2021-03-18 11:46                         ` Arthur Miller
  0 siblings, 1 reply; 33+ messages in thread
From: Thierry Volpiatto @ 2021-03-18 11:11 UTC (permalink / raw)
  To: Arthur Miller; +Cc: Stefan Monnier, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]


Arthur Miller <arthur.miller@live.com> writes:

> I do use Helm when I wish to delete some files or move them around
> etc; I am not always dropping into Dired. But sometimes it is more
> convenient to do more "direct editing" and file management operations as
> in a conventional file manager. In those cases I find dired + wdired
> very nice.

Exactly, it is what Helm does, switching to dired+wdired but only with
needed files (in your case no need to have 2000 files in the dired
buffer when you want to edit only one or two files).

> I can't remember when I last opened Dolphin (or some other fm).

Same here ;-).

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-18 11:11                       ` Thierry Volpiatto
@ 2021-03-18 11:46                         ` Arthur Miller
  0 siblings, 0 replies; 33+ messages in thread
From: Arthur Miller @ 2021-03-18 11:46 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

Thierry Volpiatto <thievol@posteo.net> writes:

> Arthur Miller <arthur.miller@live.com> writes:
>
>> I do use Helm when I wish to delete some files or move them around
>> etc; I am not always dropping into Dired. But sometimes it is more
>> convenient to do more "direct editing" and file management operations as
>> in a conventional file manager. In those cases I find dired + wdired
>> very nice.
>
> Exactly, it is what Helm does, switching to dired+wdired but only with
> needed files (in your case no need to have 2000 files in the dired
> buffer when you want to edit only one or two files).

Definitely something I'll have to get into my habits! :-)

Thanks!



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-18 10:26                 ` Arthur Miller
  2021-03-18 10:32                   ` Thierry Volpiatto
@ 2021-03-18 14:21                   ` Stefan Monnier
  2021-03-19 11:15                     ` Arthur Miller
  1 sibling, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2021-03-18 14:21 UTC (permalink / raw)
  To: Arthur Miller; +Cc: emacs-devel

>> As you can guess from the previous comment, we should loop over BEG...END
>> to process all the lines involved.
> Can you elaborate more on this please?

Because that's how `before-change-functions` is defined.  We can try and
argue that in the specific case of wdired buffer the multiline case will
never happen (presumably because of all the `read-only` annotations) or
that it will always be "on the same line as point", but I'm pretty sure
we'll end up finding corner cases where these are not true, so I find it
easier to slap a simple while loop around the code and stop worrying
about corner cases.

[ As a general rule, I find it easier to write code if I just rely on
  the few things which I know to be true, rather than having to think of
  a set of unbounded possible cases.  Here, what we know to be true is
  that the upcoming changes will not affect anything outside BEG...END
  (at least not until `before-change-functions` are called again).  ]

> This is what I got for each char I typed in:

Try something like

    M-: (subst-char-in-region (point-min) (point-min) ?. ?-) RET

> I can change it back to three smaller defuns, but I will still though
> prefer to have them following each other in the code rather than
> scattered around in wdired.el for no reason as it is now. They are not
> refered from any other code.

Sounds great, thanks.


        Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-18 14:21                   ` Stefan Monnier
@ 2021-03-19 11:15                     ` Arthur Miller
  2021-03-19 16:18                       ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: Arthur Miller @ 2021-03-19 11:15 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1715 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> As you can guess from the previous comment, we should loop over BEG...END
>>> to process all the lines involved.
>> Can you elaborate more on this please?
>
> Because that's how `before-change-functions` is defined.  We can try and
> argue that in the specific case of wdired buffer the multiline case will
> never happen (presumably because of all the `read-only` annotations) or
> that it will always be "on the same line as point", but I'm pretty sure
> we'll end up finding corner cases where these are not true, so I find it
> easier to slap a simple while loop around the code and stop worrying
> about corner cases.

Ok, thanks for the explanation.

> [ As a general rule, I find it easier to write code if I just rely on
>   the few things which I know to be true, rather than having to think of
>   a set of unbounded possible cases.  Here, what we know to be true is
>   that the upcoming changes will not affect anything outside BEG...END
>   (at least not until `before-change-functions` are called again).  ]
>
>> This is what I got for each char I typed in:
>
> Try something like
>
>     M-: (subst-char-in-region (point-min) (point-min) ?. ?-) RET

Indeed, I was only concentrated on "interactive" editing, didn't thought
so much of other elisp functions.

Jere is a sketch with region, however, I am not able to get
editing permissions correctly. I am not really sure what is going on.

Maybe you or someone else see what is wrong there. Otherwise editing
names and symlinks works fine. I hope the general "setup" to loop
through the region is acceptable. I have attached my working file and as
a patch so take a look at whichever is easier.


[-- Attachment #2: partial-wdired.el --]
[-- Type: text/plain, Size: 7412 bytes --]

;;; partial-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)

;;;###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 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)
  (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--before-change-fn (beg end)
  (save-excursion
    ;; make sure to process at least entire line
    (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 beg end)
        (when wdired-allow-to-change-permissions
          (wdired--preprocess-perms beg end))
        (when (fboundp 'make-symbolic-link)
          (wdired--preprocess-symlinks beg end)))
      (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 (beg end)
  (save-excursion
    (with-silent-modifications
      (goto-char beg)
      (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 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)))))))

;; Put the needed properties to allow the user to change links' targets
(defun wdired--preprocess-symlinks (beg end)
  (save-excursion
    (with-silent-modifications
      (goto-char beg)
      (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 (beg end)
  (save-excursion
    (with-silent-modifications
        (setq-local wdired-col-perm nil)
        (goto-char beg)
	  (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-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"))

(provide 'partial-wdired)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Lazy-wdired-preprocessing.patch --]
[-- Type: text/x-patch, Size: 10341 bytes --]

From fe606faf0bd6dedadf5e8d90183d929e2bea60c4 Mon Sep 17 00:00:00 2001
From: Arthur Miller <arthur.miller@live.com>
Date: Fri, 19 Mar 2021 11:50:06 +0100
Subject: [PATCH] Lazy wdired preprocessing.

---
 lisp/wdired.el | 174 ++++++++++++++++++++++++++-----------------------
 1 file changed, 91 insertions(+), 83 deletions(-)

diff --git a/lisp/wdired.el b/lisp/wdired.el
index c495d8de34..88953d3bbd 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -249,21 +249,12 @@ wdired-change-to-wdired-mode
   (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 'after-change-functions 'wdired--restore-properties nil t)
+  (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)
-  ;; 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)
@@ -271,6 +262,27 @@ wdired-change-to-wdired-mode
 		 "Press \\[wdired-finish-edit] when finished \
 or \\[wdired-abort-changes] to abort changes")))
 
+(defun wdired--before-change-fn (beg end)
+  (save-excursion
+    ;; make sure to process at least entire line
+    (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 beg end)
+        (when wdired-allow-to-change-permissions
+          (wdired--preprocess-perms beg end))
+        (when (fboundp 'make-symbolic-link)
+          (wdired--preprocess-symlinks beg end)))
+      (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)
@@ -278,35 +290,33 @@ 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 (beg end)
   (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
+      (goto-char beg)
+      (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) '("." ".."))))
+		   (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.
+	  ;; 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 b-protection (point) 'read-only t)
+	   (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)))
-	  (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)
@@ -369,7 +379,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)
@@ -387,14 +396,15 @@ wdired-change-to-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."
+  "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))
@@ -710,21 +720,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 (beg end)
+  (save-excursion
+    (with-silent-modifications
+      (goto-char beg)
+      (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.
@@ -828,36 +836,36 @@ wdired-perm-mode-map
     (define-key map [down-mouse-1] 'wdired-mouse-toggle-bit)
     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)))))
+;; 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 (beg end)
+  (save-excursion
+    (with-silent-modifications
+        (setq-local wdired-col-perm nil)
+        (goto-char beg)
+	  (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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-19 11:15                     ` Arthur Miller
@ 2021-03-19 16:18                       ` Stefan Monnier
  2021-03-19 20:40                         ` Sv: " arthur miller
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2021-03-19 16:18 UTC (permalink / raw)
  To: Arthur Miller; +Cc: emacs-devel

> Indeed, I was only concentrated on "interactive" editing, didn't thought
> so much of other elisp functions.

That's the advantage of `before-change-functions`: you just don't need
to think about all the many different cases.

> Jere is a sketch with region, however, I am not able to get
> editing permissions correctly. I am not really sure what is going on.

I assume you tried with `(setq wdired-allow-to-change-permissions t)`.
I think I see what's going on:

- I go to the permissions of a "fresh" line (one that hasn't yet been
  processed by `wdired--before-change-fn`).
- I hit `x` with the intention to toggle the execute bit.
- Because it's "fresh" there's no keymap set so the key lookup decides
  I want to run `self-insert-command` rather than `wdired-set-bit`.
- Then we see another problem: the "read-only" test is performed before
  running the `before-change-functions` so the insertion of `x` is
  allowed even though we will end up placing a `read-only` property over
  that text just before the `x` is actually inserted.

I see other problems linked to this ordering problem:

- Go to the "non-editable" part of a "fresh" line.
- Hit `C-y`
- The yank is happily accepted.
- Worse: the text we yanked doesn't have the `read-only` property so you
  can keep modifying it (tho the rest of line now is marked `read-only`).

Hmm....


        Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Sv: Partial wdired (edit just filename at the point)
  2021-03-19 16:18                       ` Stefan Monnier
@ 2021-03-19 20:40                         ` arthur miller
  2021-03-19 21:58                           ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: arthur miller @ 2021-03-19 20:40 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel@gnu.org

[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]

Yes, of course, I had `wdired-allow-to-change-permissions' set to t; it is the pre-requisita.

It works fine when `wdired-allow-to-change-permissions' is set to 'advanced. Than the read only
property of permissions part is set to nil. But in ordinary case the read-only property does not
seem to be modified by the preprocessing routine. Instead preprocessing just marks where
permission part starts and ends. and it seems like actuall manipulation is done elsewhere in code
below.

Thanks for clarifying steps involved; I understand now, I didn't really understood them :-).

Ok, originally permissions flags are preprocessed before change to wdired is finalized. So something somewhere in
wdired setup is checking for permission bits? I think.
________________________________
Från: Stefan Monnier <monnier@iro.umontreal.ca>
Skickat: den 19 mars 2021 17:18
Till: Arthur Miller <arthur.miller@live.com>
Kopia: emacs-devel@gnu.org <emacs-devel@gnu.org>
Ämne: Re: Partial wdired (edit just filename at the point)

> Indeed, I was only concentrated on "interactive" editing, didn't thought
> so much of other elisp functions.

That's the advantage of `before-change-functions`: you just don't need
to think about all the many different cases.

> Jere is a sketch with region, however, I am not able to get
> editing permissions correctly. I am not really sure what is going on.

I assume you tried with `(setq wdired-allow-to-change-permissions t)`.
I think I see what's going on:

- I go to the permissions of a "fresh" line (one that hasn't yet been
  processed by `wdired--before-change-fn`).
- I hit `x` with the intention to toggle the execute bit.
- Because it's "fresh" there's no keymap set so the key lookup decides
  I want to run `self-insert-command` rather than `wdired-set-bit`.
- Then we see another problem: the "read-only" test is performed before
  running the `before-change-functions` so the insertion of `x` is
  allowed even though we will end up placing a `read-only` property over
  that text just before the `x` is actually inserted.

I see other problems linked to this ordering problem:

- Go to the "non-editable" part of a "fresh" line.
- Hit `C-y`
- The yank is happily accepted.
- Worse: the text we yanked doesn't have the `read-only` property so you
  can keep modifying it (tho the rest of line now is marked `read-only`).

Hmm....


        Stefan


[-- Attachment #2: Type: text/html, Size: 5229 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Sv: Partial wdired (edit just filename at the point)
  2021-03-19 20:40                         ` Sv: " arthur miller
@ 2021-03-19 21:58                           ` Stefan Monnier
  2021-03-20 11:23                             ` Sv: " arthur miller
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2021-03-19 21:58 UTC (permalink / raw)
  To: arthur miller; +Cc: emacs-devel@gnu.org

> Yes, of course, I had `wdired-allow-to-change-permissions' set to t; it is the pre-requisita.

Well, I first tried it "as-is" and was quite perplexed until I saw that
I needed to set `wdired-allow-to-change-permissions` before anything
would have a chance of working ;-)

> Ok, originally permissions flags are preprocessed before change to wdired is
> finalized. So something somewhere in
> wdired setup is checking for permission bits? I think.

It's simply that the way the edition of permissions works when
`wdired-allow-to-change-permissions' is set to t is that wdired places
a `keymap` property on those permissions.  This way when you hit `x`
while point is in that area, it doesn't run `self-insert-command` but
`wdired-set-bit` (which edits the buffer manually with
inhibit-read-only).

But with the new code, that `keymap` property is not added right away,
so `x` works if that line has already been processed by our
`dired--before-change-fn` but not otherwise, because there's no `keymap`
property yet and hence the `x` runs `self-insert-command` rather than
`wdired-set-bit`.


        Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Sv: Sv: Partial wdired (edit just filename at the point)
  2021-03-19 21:58                           ` Stefan Monnier
@ 2021-03-20 11:23                             ` arthur miller
  0 siblings, 0 replies; 33+ messages in thread
From: arthur miller @ 2021-03-20 11:23 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel@gnu.org


[-- Attachment #1.1: Type: text/plain, Size: 3033 bytes --]

Ok. Thanks for the guidance, I really wasn't that introduced into wdired, so
that last explanation really helped.

So here is a suggestion to one strategy:

When self-insert-command is called, check if point is in column bounds for
permissions. If that is case we check if the line was already processed. If line
has not been processed we call our before change function which will trigger
processing of the line so that keymap property is properly sett. Finally we put
back the event and let Emacs do it's thing: it will check again on same event
but this time there will be keymap property and wdired-set-bit will be triggered.

(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)))

For it to work I had to redirect self-insert-command which manual says against,
but seems to work fine here.

No idea if the implementation is the most elegant thing one could come up
with, and there is maybe a better way, but this one seems to work.

I am not at home, so I can't make a patch, I don't have entire source here
so I was just playing with my working file. I have attached it if someone would
be nice to test it. Can make a patch early next week if this is acceptable.

________________________________
Från: Stefan Monnier <monnier@iro.umontreal.ca>
Skickat: den 19 mars 2021 22:58
Till: arthur miller <arthur.miller@live.com>
Kopia: emacs-devel@gnu.org <emacs-devel@gnu.org>
Ämne: Re: Sv: Partial wdired (edit just filename at the point)

> Yes, of course, I had `wdired-allow-to-change-permissions' set to t; it is the pre-requisita.

Well, I first tried it "as-is" and was quite perplexed until I saw that
I needed to set `wdired-allow-to-change-permissions` before anything
would have a chance of working ;-)

> Ok, originally permissions flags are preprocessed before change to wdired is
> finalized. So something somewhere in
> wdired setup is checking for permission bits? I think.

It's simply that the way the edition of permissions works when
`wdired-allow-to-change-permissions' is set to t is that wdired places
a `keymap` property on those permissions.  This way when you hit `x`
while point is in that area, it doesn't run `self-insert-command` but
`wdired-set-bit` (which edits the buffer manually with
inhibit-read-only).

But with the new code, that `keymap` property is not added right away,
so `x` works if that line has already been processed by our
`dired--before-change-fn` but not otherwise, because there's no `keymap`
property yet and hence the `x` runs `self-insert-command` rather than
`wdired-set-bit`.


        Stefan


[-- Attachment #1.2: Type: text/html, Size: 7630 bytes --]

[-- Attachment #2: partial-wdired.el --]
[-- Type: application/octet-stream, Size: 9008 bytes --]

;;; partial-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)
  ;;(advice-add 'self-insert-command :around #'wdired--self-insert-advice)
  (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)
  (wdired--calculate-permission-coords)
  (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--before-change-fn (beg end)
  (save-excursion
    ;; make sure to process at least entire line
    (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 beg end)
        (when wdired-allow-to-change-permissions
          (wdired--preprocess-perms beg end))
        (when (fboundp 'make-symbolic-link)
          (wdired--preprocess-symlinks beg end)))
      (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--calculate-permission-coords ()
  (save-excursion
    (let (beg end)
      ;; lets find one column with permissions
      (re-search-forward dired-re-perms (point-max) 'eob)
      (setq beg (match-beginning 0))
      (setq end (match-end 0))
      ;; "transform" buffer positions to columns
      (setq-local wdired-perm-beg (- beg (line-beginning-position)))
      (setq-local wdired-perm-end (- end (line-beginning-position))))))

(defun wdired--point-at-perms-p ()
  "True if point is in a column between permission text start and end."
  (let ((beg (- (point) (line-beginning-position)))
        (end (- (point) (line-beginning-position))))
    (and (>= beg wdired-perm-beg) (<= end 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)))

;; 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 (beg end)
  (save-excursion
    (with-silent-modifications
      (goto-char beg)
      (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 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)))))))

;; Put the needed properties to allow the user to change links' targets
(defun wdired--preprocess-symlinks (beg end)
  (save-excursion
    (with-silent-modifications
      (goto-char beg)
      (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 (beg end)
  (save-excursion
    (with-silent-modifications
        (setq-local wdired-col-perm nil)
        (goto-char beg)
	  (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-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"))

(provide 'partial-wdired)

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-18  2:10               ` Stefan Monnier
  2021-03-18 10:26                 ` Arthur Miller
@ 2021-03-21 22:17                 ` Tomas Hlavaty
  2021-03-22  8:12                   ` tomas
  2021-03-22 13:11                   ` Stefan Monnier
  1 sibling, 2 replies; 33+ messages in thread
From: Tomas Hlavaty @ 2021-03-21 22:17 UTC (permalink / raw)
  To: Stefan Monnier, Arthur Miller; +Cc: emacs-devel

On Wed 17 Mar 2021 at 22:10, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> +  (add-hook 'before-change-functions 'wdired--preprocess-line nil t)
>   Please use #' rather than ' to quote functions (I know the rest of the
>   code here doesn't (yet), but we should move towards more systematic
>   use of #' so as to better catch obsolete functions and other issues).  ]

That is a strange advice.  I would expect #' to remove the indirection
via symbol with consequence, that redefining the named function would
not be reflected in the original #' value.

What is the difference between #' and ' in emacs lisp?
Is it different from Common Lisp?



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-21 22:17                 ` Tomas Hlavaty
@ 2021-03-22  8:12                   ` tomas
  2021-03-22 12:44                     ` Arthur Miller
  2021-03-22 13:11                   ` Stefan Monnier
  1 sibling, 1 reply; 33+ messages in thread
From: tomas @ 2021-03-22  8:12 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1389 bytes --]

On Sun, Mar 21, 2021 at 11:17:38PM +0100, Tomas Hlavaty wrote:

[...]

> What is the difference between #' and ' in emacs lisp?
> Is it different from Common Lisp?

It's in the docs. As ' is a shorthand for the special form quote,
#' is a shorthand for function (call this a LISP-2 symmetry ;-)

And function... the byte compiler "knows" about it:

 -- Special Form: function function-object
     This special form returns FUNCTION-OBJECT without evaluating it.
     In this, it is similar to ‘quote’ (*note Quoting::).  But unlike
     ‘quote’, it also serves as a note to the Emacs evaluator and
     byte-compiler that FUNCTION-OBJECT is intended to be used as a
     function.  Assuming FUNCTION-OBJECT is a valid lambda expression,
     this has two effects:

        • When the code is byte-compiled, FUNCTION-OBJECT is compiled
          into a byte-code function object (*note Byte Compilation::).

        • When lexical binding is enabled, FUNCTION-OBJECT is converted
          into a closure.  *Note Closures::.

     When FUNCTION-OBJECT is a symbol and the code is byte compiled, the
     byte-compiler will warn if that function is not defined or might
     not be known at run time.

so you help it make faster code and it helps you avoiding stupid
errors. That what merketing people call "win-win situation" ;-)

Cheers
 - t

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-22  8:12                   ` tomas
@ 2021-03-22 12:44                     ` Arthur Miller
  2021-03-22 14:50                       ` tomas
  0 siblings, 1 reply; 33+ messages in thread
From: Arthur Miller @ 2021-03-22 12:44 UTC (permalink / raw)
  To: tomas; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

<tomas@tuxteam.de> writes:

>         • When the code is byte-compiled, FUNCTION-OBJECT is compiled
>           into a byte-code function object (*note Byte Compilation::).
>
>         • When lexical binding is enabled, FUNCTION-OBJECT is converted
>           into a closure.  *Note Closures::.
>
>      When FUNCTION-OBJECT is a symbol and the code is byte compiled, the
>      byte-compiler will warn if that function is not defined or might
>      not be known at run time.

And we this Perl-like overloading of an operator where it has different
meanings in different contexts which makes Perl such an ugly language to
read once the code is written :-).

That is why I mean sharps are ugly.

> so you help it make faster code and it helps you avoiding stupid
> errors. That what merketing people call "win-win situation" ;-)

Yes, but marketing people don't care about other values then just
mnoney, i.e. win ^^.

I like that idea from that book about early Lisp days about simplifying
the language. Feels like CL has gone away from that idea long time ago,
and ELisp is going CL route.

Anyway, I messed up, when I sent my working code this weekend; I did
from an old laptop with old Emacs, I don't even understand why it worked
there or if I maybe send wrong version. Here is one patch that is
"almost" working: but for some reasons does not save permissions. I have
just brewed a fresh cup of coffee, so I'll see if I can see what is
wrong :).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Lazy-wdired-implementation.patch --]
[-- Type: text/x-patch, Size: 12108 bytes --]

From 44e375885a9472a9eedccce1cad6bf5fcd4e0106 Mon Sep 17 00:00:00 2001
From: Arthur Miller <arthur.miller@live.com>
Date: Mon, 22 Mar 2021 04:26:01 +0100
Subject: [PATCH] Lazy wdired implementation.

---
 lisp/wdired.el | 195 +++++++++++++++++++++++++++++--------------------
 1 file changed, 117 insertions(+), 78 deletions(-)

diff --git a/lisp/wdired.el b/lisp/wdired.el
index e040b52600..4971297cc7 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,27 +247,69 @@ 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)
+  ;;(advice-add 'self-insert-command :around #'wdired--self-insert-advice)
   (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)
+  (wdired--calculate-permission-coords)
+  (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--before-change-fn (beg end)
+  (save-excursion
+    ;; make sure to process at least entire line
+    (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 beg end)
+        (when wdired-allow-to-change-permissions
+          (wdired--preprocess-perms beg end))
+        (when (fboundp 'make-symbolic-link)
+          (wdired--preprocess-symlinks beg end)))
+      (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--calculate-permission-coords ()
+  (save-excursion
+    (let (beg end)
+      ;; lets find one column with permissions
+      (re-search-forward dired-re-perms (point-max) 'eob)
+      (setq beg (match-beginning 0))
+      (setq end (match-end 0))
+      ;; "transform" buffer positions to columns
+      (setq-local wdired-perm-beg (- beg (line-beginning-position)))
+      (setq-local wdired-perm-end (- end (line-beginning-position))))))
+
+(defun wdired--point-at-perms-p ()
+  "True if point is in a column between permission text start and end."
+  (let ((beg (- (point) (line-beginning-position)))
+        (end (- (point) (line-beginning-position))))
+    (and (>= beg wdired-perm-beg) (<= end 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-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 +317,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 (beg end)
   (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
+      (goto-char beg)
+      (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 +331,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 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)))
-	  (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 +403,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 +419,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 +744,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 (beg end)
+  (save-excursion
+    (with-silent-modifications
+      (goto-char beg)
+      (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 +862,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 (beg end)
+  (save-excursion
+    (with-silent-modifications
+        (setq-local wdired-col-perm nil)
+        (goto-char beg)
+	  (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


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-21 22:17                 ` Tomas Hlavaty
  2021-03-22  8:12                   ` tomas
@ 2021-03-22 13:11                   ` Stefan Monnier
  2021-03-22 20:08                     ` Tomas Hlavaty
  1 sibling, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2021-03-22 13:11 UTC (permalink / raw)
  To: Tomas Hlavaty; +Cc: Arthur Miller, emacs-devel

Tomas Hlavaty [2021-03-21 23:17:38] wrote:
> On Wed 17 Mar 2021 at 22:10, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>> +  (add-hook 'before-change-functions 'wdired--preprocess-line nil t)
>>   Please use #' rather than ' to quote functions (I know the rest of the
>>   code here doesn't (yet), but we should move towards more systematic
>>   use of #' so as to better catch obsolete functions and other issues).  ]
> That is a strange advice.  I would expect #' to remove the indirection
> via symbol with consequence, that redefining the named function would
> not be reflected in the original #' value.
> What is the difference between #' and ' in emacs lisp?
> Is it different from Common Lisp?

Yes, it's indeed different from Common Lisp: it doesn't have the same
effect as `symbol-function`.

In ELisp, when quoting a symbol, #' only tells Emacs that the intention
is to refer to the function by that name more than the symbol itself,
but semantically, it's almost identical to ' and almost always just
returns the symbol.  The "almost" is for the exception of references to
lexically scoped functions (where #' works like in Common Lisp).

Lexically-scoped functions occur most directly via `cl-flet` and
`cl-labels`, but they can also occur more indirectly via things like
`cl-defmethod` (where `cl-call-next-method` is lexically scoped) or
`named-let` (which rely on `cl-flet` or `cl-labels` internally).


        Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-22 12:44                     ` Arthur Miller
@ 2021-03-22 14:50                       ` tomas
  0 siblings, 0 replies; 33+ messages in thread
From: tomas @ 2021-03-22 14:50 UTC (permalink / raw)
  To: Arthur Miller; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]

On Mon, Mar 22, 2021 at 01:44:22PM +0100, Arthur Miller wrote:
> <tomas@tuxteam.de> writes:
> 
> >         • When the code is byte-compiled, FUNCTION-OBJECT is compiled
> >           into a byte-code function object (*note Byte Compilation::).
> >
> >         • When lexical binding is enabled, FUNCTION-OBJECT is converted
> >           into a closure.  *Note Closures::.
> >
> >      When FUNCTION-OBJECT is a symbol and the code is byte compiled, the
> >      byte-compiler will warn if that function is not defined or might
> >      not be known at run time.
> 
> And we this Perl-like overloading of an operator where it has different
> meanings in different contexts which makes Perl such an ugly language to
> read once the code is written :-).

Hey, but I ♥ Perl :)

[...]

> I like that idea from that book about early Lisp days about simplifying
> the language. Feels like CL has gone away from that idea long time ago,
> and ELisp is going CL route.

Yes, but early Lisps were Lisp-2's (or rather Lisp-n's for some n>=2).

And oh, BTW, Perl picked up that part, but made that explicit via
the symbol "sigils".

> Anyway, I messed up, when I sent my working code this weekend; I did
> from an old laptop with old Emacs, I don't even understand why it worked
> there or if I maybe send wrong version. Here is one patch that is
> "almost" working: but for some reasons does not save permissions. I have
> just brewed a fresh cup of coffee, so I'll see if I can see what is
> wrong :).

;-)

Cheers
 - t

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-22 13:11                   ` Stefan Monnier
@ 2021-03-22 20:08                     ` Tomas Hlavaty
  2021-03-22 20:28                       ` Andreas Schwab
  2021-03-22 21:00                       ` Stefan Monnier
  0 siblings, 2 replies; 33+ messages in thread
From: Tomas Hlavaty @ 2021-03-22 20:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Arthur Miller, emacs-devel

On Mon 22 Mar 2021 at 09:11, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> Tomas Hlavaty [2021-03-21 23:17:38] wrote:
>> On Wed 17 Mar 2021 at 22:10, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>>> +  (add-hook 'before-change-functions 'wdired--preprocess-line nil t)
>>>   Please use #' rather than ' to quote functions (I know the rest of the
>>>   code here doesn't (yet), but we should move towards more systematic
>>>   use of #' so as to better catch obsolete functions and other issues).  ]
>> That is a strange advice.  I would expect #' to remove the indirection
>> via symbol with consequence, that redefining the named function would
>> not be reflected in the original #' value.
>> What is the difference between #' and ' in emacs lisp?
>> Is it different from Common Lisp?
>
> Yes, it's indeed different from Common Lisp: it doesn't have the same
> effect as `symbol-function`.
>
> In ELisp, when quoting a symbol, #' only tells Emacs that the intention
> is to refer to the function by that name more than the symbol itself,
> but semantically, it's almost identical to ' and almost always just
> returns the symbol.  The "almost" is for the exception of references to
> lexically scoped functions (where #' works like in Common Lisp).
>
> Lexically-scoped functions occur most directly via `cl-flet` and
> `cl-labels`, but they can also occur more indirectly via things like
> `cl-defmethod` (where `cl-call-next-method` is lexically scoped) or
> `named-let` (which rely on `cl-flet` or `cl-labels` internally).

Thanks for the great explanation!

Emacs Lisp:

   (type-of 'type-of)
   -> symbol
   (type-of #'type-of)
   -> symbol
   (type-of (lambda () 42))
   -> cons
   (type-of #'(lambda () 42))
   -> cons

Common Lisp:

   (type-of 'type-of)
   -> SYMBOL
   (type-of #'type-of)
   -> FUNCTION
   (type-of (lambda () 42))
   -> FUNCTION
   (type-of #'(lambda () 42))
   -> FUNCTION



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-22 20:08                     ` Tomas Hlavaty
@ 2021-03-22 20:28                       ` Andreas Schwab
  2021-03-22 21:00                       ` Stefan Monnier
  1 sibling, 0 replies; 33+ messages in thread
From: Andreas Schwab @ 2021-03-22 20:28 UTC (permalink / raw)
  To: Tomas Hlavaty; +Cc: Stefan Monnier, Arthur Miller, emacs-devel

On Mär 22 2021, Tomas Hlavaty wrote:

> Emacs Lisp:
>
>    (type-of 'type-of)
>    -> symbol
>    (type-of #'type-of)
>    -> symbol
>    (type-of (lambda () 42))
>    -> cons
>    (type-of #'(lambda () 42))
>    -> cons

ELISP> (eval (byte-compile '(type-of '(lambda () 42))))
cons
ELISP> (eval (byte-compile '(type-of #'(lambda () 42))))
compiled-function

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-22 20:08                     ` Tomas Hlavaty
  2021-03-22 20:28                       ` Andreas Schwab
@ 2021-03-22 21:00                       ` Stefan Monnier
  2021-03-22 21:52                         ` Tomas Hlavaty
  1 sibling, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2021-03-22 21:00 UTC (permalink / raw)
  To: Tomas Hlavaty; +Cc: Arthur Miller, emacs-devel

>    (type-of 'type-of)
>    -> symbol
>    (type-of #'type-of)
>    -> symbol
>    (type-of (lambda () 42))
>    -> cons
>    (type-of #'(lambda () 42))
>    -> cons

As Andreas points out, the last two depend on whether the code is
compiled or not.  And FWIW, I'd like to get to the point where (type-of
(lambda () 42)) never returns `cons` but always some kind of "function"
type instead, indeed.


        Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-22 21:00                       ` Stefan Monnier
@ 2021-03-22 21:52                         ` Tomas Hlavaty
  2021-03-22 22:16                           ` Stefan Monnier
  2021-03-22 23:27                           ` Andreas Schwab
  0 siblings, 2 replies; 33+ messages in thread
From: Tomas Hlavaty @ 2021-03-22 21:52 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Andreas Schwab, Arthur Miller, emacs-devel

On Mon 22 Mar 2021 at 17:00, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> As Andreas points out, the last two depend on whether the code is
> compiled or not.

interesting

   ELISP> (eval (byte-compile '(type-of '(lambda () 42))))
   cons

cons obviously, it was probably meant this:

   ELISP> (eval (byte-compile '(type-of (lambda () 42))))
   compiled-function

which is the same as the result of:

   ELISP> (eval (byte-compile '(type-of #'(lambda () 42))))
   compiled-function

> And FWIW, I'd like to get to the point where (type-of (lambda () 42))
> never returns `cons` but always some kind of "function" type instead,
> indeed.

Do you mean more like the Common Lisp behaviour?

Getting back to the original advice, in compiled code or in such `some
kind of "function"' future, using #' will make it not possible to
redefine wdired--preprocess-line in the hook:

   (add-hook 'before-change-functions #'wdired--preprocess-line nil t)

while this will keep it possible to redefine wdired--preprocess-line in
the hook:

   (add-hook 'before-change-functions 'wdired--preprocess-line nil t)

Is that correct?  Or will `some kind of "function"' still behave like a
symbol (backwards compatible)?

I would like to understand when to prefer #' and when '.  In Common
Lisp, using #' for lambda is redundant; and if I know that a function
will not change I prefer #' (e.g. #'cons) but if I want to make it
possible to easily redefine the function, I prefer '
(e.g. 'my-function).  I am struggling to come up with a strategy when to
prefer #' or ' taking into account current and future Emacs Lisp
befaviour of #'.  It seems to me that using #' everywhere will make it
harder to redefine functions.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-22 21:52                         ` Tomas Hlavaty
@ 2021-03-22 22:16                           ` Stefan Monnier
  2021-03-22 22:39                             ` Tomas Hlavaty
  2021-03-22 23:27                           ` Andreas Schwab
  1 sibling, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2021-03-22 22:16 UTC (permalink / raw)
  To: Tomas Hlavaty; +Cc: Andreas Schwab, Arthur Miller, emacs-devel

>    ELISP> (eval (byte-compile '(type-of #'(lambda () 42))))
>    compiled-function
>
>> And FWIW, I'd like to get to the point where (type-of (lambda () 42))
>> never returns `cons` but always some kind of "function" type instead,
>> indeed.
>
> Do you mean more like the Common Lisp behaviour?
>
> Getting back to the original advice, in compiled code or in such `some
> kind of "function"' future, using #' will make it not possible to
> redefine wdired--preprocess-line in the hook:

No, I was talking about #'(lambda...) not about #'<symbol>.
IOW, I was talking about having `lambda` returning an actual function
object instead of returning a "list that can be treated as a function".

The use of #'<symbol> to refer to the actual symbol (so that it will
change its semantics whenever that symbol's definition is changed) is
much too ingrained in ELisp, I think changing it would introduce a lot
of breakage and I'm not sure what the benefit would be: there's already
`symbol-function` for that.

> I would like to understand when to prefer #' and when '.

Use #' when you refer to a function by name, use ' when you're just
referring to the symbol for some other reason (it might be a face,
a variable's name, some arbitrary constant, ...).

My rule of thumb is to write #'<foo> whenever I could also place here the
content of (symbol-function 'foo), just with a slightly different semantics.

For example, I prefer to write

    (defalias 'foo #'bar)

because it does something quite similar to

    (defalias 'foo (symbol-function 'bar))

whereas it does something very different from

    (defalias (symbol-function 'foo) #'bar)


-- Stefan




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-22 22:16                           ` Stefan Monnier
@ 2021-03-22 22:39                             ` Tomas Hlavaty
  0 siblings, 0 replies; 33+ messages in thread
From: Tomas Hlavaty @ 2021-03-22 22:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Andreas Schwab, Arthur Miller, emacs-devel

On Mon 22 Mar 2021 at 18:16, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> No, I was talking about #'(lambda...) not about #'<symbol>.
> IOW, I was talking about having `lambda` returning an actual function
> object instead of returning a "list that can be treated as a
> function".

I see, the case of interpreted (type-of (lambda () 42))

> The use of #'<symbol> to refer to the actual symbol (so that it will
> change its semantics whenever that symbol's definition is changed) is
> much too ingrained in ELisp, I think changing it would introduce a lot
> of breakage and I'm not sure what the benefit would be: there's already
> `symbol-function` for that.

OK, that clarifies it for me.

> Use #' when you refer to a function by name, use ' when you're just
> referring to the symbol for some other reason (it might be a face,
> a variable's name, some arbitrary constant, ...).
>
> My rule of thumb is to write #'<foo> whenever I could also place here the
> content of (symbol-function 'foo), just with a slightly different semantics.

That's great, thank you!



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-22 21:52                         ` Tomas Hlavaty
  2021-03-22 22:16                           ` Stefan Monnier
@ 2021-03-22 23:27                           ` Andreas Schwab
  1 sibling, 0 replies; 33+ messages in thread
From: Andreas Schwab @ 2021-03-22 23:27 UTC (permalink / raw)
  To: Tomas Hlavaty; +Cc: Stefan Monnier, Arthur Miller, emacs-devel

On Mär 22 2021, Tomas Hlavaty wrote:

> interesting
>
>    ELISP> (eval (byte-compile '(type-of '(lambda () 42))))
>    cons
>
> cons obviously, it was probably meant this:
>
>    ELISP> (eval (byte-compile '(type-of (lambda () 42))))
>    compiled-function

No, the point is to demonstrate the difference of ' vs #'.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Partial wdired (edit just filename at the point)
  2021-03-18 10:32                   ` Thierry Volpiatto
  2021-03-18 11:00                     ` Arthur Miller
@ 2021-03-23 23:32                     ` Michael Heerdegen
  1 sibling, 0 replies; 33+ messages in thread
From: Michael Heerdegen @ 2021-03-23 23:32 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, Arthur Miller, emacs-devel

Thierry Volpiatto <thievol@posteo.net> writes:

> Normally one can pass a list of files with absolute path to dired to get
> a dired buffer with only the needed file names to possibly edit with
> wdired, however this is broken in Emacs since ages; Helm is fixing this
> by advice, see
> https://github.com/emacs-helm/helm/blob/master/helm-lib.el#L186.
> This allow selecting files from helm-find-files and switching to a dired
> buffer with only those files.

I think something like this should definitely be offered by dired.

Michael.



^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2021-03-23 23:32 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 18:23 Partial wdired (edit just filename at the point) Arthur Miller
2021-03-17  1:18 ` Stefan Monnier
2021-03-17  2:21   ` Arthur Miller
2021-03-17  2:34     ` Stefan Monnier
2021-03-17 13:58       ` Arthur Miller
2021-03-17 14:09         ` Stefan Monnier
2021-03-17 19:56           ` Arthur Miller
2021-03-17 22:40             ` Arthur Miller
2021-03-18  2:10               ` Stefan Monnier
2021-03-18 10:26                 ` Arthur Miller
2021-03-18 10:32                   ` Thierry Volpiatto
2021-03-18 11:00                     ` Arthur Miller
2021-03-18 11:11                       ` Thierry Volpiatto
2021-03-18 11:46                         ` Arthur Miller
2021-03-23 23:32                     ` Michael Heerdegen
2021-03-18 14:21                   ` Stefan Monnier
2021-03-19 11:15                     ` Arthur Miller
2021-03-19 16:18                       ` Stefan Monnier
2021-03-19 20:40                         ` Sv: " arthur miller
2021-03-19 21:58                           ` Stefan Monnier
2021-03-20 11:23                             ` Sv: " arthur miller
2021-03-21 22:17                 ` Tomas Hlavaty
2021-03-22  8:12                   ` tomas
2021-03-22 12:44                     ` Arthur Miller
2021-03-22 14:50                       ` tomas
2021-03-22 13:11                   ` Stefan Monnier
2021-03-22 20:08                     ` Tomas Hlavaty
2021-03-22 20:28                       ` Andreas Schwab
2021-03-22 21:00                       ` Stefan Monnier
2021-03-22 21:52                         ` Tomas Hlavaty
2021-03-22 22:16                           ` Stefan Monnier
2021-03-22 22:39                             ` Tomas Hlavaty
2021-03-22 23:27                           ` Andreas Schwab

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).