unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
@ 2012-12-12  2:23 Leo
  2012-12-12  2:47 ` Glenn Morris
  2012-12-12  9:29 ` Juri Linkov
  0 siblings, 2 replies; 29+ messages in thread
From: Leo @ 2012-12-12  2:23 UTC (permalink / raw)
  To: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-12  2:23 bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed Leo
@ 2012-12-12  2:47 ` Glenn Morris
  2012-12-12  5:30   ` Leo
  2012-12-12  9:29 ` Juri Linkov
  1 sibling, 1 reply; 29+ messages in thread
From: Glenn Morris @ 2012-12-12  2:47 UTC (permalink / raw)
  To: Leo; +Cc: 13152

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?





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-12  2:47 ` Glenn Morris
@ 2012-12-12  5:30   ` Leo
  2012-12-12  8:13     ` Glenn Morris
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2012-12-12  5:30 UTC (permalink / raw)
  To: 13152

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






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-12  5:30   ` Leo
@ 2012-12-12  8:13     ` Glenn Morris
  0 siblings, 0 replies; 29+ messages in thread
From: Glenn Morris @ 2012-12-12  8:13 UTC (permalink / raw)
  To: Leo; +Cc: 13152

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.





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-12  2:23 bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed Leo
  2012-12-12  2:47 ` Glenn Morris
@ 2012-12-12  9:29 ` Juri Linkov
  2012-12-12 11:32   ` Leo
  1 sibling, 1 reply; 29+ messages in thread
From: Juri Linkov @ 2012-12-12  9:29 UTC (permalink / raw)
  To: Leo; +Cc: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-12  9:29 ` Juri Linkov
@ 2012-12-12 11:32   ` Leo
  2012-12-12 23:11     ` Juri Linkov
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2012-12-12 11:32 UTC (permalink / raw)
  To: 13152

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






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-12 11:32   ` Leo
@ 2012-12-12 23:11     ` Juri Linkov
  2012-12-13  1:20       ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: Juri Linkov @ 2012-12-12 23:11 UTC (permalink / raw)
  To: Leo; +Cc: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-12 23:11     ` Juri Linkov
@ 2012-12-13  1:20       ` Leo
  2012-12-14  0:46         ` Juri Linkov
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2012-12-13  1:20 UTC (permalink / raw)
  To: 13152

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






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-13  1:20       ` Leo
@ 2012-12-14  0:46         ` Juri Linkov
  2012-12-14  3:05           ` Leo
  2012-12-14 10:26           ` martin rudalics
  0 siblings, 2 replies; 29+ messages in thread
From: Juri Linkov @ 2012-12-14  0:46 UTC (permalink / raw)
  To: Leo; +Cc: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-14  0:46         ` Juri Linkov
@ 2012-12-14  3:05           ` Leo
  2012-12-14 10:26             ` martin rudalics
  2012-12-14 10:26           ` martin rudalics
  1 sibling, 1 reply; 29+ messages in thread
From: Leo @ 2012-12-14  3:05 UTC (permalink / raw)
  To: 13152

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






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-14  0:46         ` Juri Linkov
  2012-12-14  3:05           ` Leo
@ 2012-12-14 10:26           ` martin rudalics
  2012-12-15 10:59             ` Juri Linkov
  2013-01-04  3:46             ` Leo Liu
  1 sibling, 2 replies; 29+ messages in thread
From: martin rudalics @ 2012-12-14 10:26 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Leo, 13152

 > 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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-14  3:05           ` Leo
@ 2012-12-14 10:26             ` martin rudalics
  2012-12-14 10:43               ` Leo
  0 siblings, 1 reply; 29+ messages in thread
From: martin rudalics @ 2012-12-14 10:26 UTC (permalink / raw)
  To: Leo; +Cc: 13152

 > 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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-14 10:26             ` martin rudalics
@ 2012-12-14 10:43               ` Leo
  2012-12-14 10:58                 ` martin rudalics
  0 siblings, 1 reply; 29+ messages in thread
From: Leo @ 2012-12-14 10:43 UTC (permalink / raw)
  To: 13152

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






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-14 10:43               ` Leo
@ 2012-12-14 10:58                 ` martin rudalics
  0 siblings, 0 replies; 29+ messages in thread
From: martin rudalics @ 2012-12-14 10:58 UTC (permalink / raw)
  To: Leo; +Cc: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-14 10:26           ` martin rudalics
@ 2012-12-15 10:59             ` Juri Linkov
  2013-01-04  3:49               ` Leo Liu
  2013-01-04  3:46             ` Leo Liu
  1 sibling, 1 reply; 29+ messages in thread
From: Juri Linkov @ 2012-12-15 10:59 UTC (permalink / raw)
  To: martin rudalics; +Cc: Leo, 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-14 10:26           ` martin rudalics
  2012-12-15 10:59             ` Juri Linkov
@ 2013-01-04  3:46             ` Leo Liu
  2013-01-04  7:08               ` martin rudalics
  1 sibling, 1 reply; 29+ messages in thread
From: Leo Liu @ 2013-01-04  3:46 UTC (permalink / raw)
  To: martin rudalics; +Cc: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2012-12-15 10:59             ` Juri Linkov
@ 2013-01-04  3:49               ` Leo Liu
  0 siblings, 0 replies; 29+ messages in thread
From: Leo Liu @ 2013-01-04  3:49 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-04  3:46             ` Leo Liu
@ 2013-01-04  7:08               ` martin rudalics
  2013-01-07  2:38                 ` Leo Liu
  0 siblings, 1 reply; 29+ messages in thread
From: martin rudalics @ 2013-01-04  7:08 UTC (permalink / raw)
  To: Leo Liu; +Cc: 13152

 > 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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-04  7:08               ` martin rudalics
