GNU bug report logs - #13152
24.2.90; [REGRESSION] dired-get-marked-files changed

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 13152 in the body.
You can then email your comments to 13152 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: Leo <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Wed, 12 Dec 2012 10:23:26 +0800

1. Eval
(progn
  (require 'dired-x)
  (setq dired-guess-shell-alist-user
        '(("." (let ((files (dired-get-marked-files t current-prefix-arg)))
                 (message "%S" (current-buffer))
                 (dired-guess-default files))))))

2. In a dired buffer, run `!' (dired-do-shell-command) twice
   first without marking any file then with some files marked

You should get an error when files are marked. You'll notice in the
second run, the current-buffer has changed to ' *Marked Files*'.

Note in previous versions of Emacs, dired-get-marked-files throws an
error when run in a non-dired buffer, but in 24.2.90 it returns '(nil).

Leo

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

From: Glenn Morris <rgm <at> gnu.org>
To: Leo <sdl.web <at> gmail.com>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Tue, 11 Dec 2012 21:47:09 -0500

Leo wrote:

> 1. Eval
> (progn
>   (require 'dired-x)
>   (setq dired-guess-shell-alist-user
>         '(("." (let ((files (dired-get-marked-files t current-prefix-arg)))
>                  (message "%S" (current-buffer))
>                  (dired-guess-default files))))))
>
> 2. In a dired buffer, run `!' (dired-do-shell-command) 

Simply doing that, I get

  Variable binding depth exceeds max-specpdl-size

(or its equivalent) all the way back to 22.1.

Is this a recipe from emacs -Q, and if so which versions of Emacs is it
supposed to work in?

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

From: Leo <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Wed, 12 Dec 2012 13:30:05 +0800

On 2012-12-12 10:47 +0800, Glenn Morris wrote:
> Is this a recipe from emacs -Q, and if so which versions of Emacs is it
> supposed to work in?

Sorry. My fault. Eval this instead:

(progn
  (require 'dired-x)
  (setq dired-guess-shell-alist-user
        '(("." (let ((files (dired-get-marked-files t current-prefix-arg)))
                 (message "%S" (current-buffer))
                 (let (dired-guess-shell-alist-user)
                   (dired-guess-default files)))))))


Eval (dired-get-marked-files t current-prefix-arg) in dired and
non-dired buffers will do too.

My recipe is to single out the problem of buffer changing by
dired-get-marked-files, which makes forms in
dired-guess-shell-alist-user hard to predict.

Leo

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

From: Glenn Morris <rgm <at> gnu.org>
To: Leo <sdl.web <at> gmail.com>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Wed, 12 Dec 2012 03:13:28 -0500

Leo wrote:

> (progn
>   (require 'dired-x)
>   (setq dired-guess-shell-alist-user
>         '(("." (let ((files (dired-get-marked-files t current-prefix-arg)))
>                  (message "%S" (current-buffer))
>                  (let (dired-guess-shell-alist-user)
>                    (dired-guess-default files)))))))

When more than one file is marked, this says "no file on this line" in
24.2, and "Wrong type argument: stringp, nil" in 24.2.90. It's not
obvious to me that this matters.

> Eval (dired-get-marked-files t current-prefix-arg) in dired and
> non-dired buffers will do too.

Why does is matter what a dired command does in non-dired buffers?

> My recipe is to single out the problem of buffer changing by
> dired-get-marked-files,

I'm totally lost. Maybe someone else gets it.

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

From: Juri Linkov <juri <at> jurta.org>
To: Leo <sdl.web <at> gmail.com>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Wed, 12 Dec 2012 11:29:20 +0200

> You'll notice in the second run, the current-buffer has changed to
> ' *Marked Files*'.

Despite the recent changes in `dired-mark-pop-up' that added
`with-temp-buffer-window' the same behavior is observable
in older versions too.  For more than 1 file the call to
`(apply function args)' is performed in the buffer " *Marked Files*"
(in the QUIT-FUNCTION arg of `with-temp-buffer-window').

> Note in previous versions of Emacs, dired-get-marked-files throws an
> error when run in a non-dired buffer, but in 24.2.90 it returns '(nil).

This is a result of the following change:
http://bzr.savannah.gnu.org/lh/emacs/trunk/revision/110668
I see no problems with this.

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

From: Leo <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Wed, 12 Dec 2012 19:32:13 +0800

On 2012-12-12 17:29 +0800, Juri Linkov wrote:
[snipped 3 lines]
> I see no problems with this.

On 2012-12-12 16:13 +0800, Glenn Morris wrote:
[snipped 22 lines]
> I'm totally lost. Maybe someone else gets it.

