all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* 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 external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.