* 23.0.50; Deleting files in wdired does not work
@ 2008-01-21 5:41 Phil Sung
2008-01-25 7:35 ` martin rudalics
0 siblings, 1 reply; 6+ messages in thread
From: Phil Sung @ 2008-01-21 5:41 UTC (permalink / raw)
To: emacs-pretest-bug
Marking files for deletion in wdired by deleting their filenames (as
advertised in wdired.el and in the Info) does not work.
Steps to reproduce:
Open a directory in dired, e.g.: C-x C-f ~/wd/test RET
Change to wdired mode: M-x wdired-change-to-wdired-mode
The buffer looks like this:
/home/phil/wd/test:
total used in directory 8 available 130996008
drwxr-xr-x 2 phil phil 4096 2008-01-20 22:51 .
drwxr-xr-x 15 phil phil 4096 2008-01-20 03:34 ..
-rw-r--r-- 1 phil phil 0 2008-01-20 22:51 bar
-rw-r--r-- 1 phil phil 0 2008-01-20 22:51 foo
Delete a filename, like so:
/home/phil/wd/test:
total used in directory 8 available 130996008
drwxr-xr-x 2 phil phil 4096 2008-01-20 22:51 .
drwxr-xr-x 15 phil phil 4096 2008-01-20 03:34 ..
-rw-r--r-- 1 phil phil 0 2008-01-20 22:51
-rw-r--r-- 1 phil phil 0 2008-01-20 22:51 foo
Save the changes: C-c C-c
The buffer now looks like this:
/home/phil/wd/test:
total used in directory 8 available 130996008
drwxr-xr-x 2 phil phil 4096 2008-01-20 22:52 .
drwxr-xr-x 15 phil phil 4096 2008-01-20 03:34 ..
-rw-r--r-- 1 phil phil 0 2008-01-20 22:51
-rw-r--r-- 1 phil phil 0 2008-01-20 22:51 foo
Expected output-- the file name should have been restored and the file
marked for deletion:
/home/phil/wd/test:
total used in directory 8 available 130984448
drwxr-xr-x 2 phil phil 4096 2008-01-20 23:11 .
drwxr-xr-x 15 phil phil 4096 2008-01-20 03:34 ..
D -rw-r--r-- 1 phil phil 0 2008-01-20 23:11 bar
-rw-r--r-- 1 phil phil 0 2008-01-20 23:11 foo
It looks like wdired-get-filename adds a spurious newline to the result
whenever the filename has been deleted in wdired. In addition, in
wdired-finish-edit, to test whether a filename has been deleted, we should look
at (wdired-get-filename t), which returns the basename only, and see whether it
equals "".
I propose the following patch:
*** lisp/wdired.el 8 Jan 2008 20:44:46 -0000 1.33
--- lisp/wdired.el 21 Jan 2008 05:03:35 -0000
***************
*** 324,328 ****
(if old
(setq file (get-text-property beg 'old-name))
! (setq end (next-single-property-change (1+ beg) 'end-name))
(setq file (buffer-substring-no-properties (1+ beg) end)))
(and file (setq file (wdired-normalize-filename file))))
--- 324,332 ----
(if old
(setq file (get-text-property beg 'old-name))
! (setq end (min (line-end-position)
! ; n-s-p-c can return nil on the last filename in the
! ; buffer when that filename has been deleted
! (or (next-single-property-change (1+ beg) 'end-name)
! (point-max))))
(setq file (buffer-substring-no-properties (1+ beg) end)))
(and file (setq file (wdired-normalize-filename file))))
***************
*** 390,394 ****
(when (and file-ori (not (equal file-new file-ori)))
(setq changes t)
! (if (not file-new) ;empty filename!
(setq files-deleted (cons file-ori files-deleted))
(setq file-new (substitute-in-file-name file-new))
--- 394,398 ----
(when (and file-ori (not (equal file-new file-ori)))
(setq changes t)
! (if (zerop (length (wdired-get-filename t))) ; empty filename
(setq files-deleted (cons file-ori files-deleted))
(setq file-new (substitute-in-file-name file-new))
---
In GNU Emacs 23.0.50.3 (i686-pc-linux-gnu, GTK+ Version 2.12.3)
of 2008-01-20 on phil
Important settings:
value of $LC_ALL: nil
value of $LC_COLLATE: nil
value of $LC_CTYPE: nil
value of $LC_MESSAGES: nil
value of $LC_MONETARY: nil
value of $LC_NUMERIC: nil
value of $LC_TIME: nil
value of $LANG: en_US.UTF-8
locale-coding-system: utf-8
default-enable-multibyte-characters: t
Major mode: Dired
Minor modes in effect:
shell-dirtrack-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
unify-8859-on-encoding-mode: t
utf-translate-cjk-mode: t
auto-compression-mode: t
line-number-mode: t
Recent input:
C-a C-f C-f C-f ESC : ESC p C-e RET C-e DEL DEL DEL
DEL DEL DEL C-a ESC : ESC p C-e ESC O D SPC t C-e RET
C-e a C-a ESC : ESC p C-e RET C-e DEL C-x o C-s w d
i r e d - g e t - f i l e n a m e C-s C-s C-s C-s C-s
C-s C-s C-s C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n
C-n C-n C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-n C-n C-n C-p C-p C-p C-n C-x b w d i r e
d l DEL . e l RET C-n C-s w d i r e d - g e t - f i
l e n a m e C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n
C-n C-n C-n C-n C-p C-p C-p ESC C-x C-n C-n C-p C-e
C-p C-e C-n C-n C-p C-p C-a C-e C-a C-n C-e C-a C-p
C-e C-a C-e C-a C-n C-e C-a C-p C-e C-a C-n C-e C-a
C-p C-e C-a C-n C-e C-a C-p C-e C-a C-n C-p C-n C-p
C-s w d i r e d - f i n i s h - e d i t C-s C-r C-r
C-n ESC C-x C-x o C-c ESC C-n g C-x C-q C-e C-p DEL
DEL DEL C-x C-s C-n C-n C-p C-p C-p C-n C-n C-n C-p
C-p C-p C-p C-p C-n C-n C-n C-n C-n C-p C-p C-p C-p
C-p C-p C-n C-n C-n C-n C-n C-n C-p C-p C-p C-p C-p
C-n C-n C-n C-n C-n ESC x r e p o r t - e m a c s -
b u g TAB RET
Recent messages:
"/home/phil/wd/test/abcdef"
" [2 times]
"a"
Mark saved where search started [2 times]
wdired-get-filename
Mark saved where search started
wdired-finish-edit
Changes aborted
Press C-c C-c when finished or C-c ESC to abort changes
Source file `/home/phil/source/emacs/lisp/mail/sendmail.el' newer than
byte-compiled file
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 23.0.50; Deleting files in wdired does not work
2008-01-21 5:41 23.0.50; Deleting files in wdired does not work Phil Sung
@ 2008-01-25 7:35 ` martin rudalics
2008-01-25 10:16 ` Phil Sung
0 siblings, 1 reply; 6+ messages in thread
From: martin rudalics @ 2008-01-25 7:35 UTC (permalink / raw)
To: Phil Sung; +Cc: emacs-pretest-bug
Thank you for your report and the patch. Sorry for the delay in
answering it.
> It looks like wdired-get-filename adds a spurious newline to the result
> whenever the filename has been deleted in wdired.
The 'end-name text property assigned by `wdired-preprocess-files' covers
exactly one character after the filename, usually the newline. Deleting
the filename and calling `next-single-property-change' from (1+ beg)
will thus return the position after the newline and set `file' to "\n"
in `wdired-get-filename' - as you observed. For the last filename the
'end-name text-property remains constant till the end of the buffer,
(setq end (next-single-property-change (1+ beg) 'end-name))
returns nil, and the subsequent
(setq file (buffer-substring-no-properties (1+ beg) end)))
fails with a "Wrong type argument: integer-or-marker-p, nil", am I
right? Hence, you indeed detected two different bugs.
> In addition, in
> wdired-finish-edit, to test whether a filename has been deleted, we should look
> at (wdired-get-filename t), which returns the basename only, and see whether it
> equals "".
>
> I propose the following patch:
Your patch fixes both bugs. However, I'd prefer if you replaced the
original
(setq end (next-single-property-change (1+ beg) 'end-name))
by something like the untested
(if (get-text-property (1+ beg) 'end-name)
""
(setq end (next-single-property-change (1+ beg) 'end-name))
(setq file (buffer-substring-no-properties (1+ beg) end))))
It would allow this part of wdired to also work in the case where the
filename does not appear at the end of the line in dired listings. I
don't know how realistic that is - but we shouldn't hardcode anything
into wdired which wasn't there initially. Could you try to do that?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 23.0.50; Deleting files in wdired does not work
2008-01-25 7:35 ` martin rudalics
@ 2008-01-25 10:16 ` Phil Sung
2008-01-25 13:17 ` martin rudalics
0 siblings, 1 reply; 6+ messages in thread
From: Phil Sung @ 2008-01-25 10:16 UTC (permalink / raw)
To: martin rudalics; +Cc: emacs-pretest-bug
On Jan 25, 2008 2:35 AM, martin rudalics <rudalics@gmx.at> wrote:
> For the last filename [...]
>
> (setq file (buffer-substring-no-properties (1+ beg) end)))
>
> fails with a "Wrong type argument: integer-or-marker-p, nil", am I
> right?
That's correct.
> However, I'd prefer if you replaced the original
>
> (setq end (next-single-property-change (1+ beg) 'end-name))
>
> by something like the untested
>
> (if (get-text-property (1+ beg) 'end-name)
> ""
> (setq end (next-single-property-change (1+ beg) 'end-name))
> (setq file (buffer-substring-no-properties (1+ beg) end))))
>
> It would allow this part of wdired to also work in the case where the
> filename does not appear at the end of the line in dired listings. I
> don't know how realistic that is - but we shouldn't hardcode anything
> into wdired which wasn't there initially.
Sure. The following works for me, assuming (wdired-get-filename t)
ought to return "" rather than nil on a deleted filename:
(if (get-text-property (1+ beg) 'end-name)
(setq file "")
(setq end (next-single-property-change (1+ beg) 'end-name))
(setq file (buffer-substring-no-properties (1+ beg) end)))))
An updated patch follows.
--Phil
*** lisp/wdired.el 8 Jan 2008 20:44:46 -0000 1.33
--- lisp/wdired.el 25 Jan 2008 09:44:35 -0000
***************
*** 324,329 ****
(if old
(setq file (get-text-property beg 'old-name))
! (setq end (next-single-property-change (1+ beg) 'end-name))
! (setq file (buffer-substring-no-properties (1+ beg) end)))
(and file (setq file (wdired-normalize-filename file))))
(if (or no-dir old)
--- 324,331 ----
(if old
(setq file (get-text-property beg 'old-name))
! (if (get-text-property (1+ beg) 'end-name)
! (setq file "")
! (setq end (next-single-property-change (1+ beg) 'end-name))
! (setq file (buffer-substring-no-properties (1+ beg) end)))))
(and file (setq file (wdired-normalize-filename file))))
(if (or no-dir old)
***************
*** 390,394 ****
(when (and file-ori (not (equal file-new file-ori)))
(setq changes t)
! (if (not file-new) ;empty filename!
(setq files-deleted (cons file-ori files-deleted))
(setq file-new (substitute-in-file-name file-new))
--- 392,396 ----
(when (and file-ori (not (equal file-new file-ori)))
(setq changes t)
! (if (zerop (length (wdired-get-filename t))) ; empty filename
(setq files-deleted (cons file-ori files-deleted))
(setq file-new (substitute-in-file-name file-new))
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 23.0.50; Deleting files in wdired does not work
2008-01-25 10:16 ` Phil Sung
@ 2008-01-25 13:17 ` martin rudalics
2008-01-25 22:43 ` Phil Sung
0 siblings, 1 reply; 6+ messages in thread
From: martin rudalics @ 2008-01-25 13:17 UTC (permalink / raw)
To: Phil Sung; +Cc: emacs-pretest-bug
> The following works for me, assuming (wdired-get-filename t)
> ought to return "" rather than nil on a deleted filename:
Looks good, thank you. I think this ought to return "" here because nil
should be reserved for `wdired-get-filename' was not able to figure out
the user's intentions from the text it found there. And marking a file
for deletion is a fairly dangerous undertaking.
If no one objects I'll install this in Emacs 22.2 in a couple of days.
Meanwhile you could do two things:
- Use
(string-equal (wdired-get-filename t) "")
instead of
(zerop (length (wdired-get-filename t)))
because the latter will return t when `wdired-get-filename' returns nil.
- Add a couple of comments documenting what was wrong and what you have
done to fix it.
Thanks again.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 23.0.50; Deleting files in wdired does not work
2008-01-25 13:17 ` martin rudalics
@ 2008-01-25 22:43 ` Phil Sung
2008-01-26 17:27 ` martin rudalics
0 siblings, 1 reply; 6+ messages in thread
From: Phil Sung @ 2008-01-25 22:43 UTC (permalink / raw)
To: martin rudalics; +Cc: emacs-pretest-bug
After re-reading your previous diagnosis, I think there is a shorter
fix. Searching for 'end-name starting from beg instead of (1+ beg)
finds the end of filename correctly whether the filename is empty or
not.
Once wdired-get-filename is fixed, wdired-finish-edit actually needs
no changes, because (wdired-get-filename) does return nil when the
filename is empty.
* wdired.el (wdired-get-filename): Change `(1+ beg)' to `beg' so
that the filename end is found even when the filename is empty.
Fixes error and spurious newlines when marking files for deletion.
--Phil
*** lisp/wdired.el 8 Jan 2008 20:44:46 -0000 1.33
--- lisp/wdired.el 25 Jan 2008 22:08:31 -0000
***************
*** 324,328 ****
(if old
(setq file (get-text-property beg 'old-name))
! (setq end (next-single-property-change (1+ beg) 'end-name))
(setq file (buffer-substring-no-properties (1+ beg) end)))
(and file (setq file (wdired-normalize-filename file))))
--- 324,328 ----
(if old
(setq file (get-text-property beg 'old-name))
! (setq end (next-single-property-change beg 'end-name))
(setq file (buffer-substring-no-properties (1+ beg) end)))
(and file (setq file (wdired-normalize-filename file))))
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 23.0.50; Deleting files in wdired does not work
2008-01-25 22:43 ` Phil Sung
@ 2008-01-26 17:27 ` martin rudalics
0 siblings, 0 replies; 6+ messages in thread
From: martin rudalics @ 2008-01-26 17:27 UTC (permalink / raw)
To: Phil Sung; +Cc: emacs-pretest-bug
> After re-reading your previous diagnosis, I think there is a shorter
> fix. Searching for 'end-name starting from beg instead of (1+ beg)
> finds the end of filename correctly whether the filename is empty or
> not.
>
> Once wdired-get-filename is fixed, wdired-finish-edit actually needs
> no changes, because (wdired-get-filename) does return nil when the
> filename is empty.
>
> * wdired.el (wdired-get-filename): Change `(1+ beg)' to `beg' so
> that the filename end is found even when the filename is empty.
> Fixes error and spurious newlines when marking files for deletion.
Checked into Emacs 22 branch and the trunk. Thanks again.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-01-26 17:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-21 5:41 23.0.50; Deleting files in wdired does not work Phil Sung
2008-01-25 7:35 ` martin rudalics
2008-01-25 10:16 ` Phil Sung
2008-01-25 13:17 ` martin rudalics
2008-01-25 22:43 ` Phil Sung
2008-01-26 17:27 ` martin rudalics
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).