See dired-guess-shell-alist-user, COMMAND can be an expression which is
eval'd in different contexts (buffers) and that is a problem.

Leo

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

From: Juri Linkov <juri <at> jurta.org>
To: Leo <sdl.web <at> gmail.com>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Thu, 13 Dec 2012 01:11:39 +0200

>> I see no problems with this.
>
> See dired-guess-shell-alist-user, COMMAND can be an expression which is
> eval'd in different contexts (buffers) and that is a problem.

Actually I meant there is no bug because the documentation of
dired-guess-shell-alist-user says nothing about the current buffer.

But indeed you have a problem when trying to do undocumented things.
Without changing the source code you can reuse the existing variable
`files' dynamically bound in `dired-guess-shell-command' like:

(progn
  (require 'dired-x)
  (setq dired-guess-shell-alist-user
        '(("." (progn
                 (message "%S" files)
                 (let (dired-guess-shell-alist-user)
                   (dired-guess-default files)))))))

This is bad practice but acceptable for ~/.emacs.

Regarding changing the current buffer in `dired-mark-pop-up'
where an expression is evaluated, I have doubts because
it might broke code that relies on the fact that the
selected window and the current buffer should be " *Marked Files*"
as it was in all older versions.

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

From: Leo <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Thu, 13 Dec 2012 09:20:31 +0800

On 2012-12-13 07:11 +0800, Juri Linkov wrote:
> (progn
>   (require 'dired-x)
>   (setq dired-guess-shell-alist-user
>         '(("." (progn
>                  (message "%S" files)
>                  (let (dired-guess-shell-alist-user)
>                    (dired-guess-default files)))))))
>
> This is bad practice but acceptable for ~/.emacs.

I fixed my setup before sending the bug report, pretty much as you did
here.

> Regarding changing the current buffer in `dired-mark-pop-up'
> where an expression is evaluated, I have doubts because
> it might broke code that relies on the fact that the
> selected window and the current buffer should be " *Marked Files*"
> as it was in all older versions.

Note in previous versions dired-mark-pop-up throws an error in that
buffer so I am pretty sure no code will expect COMMAND to be eval'd in 
" *Marked Files*".

Leo

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

From: Juri Linkov <juri <at> jurta.org>
To: Leo <sdl.web <at> gmail.com>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Fri, 14 Dec 2012 02:46:43 +0200

