GNU bug report logs - #58256
Possible mistake in recent `dired-do-flagged-delete' change

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 58256 in the body.
You can then email your comments to 58256 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: Stefan Kangas <stefankangas <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Possible mistake in recent `dired-do-flagged-delete' change
Date: Sun, 2 Oct 2022 20:42:17 +0200

It seems like there might be a mistake in the `dired-do-flagged-delete'
of this commit:

    commit 194d54a929a83fede75d618b104acd1b544feb10
    Author: Stephen Berman <stephen.berman <at> gmx.net>
    Date:   Fri Jun 4 12:01:41 2021 +0200

        Fix placement of point in Dired deletion operations

It seems like there is a `dolist' that will always run on the empty
list.  Was perhaps the below the intended change?

If so, I wonder how this code would have worked without that `dolist' so
far, and if that line could just be removed instead?

diff --git a/lisp/dired.el b/lisp/dired.el
index b9e89292e2..358e815c88 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -3664,16 +3664,17 @@ dired-do-flagged-delete
 	 case-fold-search markers)
     (if (save-excursion (goto-char (point-min))
 			(re-search-forward regexp nil t))
-	(dired-internal-do-deletions
-         (nreverse
-	  ;; this can't move point since ARG is nil
-	  (dired-map-over-marks (cons (dired-get-filename)
-                                      (let ((m (point-marker)))
-                                        (push m markers)
-                                        m))
-			        nil))
-	 nil t)
-      (dolist (m markers) (set-marker m nil))
+        (progn
+          (dired-internal-do-deletions
+           (nreverse
+            ;; this can't move point since ARG is nil
+            (dired-map-over-marks (cons (dired-get-filename)
+                                   (let ((m (point-marker)))
+                                     (push m markers)
+                                     m))
+                             nil))
+           nil t)
+          (dolist (m markers) (set-marker m nil)))
       (or nomessage
 	  (message "(No deletions requested)")))))

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: 58256 <at> debbugs.gnu.org
Cc: Stephen Berman <stephen.berman <at> gmx.net>
Subject: Re: bug#58256: Possible mistake in recent `dired-do-flagged-delete'
 change
Date: Sun, 2 Oct 2022 19:17:27 +0000

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefankangas <at> gmail.com> writes:

> If so, I wonder how this code would have worked without that `dolist' so
> far, and if that line could just be removed instead?

The command seems to be working correctly with the attached patch.

Am I missing something here?

[0001-lisp-dired.el-dired-do-flagged-delete-Simplify.patch (text/x-diff, attachment)]

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

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 58256 <at> debbugs.gnu.org
Subject: Re: bug#58256: Possible mistake in recent `dired-do-flagged-delete'
 change
Date: Sun, 02 Oct 2022 22:46:15 +0200

On Sun, 2 Oct 2022 20:42:17 +0200 Stefan Kangas <stefankangas <at> gmail.com> wrote:

> It seems like there might be a mistake in the `dired-do-flagged-delete'
> of this commit:
>
>     commit 194d54a929a83fede75d618b104acd1b544feb10
>     Author: Stephen Berman <stephen.berman <at> gmx.net>
>     Date:   Fri Jun 4 12:01:41 2021 +0200
>
>         Fix placement of point in Dired deletion operations
>
> It seems like there is a `dolist' that will always run on the empty
> list.  Was perhaps the below the intended change?

Yeah, it looks like I mistakenly put the dolist in the 'else' clause
instead of the 'then' clause.  Thanks for catching that.

On Sun, 2 Oct 2022 19:17:27 +0000 Stefan Kangas <stefankangas <at> gmail.com> wrote:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> If so, I wonder how this code would have worked without that `dolist' so
>> far, and if that line could just be removed instead?
>
> The command seems to be working correctly with the attached patch.
>
> Am I missing something here?

I think doing it that way leaves the markers in the buffer, though
probably for typical use cases that's not a problem, and it is simpler
that way (as I noted when I posted my patch).  I guess the same
simplification can be applied to `dired-do-delete' as well.

Steve Berman

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 58256 <at> debbugs.gnu.org
Subject: Re: bug#58256: Possible mistake in recent `dired-do-flagged-delete'
 change
Date: Mon, 3 Oct 2022 00:30:13 +0200

close 58256 29.1
thanks

Stephen Berman <stephen.berman <at> gmx.net> writes:

> Yeah, it looks like I mistakenly put the dolist in the 'else' clause
> instead of the 'then' clause.  Thanks for catching that.
[...]
> I think doing it that way leaves the markers in the buffer, though
> probably for typical use cases that's not a problem, and it is simpler
> that way (as I noted when I posted my patch).  I guess the same
> simplification can be applied to `dired-do-delete' as well.

OK.  Let's stick with the more correct way and actually delete the
markers.

I've done this in commit 0d0d59b32c, and I'm closing this bug report.

Previous Next

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