unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#7131: 24.0.50; `dired' with cons arg should not ignore arg if buffer exists
@ 2010-09-29 15:34 Drew Adams
  2017-07-18 11:39 ` Tino Calancha
  0 siblings, 1 reply; 3+ messages in thread
From: Drew Adams @ 2010-09-29 15:34 UTC (permalink / raw)
  To: 7131

(dired '("foo" "file1.c" "file2.c")) opens a Dired buffer named `foo'
with only files file1.c and file2.c.  This is a useful feature.  The doc
says only that if the arg is a cons then its first element is taken as
the directory name and the rest are the files to list.
 
But if "foo" is already the name of an existing Dired buffer then that
existing buffer `foo' is simply displayed as it was.  The explicit
file-list argument is completely ignored.
 
Buffer `foo' should instead be updated (its contents replaced) to list
only the files to be included (files in the file-list arg).  That is
more useful, and it is anyway easy to recuperate the original Dired
buffer if for some reason the buffer name were a mistake.
 
The code in question is `dired-internal-noselect'.  This code comment
gives the behavior rationale for the case where the Dired buffer
already exists:
 
;; If there is an existing dired buffer for DIRNAME, just leave
;; buffer as it is (don't even call dired-revert).
;; This saves time especially for deep trees or with ange-ftp.
;; The user can type `g' easily, and it is more consistent with find-file.
;; But if SWITCHES are given they are probably different from the
;; buffer's old value, so call dired-sort-other, which does
;; revert the buffer. A pity we can't possibly do "Directory has
;; changed - refresh? " like find-file does.
 
(DIRNAME is a typo here BTW - the argument is actually called DIR-OR-LIST,
and if it is a cons then the name is just its car.)
 
Note the rationale: If SWITCHES are not given, that is, if there was no
expressed intention to change the content of the listing, then just
reuse an existing buffer.  But if such an intention was expressed, then
respect it.
 
Following that rationale, the behavior for a cons arg should be to
respect the explicit file list passed, not to simply ignore it and reuse
an existing buffer.
 
And since it would be problematic to test the file list for differences
from the existing listing (whether or not it came from a cons arg
previously), we should always just update the buffer to reflect the
file-list that is passed.  IOW, with an explicit file list, never reuse
an existing Dired buffer of the same name.
 
Note that use of a cons as arg is not the typical use, and this use case has
sometimes been forgotten/overlooked when coding, so that there have been several
(recently fixed) bugs because of oversights.  I mention this because I imagine
that is the case here: it is not that we intentionally reuse the existing buffer
and ignore the file list.  We do that probably only because the coder wasn't
thinking about the cons case.  (The DIRNAME typo supports this supposition.)
 
In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2010-09-20 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4) --no-opt --cflags
-Ic:/imagesupport/include'
 






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

* bug#7131: 24.0.50; `dired' with cons arg should not ignore arg if buffer exists
  2010-09-29 15:34 bug#7131: 24.0.50; `dired' with cons arg should not ignore arg if buffer exists Drew Adams
@ 2017-07-18 11:39 ` Tino Calancha
  2017-07-21  4:36   ` Tino Calancha
  0 siblings, 1 reply; 3+ messages in thread
From: Tino Calancha @ 2017-07-18 11:39 UTC (permalink / raw)
  To: Drew Adams; +Cc: 7131

"Drew Adams" <drew.adams@oracle.com> writes:

> (dired '("foo" "file1.c" "file2.c")) opens a Dired buffer named `foo'
> with only files file1.c and file2.c.  This is a useful feature.  The doc
> says only that if the arg is a cons then its first element is taken as
> the directory name and the rest are the files to list.
>  
> But if "foo" is already the name of an existing Dired buffer then that
> existing buffer `foo' is simply displayed as it was.  The explicit
> file-list argument is completely ignored.
>  
> Buffer `foo' should instead be updated (its contents replaced) to list
> only the files to be included (files in the file-list arg).
Agreed.

> The code in question is `dired-internal-noselect'.  This code comment
> gives the behavior rationale for the case where the Dired buffer
> already exists:
>  
> ;; If there is an existing dired buffer for DIRNAME, just leave
> ;; buffer as it is (don't even call dired-revert).
> ;; This saves time especially for deep trees or with ange-ftp.
> ;; The user can type `g' easily, and it is more consistent with find-file.
> ;; But if SWITCHES are given they are probably different from the
> ;; buffer's old value, so call dired-sort-other, which does
> ;; revert the buffer. A pity we can't possibly do "Directory has
> ;; changed - refresh? " like find-file does.
>  
> (DIRNAME is a typo here BTW - the argument is actually called DIR-OR-LIST,
> and if it is a cons then the name is just its car.)
>  
> Note the rationale: If SWITCHES are not given, that is, if there was no
> expressed intention to change the content of the listing, then just
> reuse an existing buffer.  But if such an intention was expressed, then
> respect it.
>  
> Following that rationale, the behavior for a cons arg should be to
> respect the explicit file list passed, not to simply ignore it and reuse
> an existing buffer.
>  
> And since it would be problematic to test the file list for differences
> from the existing listing (whether or not it came from a cons arg
> previously), we should always just update the buffer to reflect the
> file-list that is passed.  IOW, with an explicit file list, never reuse
> an existing Dired buffer of the same name.
IMO, if `dired-directory' is a cons and DIR-OR-LIST is a string then we
must update the buffer as well: in this case, the user want to see the
full listing of that directory.

I propose the following patch:
--8<-----------------------------cut here---------------start------------->8---
commit a77e2a01ad3f903d877f2c71c31ed33b7bf9540c
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Tue Jul 18 20:33:36 2017 +0900

    dired: Revert buffer when DIRNAME is a cons
    
    * lisp/dired.el (dired-internal-noselect): Revert buffer if DIR-OR-LIST
    is a cons, or dired-directory is a cons and DIR-OR-LIST a string (Bug#7131).
    Update the comments.

diff --git a/lisp/dired.el b/lisp/dired.el
index 4fb4fe78f8..9d500a9f52 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -872,13 +872,15 @@ dired-auto-revert-buffer
   :version "23.2")
 
 (defun dired-internal-noselect (dir-or-list &optional switches mode)
-  ;; If there is an existing dired buffer for DIRNAME, just leave
-  ;; buffer as it is (don't even call dired-revert).
+  ;; If DIR-OR-LIST is a string and there is an existing dired buffer
+  ;; for it, just leave buffer as it is (don't even call dired-revert).
   ;; This saves time especially for deep trees or with ange-ftp.
   ;; The user can type `g' easily, and it is more consistent with find-file.
   ;; But if SWITCHES are given they are probably different from the
   ;; buffer's old value, so call dired-sort-other, which does
   ;; revert the buffer.
+  ;; Revert the buffer if DIR-OR-LIST is a cons or `dired-directory'
+  ;; is a cons and DIR-OR-LIST is a string.
   ;; A pity we can't possibly do "Directory has changed - refresh? "
   ;; like find-file does.
   ;; Optional argument MODE is passed to dired-find-buffer-nocreate,
@@ -898,6 +900,11 @@ dired-internal-noselect
 	       (setq dired-directory dir-or-list)
 	       ;; this calls dired-revert
 	       (dired-sort-other switches))
+	      ;; Always revert when `dir-or-list' is a cons.  Also revert
+	      ;; if `dired-directory' is a cons but `dir-or-list' is not.
+	      ((or (consp dir-or-list) (consp dired-directory))
+	       (setq dired-directory dir-or-list)
+	       (revert-buffer))
 	      ;; Always revert regardless of whether it has changed or not.
 	      ((eq dired-auto-revert-buffer t)
 	       (revert-buffer))

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-07-18
Repository revision: a2ee81911bdf0f37b992989a9d36bb4d2ba14052





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

* bug#7131: 24.0.50; `dired' with cons arg should not ignore arg if buffer exists
  2017-07-18 11:39 ` Tino Calancha
@ 2017-07-21  4:36   ` Tino Calancha
  0 siblings, 0 replies; 3+ messages in thread
From: Tino Calancha @ 2017-07-21  4:36 UTC (permalink / raw)
  To: 7131-done

  
>> Following that rationale, the behavior for a cons arg should be to
>> respect the explicit file list passed, not to simply ignore it and reuse
>> an existing buffer.
>>  
>> And since it would be problematic to test the file list for differences
>> from the existing listing (whether or not it came from a cons arg
>> previously), we should always just update the buffer to reflect the
>> file-list that is passed.  IOW, with an explicit file list, never reuse
>> an existing Dired buffer of the same name.
> IMO, if `dired-directory' is a cons and DIR-OR-LIST is a string then we
> must update the buffer as well: in this case, the user want to see the
> full listing of that directory.
Fixed in master branch as commit 1d559e384b467b3f74e8b78695f124b561c884d9





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

end of thread, other threads:[~2017-07-21  4:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-29 15:34 bug#7131: 24.0.50; `dired' with cons arg should not ignore arg if buffer exists Drew Adams
2017-07-18 11:39 ` Tino Calancha
2017-07-21  4:36   ` Tino Calancha

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