>> Regarding changing the current buffer in `dired-mark-pop-up'
>> where an expression is evaluated, I have doubts because
>> it might broke code that relies on the fact that the
>> selected window and the current buffer should be " *Marked Files*"
>> as it was in all older versions.
>
> Note in previous versions dired-mark-pop-up throws an error in that
> buffer so I am pretty sure no code will expect COMMAND to be eval'd in
> " *Marked Files*".

The doubt raises the fact that evaluating the expression after
selecting " *Marked Files*" was intentionally coded this way
in `dired-mark-pop-up':

	       (with-selected-window window
		 (unwind-protect
		     (apply function args)
		   (when (window-live-p window)
		     (quit-restore-window window 'kill))))

I guess the reason was to allow quit-restore-window to kill the
" *Marked Files*" buffer after the minibuffer (that reads a command)
is displayed and exited in `(apply function args)'.

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

From: Leo <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Fri, 14 Dec 2012 11:05:24 +0800

On 2012-12-14 08:46 +0800, Juri Linkov wrote:
> The doubt raises the fact that evaluating the expression after
> selecting " *Marked Files*" was intentionally coded this way
> in `dired-mark-pop-up':
>
> 	       (with-selected-window window
> 		 (unwind-protect
> 		     (apply function args)
> 		   (when (window-live-p window)
> 		     (quit-restore-window window 'kill))))
>
> I guess the reason was to allow quit-restore-window to kill the
> " *Marked Files*" buffer after the minibuffer (that reads a command)
> is displayed and exited in `(apply function args)'.

Could be but I only know superficially about the new `window' thingie.
My concern is mainly from the user's point of view. This is a trap that
is there to trick every user as soon as they start using lisp expression
for COMMAND. And if we let dired-mark-pop-up run in the HIDDEN ' *marked
files*' buffer without throwing an error, users might grow to rely on
it. Harder to get it right later.

I think the old behaviour is probably not the best but better, and eval
COMMAND should be in the dired buffer. This is what users expect.

Leo

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

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> jurta.org>
Cc: Leo <sdl.web <at> gmail.com>, 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Fri, 14 Dec 2012 11:26:04 +0100

> The doubt raises the fact that evaluating the expression after
> selecting " *Marked Files*" was intentionally coded this way
> in `dired-mark-pop-up':
>
> 	       (with-selected-window window
> 		 (unwind-protect
> 		     (apply function args)
> 		   (when (window-live-p window)
> 		     (quit-restore-window window 'kill))))
>
> I guess the reason was to allow quit-restore-window to kill the
> " *Marked Files*" buffer after the minibuffer (that reads a command)
> is displayed and exited in `(apply function args)'.

I suppose we can safely do

	   #'(lambda (window _value)
	       (unwind-protect
		   (apply function args)
		 (with-selected-window window
		   (when (window-live-p window)
		     (quit-restore-window window 'kill)))))

instead if people think it's better.  But FUNCTION "should not
manipulate files, just read input \(an argument or confirmation)." so I
don't see why this matters.

martin

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

From: martin rudalics <rudalics <at> gmx.at>
To: Leo <sdl.web <at> gmail.com>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Fri, 14 Dec 2012 11:26:15 +0100

> My concern is mainly from the user's point of view. This is a trap that
> is there to trick every user as soon as they start using lisp expression
> for COMMAND. And if we let dired-mark-pop-up run in the HIDDEN ' *marked
> files*' buffer without throwing an error, users might grow to rely on
> it. Harder to get it right later.

I don't understand.  `dired-mark-pop-up' displays that buffer, so why do
you mean it's hidden?  It is considered "ephemeral and generally
uninteresting to the user" so Emacs won't switch to it automatically.

martin

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

From: Leo <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Fri, 14 Dec 2012 18:43:38 +0800

On 2012-12-14 18:26 +0800, martin rudalics wrote:
> I don't understand.  `dired-mark-pop-up' displays that buffer, so why do
> you mean it's hidden?  It is considered "ephemeral and generally
> uninteresting to the user" so Emacs won't switch to it automatically.

I mean its buffer name is prefixed with ?\s.

Leo

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

From: martin rudalics <rudalics <at> gmx.at>
To: Leo <sdl.web <at> gmail.com>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Fri, 14 Dec 2012 11:58:30 +0100

>> I don't understand.  `dired-mark-pop-up' displays that buffer, so why do
>> you mean it's hidden?  It is considered "ephemeral and generally
>> uninteresting to the user" so Emacs won't switch to it automatically.
>
> I mean its buffer name is prefixed with ?\s.

Yes.  And the Elisp manual says

      Buffers that are ephemeral and generally uninteresting to the user
   have names starting with a space, so that the `list-buffers' and
   `buffer-menu' commands don't mention them (but if such a buffer visits
   a file, it *is* mentioned).  A name starting with space also initially
   disables recording undo information; see *Note Undo::.

martin

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

From: Juri Linkov <juri <at> jurta.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: Leo <sdl.web <at> gmail.com>, 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Sat, 15 Dec 2012 12:59:00 +0200

> But FUNCTION "should not manipulate files, just read input
> \(an argument or confirmation)." so I don't see why this matters.

I agree it shouldn't matter in which buffer FUNCTION is called.
The problem is caused by the flawed design of dired-x where
`dired-guess-default' doesn't propagate the value of `files'
to the evaluated expressions in `dired-guess-shell-alist-default'.
Also note how `dired-guess-shell-alist-default' is forced to use the
dynamically bound variable `file'.

The proper fix would be to redesign `dired-guess-shell-alist-default'
to funcall lambdas with one arg `files' instead of using `eval'.

So instead of

  (setq dired-guess-shell-alist-user
        '(("." (let ((files (dired-get-marked-files t current-prefix-arg)))
                 (let (dired-guess-shell-alist-user)
                   (dired-guess-default files))))))

Leo would be able to use

  (setq dired-guess-shell-alist-user
        '(("." (lambda (files)
                 (let (dired-guess-shell-alist-user)
                   (dired-guess-default files))))))

This could help to move useful features from dired-x.el to dired-aux.el.

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

From: Leo Liu <sdl.web <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: Juri Linkov <juri <at> jurta.org>, 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Fri, 04 Jan 2013 11:46:41 +0800

On 2012-12-14 18:26 +0800, martin rudalics wrote:
> I suppose we can safely do
>
> 	   #'(lambda (window _value)
> 	       (unwind-protect
> 		   (apply function args)
> 		 (with-selected-window window
> 		   (when (window-live-p window)
> 		     (quit-restore-window window 'kill)))))
>
> instead if people think it's better.  But FUNCTION "should not
> manipulate files, just read input \(an argument or confirmation)." so I
> don't see why this matters.
>
> martin

Hello Martin,

Could you restore the behaviour to previous releases i.e. throw an
error?

The return value '(nil) doesn't make any sense and a corner case to trap
future users.

Leo

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: martin rudalics <rudalics <at> gmx.at>, 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Fri, 04 Jan 2013 11:49:59 +0800

On 2012-12-15 18:59 +0800, Juri Linkov wrote:
> I agree it shouldn't matter in which buffer FUNCTION is called.
> The problem is caused by the flawed design of dired-x where
> `dired-guess-default' doesn't propagate the value of `files'
> to the evaluated expressions in `dired-guess-shell-alist-default'.

Ideally, yes. But not common in elisp where stateful is the norm. We
just have to use the least surprise principle.

> Also note how `dired-guess-shell-alist-default' is forced to use the
> dynamically bound variable `file'.
>
> The proper fix would be to redesign `dired-guess-shell-alist-default'
> to funcall lambdas with one arg `files' instead of using `eval'.
>
> So instead of
>
>   (setq dired-guess-shell-alist-user
>         '(("." (let ((files (dired-get-marked-files t current-prefix-arg)))
>                  (let (dired-guess-shell-alist-user)
>                    (dired-guess-default files))))))
>
> Leo would be able to use
>
>   (setq dired-guess-shell-alist-user
>         '(("." (lambda (files)
>                  (let (dired-guess-shell-alist-user)
>                    (dired-guess-default files))))))
>
> This could help to move useful features from dired-x.el to dired-aux.el.

This seems like a good design. Cheers.

Leo

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

From: martin rudalics <rudalics <at> gmx.at>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: Juri Linkov <juri <at> jurta.org>, 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Fri, 04 Jan 2013 08:08:16 +0100

> Could you restore the behaviour to previous releases i.e. throw an
> error?
>
> The return value '(nil) doesn't make any sense and a corner case to trap
> future users.

I don't understand what you mean with previous behavior.  IIUC I need
the unwindform to make sure the window gets killed when a user aborts
her dialog with dired, for example, via C-g.  What am I missing?

martin

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

From: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Mon, 07 Jan 2013 10:38:32 +0800

On 2013-01-04 15:08 +0800, martin rudalics wrote:
> I don't understand what you mean with previous behavior.  IIUC I need
> the unwindform to make sure the window gets killed when a user aborts
> her dialog with dired, for example, via C-g.  What am I missing?

Eval (dired-get-marked-files t current-prefix-arg) in non dired buffers
threw an error in the released emacsen, which is a more reasonable thing
to do.

In the pretest it returns (nil).

Leo

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

From: martin rudalics <rudalics <at> gmx.at>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Mon, 07 Jan 2013 08:43:41 +0100

> Eval (dired-get-marked-files t current-prefix-arg) in non dired buffers
> threw an error in the released emacsen, which is a more reasonable thing
> to do.
>
> In the pretest it returns (nil).

But this is unrelated to the change in `dired-mark-pop-up' I suppose.

martin

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

From: Leo Liu <sdl.web <at> gmail.com>
To: 13152 <at> debbugs.gnu.org
Cc: Juri Linkov <juri <at> jurta.org>
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Tue, 08 Jan 2013 19:02:18 +0800

On 2013-01-07 10:38 +0800, Leo Liu wrote:
> Eval (dired-get-marked-files t current-prefix-arg) in non dired buffers
> threw an error in the released emacsen, which is a more reasonable thing
> to do.
>
> In the pretest it returns (nil).

The incompatibility is introduced by this change:

Author: Juri Linkov <juri <at> jurta.org>
Date:   Wed Sep 19 23:09:55 2012 +0300

    * lisp/dired-aux.el (dired-diff): Add (require 'diff) because
    `diff-latest-backup-file' is not autoloaded.
    (dired-do-chxxx, dired-do-chmod): Set `no-error-if-not-filep' arg
    of `dired-get-filename' to t to not report error when there is
    no default file on the current line.


Juri, could you take a look? thanks.

Leo

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

From: Glenn Morris <rgm <at> gnu.org>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: Juri Linkov <juri <at> jurta.org>, 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Tue, 08 Jan 2013 17:27:05 -0500

Leo Liu wrote:

> On 2013-01-07 10:38 +0800, Leo Liu wrote:
>> Eval (dired-get-marked-files t current-prefix-arg) in non dired buffers
>> threw an error in the released emacsen, which is a more reasonable thing
>> to do.
>>
>> In the pretest it returns (nil).
>
> The incompatibility is introduced by this change:
>
> Author: Juri Linkov <juri <at> jurta.org>
> Date:   Wed Sep 19 23:09:55 2012 +0300
>
>     * lisp/dired-aux.el (dired-diff): Add (require 'diff) because
>     `diff-latest-backup-file' is not autoloaded.
>     (dired-do-chxxx, dired-do-chmod): Set `no-error-if-not-filep' arg
>     of `dired-get-filename' to t to not report error when there is
>     no default file on the current line.

How can a commit that did not touch dired-get-marked-files, or any code
called by it, possibly have changed the behaviour of
dired-get-marked-files?

Presumably you mean this change

http://lists.gnu.org/archive/html/emacs-diffs/2012-10/msg00382.html

http://debbugs.gnu.org/12725

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

From: Glenn Morris <rgm <at> gnu.org>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Tue, 08 Jan 2013 17:35:44 -0500

Maybe you can just make dired-get-marked-files throw an explicit error
"Not in Dired mode" unless (derived-mode-p 'dired-mode), though I still
don't see what the big deal is here. You would need to check ever caller
to make sure this did not break anything.

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

From: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Wed, 09 Jan 2013 09:15:49 +0800

On 2013-01-09 06:35 +0800, Glenn Morris wrote:
> Maybe you can just make dired-get-marked-files throw an explicit error
> "Not in Dired mode" unless (derived-mode-p 'dired-mode), though I still
> don't see what the big deal is here. You would need to check ever caller
> to make sure this did not break anything.

dired-get-marked-files can now return (nil) which is non-nil and yet not
a list of files. It is an incorrect return value.

OK, I'll have a look tonight. Thanks.

Leo

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

From: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Wed, 09 Jan 2013 09:19:18 +0800

On 2013-01-09 06:27 +0800, Glenn Morris wrote:
> Presumably you mean this change
>
> http://lists.gnu.org/archive/html/emacs-diffs/2012-10/msg00382.html

That is it. Sorry. The two commits were what I found but I mixed up.
Anyway I'll take a look tonight.

Leo

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Wed, 09 Jan 2013 19:07:57 +0800

On 2013-01-09 06:35 +0800, Glenn Morris wrote:
> Maybe you can just make dired-get-marked-files throw an explicit error
> "Not in Dired mode" unless (derived-mode-p 'dired-mode), though I still
> don't see what the big deal is here. You would need to check ever caller
> to make sure this did not break anything.

After looking at this function more closely, I am reluctant to put in a
(derived-mode-p 'dired-mode) check since all the related functions work
more generally (using regexps) and do not depend on mode checking.

So I propose the following patch instead which avoids returning values
such as (nil) or (t nil). What do you think? - Leo


diff --git a/lisp/dired.el b/lisp/dired.el
index b62b4d1a..2f7d5b37 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -620,12 +620,14 @@ (defun dired-get-marked-files (&optional localp arg filter distinguish-one-marke
 If DISTINGUISH-ONE-MARKED is non-nil, then if we find just one marked file,
 return (t FILENAME) instead of (FILENAME).
 Don't use that together with FILTER."
-  (let* ((all-of-them
-	  (save-excursion
-	    (dired-map-over-marks
-	     (dired-get-filename localp 'no-error-if-not-filep)
-	     arg nil distinguish-one-marked)))
-	 result)
+  (let ((all-of-them
+	 (save-excursion
+	   (delq nil (dired-map-over-marks
+		      (dired-get-filename localp 'no-error-if-not-filep)
+		      arg nil distinguish-one-marked))))
+	result)
+    (when (equal all-of-them '(t))
+      (setq all-of-them nil))
     (if (not filter)
 	(if (and distinguish-one-marked (eq (car all-of-them) t))
 	    all-of-them

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

From: Juri Linkov <juri <at> jurta.org>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: Glenn Morris <rgm <at> gnu.org>, 13152 <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Thu, 10 Jan 2013 02:50:10 +0200

> So I propose the following patch instead which avoids returning values
> such as (nil) or (t nil). What do you think?

Could you provide a test case that demonstrates the problem
that you are trying to fix?

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

From: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Thu, 10 Jan 2013 09:04:58 +0800

On 2013-01-10 08:50 +0800, Juri Linkov wrote:
> Could you provide a test case that demonstrates the problem
> that you are trying to fix?

Like (dired-get-marked-files nil nil nil t)?

Leo

Message #93 received at 13152-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Liu <sdl.web <at> gmail.com>
To: 13152-done <at> debbugs.gnu.org
Subject: Re: bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
Date: Sat, 19 Jan 2013 02:40:11 +0800

Fixed in 24.3.

Previous Next

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