unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop'
@ 2018-09-09 15:52 Juri Linkov
  2018-09-11 22:08 ` Stephen Berman
  0 siblings, 1 reply; 7+ messages in thread
From: Juri Linkov @ 2018-09-09 15:52 UTC (permalink / raw)
  To: 32673

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.





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

* bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop'
  2018-09-09 15:52 bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop' Juri Linkov
@ 2018-09-11 22:08 ` Stephen Berman
  2018-09-11 23:32   ` Juri Linkov
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Berman @ 2018-09-11 22:08 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32673

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.





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

* bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop'
  2018-09-11 22:08 ` Stephen Berman
@ 2018-09-11 23:32   ` Juri Linkov
  2018-09-12 10:02     ` Stephen Berman
  2018-09-12 13:57     ` Eli Zaretskii
  0 siblings, 2 replies; 7+ messages in thread
From: Juri Linkov @ 2018-09-11 23:32 UTC (permalink / raw)
  To: Stephen Berman; +Cc: 32673

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





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

* bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop'
  2018-09-11 23:32   ` Juri Linkov
@ 2018-09-12 10:02     ` Stephen Berman
  2018-09-12 13:57     ` Eli Zaretskii
  1 sibling, 0 replies; 7+ messages in thread
From: Stephen Berman @ 2018-09-12 10:02 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32673

On Wed, 12 Sep 2018 02:32:09 +0300 Juri Linkov <juri@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





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

* bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop'
  2018-09-11 23:32   ` Juri Linkov
  2018-09-12 10:02     ` Stephen Berman
@ 2018-09-12 13:57     ` Eli Zaretskii
  2018-09-13 20:21       ` Stephen Berman
  1 sibling, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2018-09-12 13:57 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 32673, stephen.berman

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

Agreed.





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

* bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop'
  2018-09-12 13:57     ` Eli Zaretskii
@ 2018-09-13 20:21       ` Stephen Berman
  2020-02-12 21:53         ` Juri Linkov
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Berman @ 2018-09-13 20:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 32673, Juri Linkov

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

>> From: Juri Linkov <juri@linkov.net>
>> Date: Wed, 12 Sep 2018 02:32:09 +0300
>> Cc: 32673@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





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

* bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop'
  2018-09-13 20:21       ` Stephen Berman
@ 2020-02-12 21:53         ` Juri Linkov
  0 siblings, 0 replies; 7+ messages in thread
From: Juri Linkov @ 2020-02-12 21:53 UTC (permalink / raw)
  To: Stephen Berman; +Cc: 32673

tags 32673 fixed
close 32673 27.1
quit

> Ok, pushed to master as commit 755fa34.

Thanks, closed.





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

end of thread, other threads:[~2020-02-12 21:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-09 15:52 bug#32673: 27.0.50; wdired: broken 'wdired--restore-dired-filename-prop' Juri Linkov
2018-09-11 22:08 ` Stephen Berman
2018-09-11 23:32   ` Juri Linkov
2018-09-12 10:02     ` Stephen Berman
2018-09-12 13:57     ` Eli Zaretskii
2018-09-13 20:21       ` Stephen Berman
2020-02-12 21:53         ` Juri Linkov

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