@ 2013-01-07  2:38                 ` Leo Liu
  2013-01-07  7:43                   ` martin rudalics
  2013-01-08 11:02                   ` Leo Liu
  0 siblings, 2 replies; 29+ messages in thread
From: Leo Liu @ 2013-01-07  2:38 UTC (permalink / raw)
  To: 13152

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






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-07  2:38                 ` Leo Liu
@ 2013-01-07  7:43                   ` martin rudalics
  2013-01-08 11:02                   ` Leo Liu
  1 sibling, 0 replies; 29+ messages in thread
From: martin rudalics @ 2013-01-07  7:43 UTC (permalink / raw)
  To: Leo Liu; +Cc: 13152

 > 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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-07  2:38                 ` Leo Liu
  2013-01-07  7:43                   ` martin rudalics
@ 2013-01-08 11:02                   ` Leo Liu
  2013-01-08 22:27                     ` Glenn Morris
  1 sibling, 1 reply; 29+ messages in thread
From: Leo Liu @ 2013-01-08 11:02 UTC (permalink / raw)
  To: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-08 11:02                   ` Leo Liu
@ 2013-01-08 22:27                     ` Glenn Morris
  2013-01-08 22:35                       ` Glenn Morris
  2013-01-09  1:19                       ` Leo Liu
  0 siblings, 2 replies; 29+ messages in thread
From: Glenn Morris @ 2013-01-08 22:27 UTC (permalink / raw)
  To: Leo Liu; +Cc: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-08 22:27                     ` Glenn Morris
@ 2013-01-08 22:35                       ` Glenn Morris
  2013-01-09  1:15                         ` Leo Liu
  2013-01-09 11:07                         ` Leo Liu
  2013-01-09  1:19                       ` Leo Liu
  1 sibling, 2 replies; 29+ messages in thread
From: Glenn Morris @ 2013-01-08 22:35 UTC (permalink / raw)
  To: Leo Liu; +Cc: 13152


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.






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-08 22:35                       ` Glenn Morris
@ 2013-01-09  1:15                         ` Leo Liu
  2013-01-09 11:07                         ` Leo Liu
  1 sibling, 0 replies; 29+ messages in thread
From: Leo Liu @ 2013-01-09  1:15 UTC (permalink / raw)
  To: 13152

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






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-08 22:27                     ` Glenn Morris
  2013-01-08 22:35                       ` Glenn Morris
@ 2013-01-09  1:19                       ` Leo Liu
  1 sibling, 0 replies; 29+ messages in thread
From: Leo Liu @ 2013-01-09  1:19 UTC (permalink / raw)
  To: 13152

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






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-08 22:35                       ` Glenn Morris
  2013-01-09  1:15                         ` Leo Liu
@ 2013-01-09 11:07                         ` Leo Liu
  2013-01-10  0:50                           ` Juri Linkov
  2013-01-18 18:40                           ` Leo Liu
  1 sibling, 2 replies; 29+ messages in thread
From: Leo Liu @ 2013-01-09 11:07 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-09 11:07                         ` Leo Liu
@ 2013-01-10  0:50                           ` Juri Linkov
  2013-01-10  1:04                             ` Leo Liu
  2013-01-18 18:40                           ` Leo Liu
  1 sibling, 1 reply; 29+ messages in thread
From: Juri Linkov @ 2013-01-10  0:50 UTC (permalink / raw)
  To: Leo Liu; +Cc: 13152

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





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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-10  0:50                           ` Juri Linkov
@ 2013-01-10  1:04                             ` Leo Liu
  0 siblings, 0 replies; 29+ messages in thread
From: Leo Liu @ 2013-01-10  1:04 UTC (permalink / raw)
  To: 13152

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






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

* bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed
  2013-01-09 11:07                         ` Leo Liu
  2013-01-10  0:50                           ` Juri Linkov
@ 2013-01-18 18:40                           ` Leo Liu
  1 sibling, 0 replies; 29+ messages in thread
From: Leo Liu @ 2013-01-18 18:40 UTC (permalink / raw)
  To: 13152-done

Fixed in 24.3.





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

end of thread, other threads:[~2013-01-18 18:40 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-12  2:23 bug#13152: 24.2.90; [REGRESSION] dired-get-marked-files changed Leo
2012-12-12  2:47 ` Glenn Morris
2012-12-12  5:30   ` Leo
2012-12-12  8:13     ` Glenn Morris
2012-12-12  9:29 ` Juri Linkov
2012-12-12 11:32   ` Leo
2012-12-12 23:11     ` Juri Linkov
2012-12-13  1:20       ` Leo
2012-12-14  0:46         ` Juri Linkov
2012-12-14  3:05           ` Leo
2012-12-14 10:26             ` martin rudalics
2012-12-14 10:43               ` Leo
2012-12-14 10:58                 ` martin rudalics
2012-12-14 10:26           ` martin rudalics
2012-12-15 10:59             ` Juri Linkov
2013-01-04  3:49               ` Leo Liu
2013-01-04  3:46             ` Leo Liu
2013-01-04  7:08               ` martin rudalics
2013-01-07  2:38                 ` Leo Liu
2013-01-07  7:43                   ` martin rudalics
2013-01-08 11:02                   ` Leo Liu
2013-01-08 22:27                     ` Glenn Morris
2013-01-08 22:35                       ` Glenn Morris
2013-01-09  1:15                         ` Leo Liu
2013-01-09 11:07                         ` Leo Liu
2013-01-10  0:50                           ` Juri Linkov
2013-01-10  1:04                             ` Leo Liu
2013-01-18 18:40                           ` Leo Liu
2013-01-09  1:19                       ` Leo Liu

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