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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 32673 in the body.
You can then email your comments to 32673 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop'
Date: Sun, 09 Sep 2018 18:52:55 +0300

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.

Message #8 received at 32673 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Juri Linkov <juri <at> linkov.net>
Cc: 32673 <at> debbugs.gnu.org
Subject: Re: bug#32673: 27.0.50;
 wdired: broken 'wdired--restore-dired-filename-prop'
Date: Wed, 12 Sep 2018 00:08:52 +0200

On Sun, 09 Sep 2018 18:52:55 +0300 Juri Linkov <juri <at> 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.

Message #11 received at 32673 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 32673 <at> debbugs.gnu.org
Subject: Re: bug#32673: 27.0.50;
 wdired: broken 'wdired--restore-dired-filename-prop'
Date: Wed, 12 Sep 2018 02:32:09 +0300

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

Thanks, since your patch fixes the reported problem, there is no reason
to postpone installing it.  Regarding improving the handling of symlinks,
maybe you can use some code from dired-move-to-end-of-filename,
especially handling dired-ls-F-marks-symlinks.

Message #14 received at 32673 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Juri Linkov <juri <at> linkov.net>
Cc: 32673 <at> debbugs.gnu.org
Subject: Re: bug#32673: 27.0.50;
 wdired: broken 'wdired--restore-dired-filename-prop'
Date: Wed, 12 Sep 2018 12:02:00 +0200

On Wed, 12 Sep 2018 02:32:09 +0300 Juri Linkov <juri <at> linkov.net> wrote:

>> 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.
>
> Thanks, since your patch fixes the reported problem, there is no reason
> to postpone installing it.  

Ok, but I'll wait another day or two, to give others a chance to object.

>                             Regarding improving the handling of symlinks,
> maybe you can use some code from dired-move-to-end-of-filename,
> especially handling dired-ls-F-marks-symlinks.

Thanks for the pointer.  I don't think I'll have time to look into that
before leaving, but maybe I can do so while I'm away, otherwise after
returning.

Steve Berman

Message #17 received at 32673 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 32673 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
Subject: Re: bug#32673: 27.0.50;
 wdired: broken 'wdired--restore-dired-filename-prop'
Date: Wed, 12 Sep 2018 16:57:09 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Date: Wed, 12 Sep 2018 02:32:09 +0300
> Cc: 32673 <at> debbugs.gnu.org
> 
> Thanks, since your patch fixes the reported problem, there is no reason
> to postpone installing it.

Agreed.

Message #20 received at 32673 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 32673 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#32673: 27.0.50;
 wdired: broken 'wdired--restore-dired-filename-prop'
Date: Thu, 13 Sep 2018 22:21:46 +0200

On Wed, 12 Sep 2018 16:57:09 +0300 Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Juri Linkov <juri <at> linkov.net>
>> Date: Wed, 12 Sep 2018 02:32:09 +0300
>> Cc: 32673 <at> debbugs.gnu.org
>> 
>> Thanks, since your patch fixes the reported problem, there is no reason
>> to postpone installing it.
>
> Agreed.

Ok, pushed to master as commit 755fa34.

Steve Berman

Message #23 received at 32673 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 32673 <at> debbugs.gnu.org
Subject: Re: bug#32673: 27.0.50; wdired: broken
 'wdired--restore-dired-filename-prop'
Date: Wed, 12 Feb 2020 23:53:30 +0200

tags 32673 fixed
close 32673 27.1
quit

> Ok, pushed to master as commit 755fa34.

Thanks, closed.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.