bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop'

[Stephen Berman]

On Sun, 09 Sep 2018 18:52:55 +0300 Juri Linkov <juri@linkov.net> wrote:

> As reported in 
> https://lists.gnu.org/archive/html/emacs-devel/2018-09/msg00372.html
> Wdired breaks delete-selection-mode in the latest master 27.0.50:
>
> 1. create scratch directory with a file
>
>    mkdir /tmp/test
>    cd /tmp/test
>    touch foo.c
>
> 2. start emacs
>
>    LC_ALL=C emacs -Q -nw
>
> 3. activate delete-selection-mode
>
>    M-x delete-selection-mode RET
>
> 4. go into the folder
>
>    C-x C-f /tmp/test
>
> 5. enter wdired mode
>
>    C-x C-q
>
> 6. replace 'foo' with 'test' by selecting the whole file name
>    and typing a new one, e.g.
>
>    M-2 C-M-SPC test
>
>    or
>
>    C-SPC C-e test
>
> When typing the first letter, it removes delete-selection-pre-hook from
> pre-command-hook, thus breaking delete-selection-mode, because of this
> error seen in the *Messages* buffer:
>
>   Error in pre-command-hook (delete-selection-pre-hook): (error "No file on 
> this line")
>
> The same error is reproducible when simply deleting an old file name
> before typing a new one in the step 6, e.g.:
>
>    M-d test
>
> The raised error is:
>
>   Debugger entered--Lisp error: (error "No file on this line")
>     signal(error ("No file on this line"))
>     error("%s" "No file on this line")
>     dired-move-to-end-of-filename(nil)
>     dired-get-filename()
>     wdired--restore-dired-filename-prop(198 198 1)
>     delete-char(1 nil)
>     funcall-interactively(delete-char 1 nil)
>     call-interactively(delete-char nil nil)
>     command-execute(delete-char)
>
> This is caused by changes in bug#32173.

The immediate cause of this is that wdired--restore-dired-filename-prop,
which is on after-change-functions, calls dired-get-filename without
making sure that the file name is not the empty string.  But it turns
out that using dired-get-filename in that function also doesn't work
with symlinks as I had thought (and tested, but evidently not enough):
after enabling wdired-mode but before making a change, (file-symlink-p
(dired-get-filename)) on a symlink returns the name of the target, but
as soon as a change is made, that sexp returns nil, which causes a
text-read-only signal.  The patch below replaces this sexp and according
to my tests avoids the error which breaks delete-selection-mode and also
most text-read-only signals (one remains: when deleting the whole link
name; I haven't been able to figure out how to avoid that).  The patch
should get more testing (in particular, is looking for the `l' flag a
reliable way to identify a symlink?), and an ERT test would also be
good, but I probably can't do either for the next few weeks, because
I'll be travelling.  I can install the patch before I leave, since it
seems at least better than the status quo, or maybe someone else can
take a look and either confirm that it is good enough or else come up
with a better fix.

Steve Berman

diff --git a/lisp/wdired.el b/lisp/wdired.el
index be0bde290a..3157e887d7 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -607,15 +607,22 @@ wdired-check-kill-buffer
 (defun wdired--restore-dired-filename-prop (beg end _len)
   (save-match-data
     (save-excursion
-      (beginning-of-line)
-      (when (re-search-forward directory-listing-before-filename-regexp
-                               (line-end-position) t)
-        (setq beg (point)
-              end (if (and (file-symlink-p (dired-get-filename))
-                           (search-forward " -> " (line-end-position) t))
-                      (goto-char (match-beginning 0))
-                    (line-end-position)))
-        (put-text-property beg end 'dired-filename t)))))
+      (let ((lep (line-end-position)))
+        (beginning-of-line)
+        (when (re-search-forward
+               directory-listing-before-filename-regexp lep t)
+          (setq beg (point)
+                ;; If the file is a symlink, put the dired-filename
+                ;; property only on the link name.  (Using
+                ;; (file-symlink-p (dired-get-filename)) fails in
+                ;; wdired-mode, bug#32673.)
+                end (if (and (re-search-backward
+                              dired-permission-flags-regexp nil t)
+                             (looking-at "l")
+                             (search-forward " -> " lep t))
+                        (goto-char (match-beginning 0))
+                      lep))
+          (put-text-property beg end 'dired-filename t))))))
 
 (defun wdired-next-line (arg)
   "Move down lines then position at filename or the current column.