bug#44023: dired-allow-duplicate-buffers

bug#44023: dired-allow-duplicate-buffers

[Boruch Baum]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

The attached patch adds a defcustom option to allow multiple buffers to
visit the same directory. This became desirable to me for an extension I
wrote (soon to be submitted) to store each chain of dired buffers'
'navigation history' and allow going back/forward to any point in the
history. As you all know, this has been a feature common to all file
*managers* since almost forever; technically dired is a directory
editor, not a file manager, but I expect many will find the feature
desirable, even expected, since all other similar programs have it.

In going through the existing dired code, it seems that some functions
presume that multiple buffers can be visiting the same directory. See:

  dired-find-buffer-nocreate
  dired-fun-in-all-buffers
  dired-clean-up-after-deletion

Also, implicit in the comment for `dired-find-buffer-nocreate' is that
more than one match may exist for a dired directory buffer.

So, maybe this once existed or was planned?

The first patch is for the option. The second patch is a change of
comments to docstrings.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[-- Attachment #2: 1.patch --]
[-- Type: text/x-diff, Size: 1669 bytes --]

diff --git a/NEWS b/NEWS
index a76447d..ca7f31b 100644
--- a/NEWS
+++ b/NEWS
@@ -284,6 +284,10 @@ time zones will use a form like "+0100" instead of "CET".

 ** Dired

++++
+*** New user option: 'dired-allow-duplicate-buffers'
+If set, allows multiple dired buffers to visit the same directory.
+
 ---
 *** Broken and circular links are shown with the 'dired-broken-symlink' face.

diff --git a/dired.el b/dired.el
index 7839ace..36c2f17 100644
--- a/dired.el
+++ b/dired.el
@@ -353,6 +353,12 @@ is anywhere on its Dired line, except the beginning of the line."
   :group 'dired
   :version "28.1")

+(defcustom dired-allow-duplicate-buffers t
+  "Allow multiple `dired' buffers to visit the same directory."
+  :type 'boolean
+  :group 'dired
+  :version "28.1")
+
 ;; Internal variables

 (defvar dired-marker-char ?*		; the answer is 42
@@ -1152,9 +1158,11 @@ The return value is the target column for the file names."
   ;; see there.
   (let* ((old-buf (current-buffer))
 	 (dirname (if (consp dir-or-list) (car dir-or-list) dir-or-list))
-         ;; Look for an existing buffer.
-         (buffer (dired-find-buffer-nocreate dirname mode))
-	 ;; Note that buffer already is in dired-mode, if found.
+         (buffer (when (not (bound-and-true-p dired-allow-duplicate-buffers))
+                   ;; Look for an existing buffer.
+                   (dired-find-buffer-nocreate dirname mode)
+                   ;; Note that buffer already is in dired-mode, if found.
+                   ))
 	 (new-buffer-p (null buffer)))
     (or buffer
         (setq buffer (create-file-buffer (directory-file-name dirname))))

[-- Attachment #3: 2.patch --]
[-- Type: text/x-diff, Size: 3573 bytes --]

diff --git a/dired.el b/dired.el
index 36c2f17..2ecd6bd 100644
--- a/dired.el
+++ b/dired.el
@@ -1143,19 +1143,21 @@ The return value is the target column for the file names."
             (forward-line)))))))

 (defun dired-internal-noselect (dir-or-list &optional switches mode)
-  ;; 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,
-  ;; see there.
+  "Internal function for `dired-noselect'.
+
+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',
+see there."
   (let* ((old-buf (current-buffer))
 	 (dirname (if (consp dir-or-list) (car dir-or-list) dir-or-list))
          (buffer (when (not (bound-and-true-p dired-allow-duplicate-buffers))
@@ -1219,20 +1221,24 @@ The return value is the target column for the file names."
     buffer))

 (defvar dired-buffers nil
-  ;; Enlarged by dired-advertise
-  ;; Queried by function dired-buffers-for-dir. When this detects a
-  ;; killed buffer, it is removed from this list.
-  "Alist of expanded directories and their associated Dired buffers.")
+  "Alist of expanded directories and their associated Dired buffers.
+The list is enlarged by `dired-advertise'. Function
+`dired-buffers-for-dir' queries this list and removes killed
+buffers from it, when detected.")

 (defvar dired-find-subdir)
+;; `dired-find-subdir' is defined in `dired-x.el'

 ;; FIXME add a doc-string, and document dired-x extensions.
 (defun dired-find-buffer-nocreate (dirname &optional mode)
-  ;; This differs from dired-buffers-for-dir in that it does not consider
-  ;; subdirs of default-directory and searches for the first match only.
-  ;; Also, the major mode must be MODE.
-  (if (and (featurep 'dired-x)
-           dired-find-subdir
+  "Internal function for `dired-internal-noselect'.
+Searches among buffers of major-mode MODE (default `dired-mode')
+for one visiting DIRNAME, and returns the first one found. This
+differs from `dired-buffers-for-dir' in that it does not consider
+subdirs of `default-directory', it searches only for the first
+match, and it can restrict its search to buffers of
+major-mode MODE."
+  (if (and (featurep 'dired-x) dired-find-subdir
            ;; Don't try to find a wildcard as a subdirectory.
 	   (string-equal dirname (file-name-directory dirname)))
       (let* ((cur-buf (current-buffer))

bug#44023: dired-allow-duplicate-buffers

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> Also, implicit in the comment for `dired-find-buffer-nocreate' is that
> more than one match may exist for a dired directory buffer.
>
> So, maybe this once existed or was planned?

Dired has always supported having several buffers visiting the same
directory, so I'm not quite sure what the proposed patch is trying to
fix.  You just rename the buffer to something else, like with any
command that defaults to a single buffer name (like, say, `M-x grep').

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44023: dired-allow-duplicate-buffers

[Boruch Baum]

On 2020-10-16 07:32, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> > Also, implicit in the comment for `dired-find-buffer-nocreate' is that
> > more than one match may exist for a dired directory buffer.
> >
> > So, maybe this once existed or was planned?
>
> Dired has always supported having several buffers visiting the same
> directory, so I'm not quite sure what the proposed patch is trying to
> fix.  You just rename the buffer to something else, like with any
> command that defaults to a single buffer name (like, say, `M-x grep').

That's not "dired support" in any shape or form! That's an emacs kludge
external to dired that exploits how dired handles buffer names (likewise
for grep). Dired internally (in the very line being modified by my
patch) actively subverts having several buffers visiting the same
directory, and my experience in testing is that no harm is done when
using the patch.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#44023: dired-allow-duplicate-buffers

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

> Dired internally (in the very line being modified by my
> patch) actively subverts having several buffers visiting the same
> directory,

What line are you referring to?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44023: dired-allow-duplicate-buffers

[Boruch Baum]

On 2020-10-16 18:25, Lars Ingebrigtsen wrote:
> Boruch Baum <boruch_baum@gmx.com> writes:
>
> > Dired internally (in the very line being modified by my
> > patch) actively subverts having several buffers visiting the same
> > directory,
>
> What line are you referring to?

-         (buffer (dired-find-buffer-nocreate dirname mode))

It's the only line changed in the patch file, at the beginning of
function dired-internal-noselect.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#44023: dired-allow-duplicate-buffers

[Lars Ingebrigtsen]

Boruch Baum <boruch_baum@gmx.com> writes:

>> > Dired internally (in the very line being modified by my
>> > patch) actively subverts having several buffers visiting the same
>> > directory,
>>
>> What line are you referring to?
>
> -         (buffer (dired-find-buffer-nocreate dirname mode))
>
> It's the only line changed in the patch file, at the beginning of
> function dired-internal-noselect.

Ah, yes, it seems like dired is working the same as when visiting a
regular file here.  As find-file says:

---
Switch to a buffer visiting file FILENAME,
creating one if none already exists.
---

Dired buffers aren't "visiting" files in the same way that regular files
are, but it tries to keep the same semantics.

So renaming a dired buffer doesn't help at all, because Emacs will
always root out the already-existing buffer that points to that
directory.  I wonder whether that's on purpose, or whether it just
happens that way by accident.  Or is there a way to rename a dired
buffer so that you can have two of them pointing to the same directory?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44023: dired-allow-duplicate-buffers

[Boruch Baum]

On 2020-10-19 10:11, Lars Ingebrigtsen wrote:
> Ah, yes
> ...
> I wonder
> ...
> is there a way to rename a dired buffer so that you can have two of
> them pointing to the same directory?

The patch does it without any need to explicitly rename anything, It all
happens blissfully auto-magically.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#44023: dired-allow-duplicate-buffers

[Stefan Monnier]

> In going through the existing dired code, it seems that some functions
> presume that multiple buffers can be visiting the same directory. See:

Because of the functionality of Dired (which allows visiting several
directories (and subdirectories)) in a single buffer, the code basically
"has to" handle that case, indeed (tho I'd be surprised if there aren't
improvements to be made in that regard).

> +(defcustom dired-allow-duplicate-buffers t
[...]
> -         ;; Look for an existing buffer.
> -         (buffer (dired-find-buffer-nocreate dirname mode))
> -	 ;; Note that buffer already is in dired-mode, if found.
> +         (buffer (when (not (bound-and-true-p dired-allow-duplicate-buffers))
> +                   ;; Look for an existing buffer.
> +                   (dired-find-buffer-nocreate dirname mode)
> +                   ;; Note that buffer already is in dired-mode, if found.
> +                   ))

[ The ";; Note ..." comment seems to be misplaced.  ]

I wonder if it's better to add a user-config variable or a new argument
to `dired-internal-noselect`?

E.g. I suspect that diredc wants/needs this feature, but it's not
diredc's job to set user options, so for diredc it would probably make
more sense to pass that as an argument.
WDYT?


        Stefan

bug#44023: dired-allow-duplicate-buffers

[Stefan Monnier]

> I wonder if it's better to add a user-config variable or a new argument
> to `dired-internal-noselect`?

Or a new function/command.


        Stefan

bug#44023: dired-allow-duplicate-buffers

[Boruch Baum]

On 2020-11-05 10:38, Stefan Monnier wrote:
> > In going through the existing dired code, it seems that some functions
> > presume that multiple buffers can be visiting the same directory. See:
>
> Because of the functionality of Dired (which allows visiting several
> directories (and subdirectories)) in a single buffer, the code basically
> "has to" handle that case, indeed (tho I'd be surprised if there aren't
> improvements to be made in that regard).

The case you mention sounds like the converse: multiple directories in a
single buffer, versus what I'm addressing which is multiple buffers for
a single directory.

> > +(defcustom dired-allow-duplicate-buffers t
> I wonder if it's better to add a user-config variable or a new argument
> to `dired-internal-noselect`?
>
> E.g. I suspect that diredc wants/needs this feature, but it's not
> diredc's job to set user options, so for diredc it would probably make
> more sense to pass that as an argument.
> WDYT?

I prefer the defcustom route because I like giving a user choices (it's
also more emacs-y). Less subjectively, adding an argument creates a mess
because the modified function is a generic internal function so the
argument would either need to be added to all its potential parents, or
diredc would need to advice or replace those parents (which just
replaces a single messy advice with several messy advices).

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#44023: dired-allow-duplicate-buffers

[Stefan Monnier]

>> > In going through the existing dired code, it seems that some functions
>> > presume that multiple buffers can be visiting the same directory. See:
>> Because of the functionality of Dired (which allows visiting several
>> directories (and subdirectories)) in a single buffer, the code basically
>> "has to" handle that case, indeed (tho I'd be surprised if there aren't
>> improvements to be made in that regard).
> The case you mention sounds like the converse: multiple directories in a
> single buffer, versus what I'm addressing which is multiple buffers for
> a single directory.

Yes, but:
start with a dired in ~/foo and another in ~/foo/bar and then in ~/foo
ask dired to add the contents of ~/foo/bar to it: pronto you have
2 dired buffers that both show the content of ~/foo/bar.  So because of
that kind of scenario, dired has to handle some kind of "two dired
buffers both showing the same directory" (tho it's not exactly the same
as your scenario, admittedly).

>> E.g. I suspect that diredc wants/needs this feature, but it's not
>> diredc's job to set user options, so for diredc it would probably make
>> more sense to pass that as an argument.
>> WDYT?
>
> I prefer the defcustom route because I like giving a user choices (it's
> also more emacs-y).

I don't see this as a question of "choice vs no-choice" but a question
is: should this be decided globally over the whole Emacs session, or
should it be decided one  a case-by-case basis (in both cases the
decision should be under the user's control, either by setting
a variable, or by calling a different command, or by using the same
command bu tin a different context, ...).

> Less subjectively, adding an argument creates a mess
> because the modified function is a generic internal function

Which is why I then suggested a separate function/command (which will
require a bit of refactoring to share the common code, of course, but
that's an implementation detail).


        Stefan

bug#44023: dired-allow-duplicate-buffers

[Boruch Baum]

On 2020-11-06 09:29, Stefan Monnier wrote:
> >> E.g. I suspect that diredc wants/needs this feature, but it's not
> >> diredc's job to set user options, so for diredc it would probably make
> >> more sense to pass that as an argument.
> >> WDYT?
> >
> > I prefer the defcustom route because I like giving a user choices (it's
> > also more emacs-y).
>
> I don't see this as a question of "choice vs no-choice" but a question
> is: should this be decided globally over the whole Emacs session,

Yes, yes.

> or should it be decided one a case-by-case basis

If you mean buffer-by-buffer, no, no, no. Think of how tedious that
would be to need to manually set that for each navigation. Once a person
sets a preference, it should become the default until changed.

> > Less subjectively, adding an argument creates a mess
> > because the modified function is a generic internal function
>
> Which is why I then suggested a separate function/command (which will
> require a bit of refactoring to share the common code, of course, but
> that's an implementation detail).

Another way of saying 'more work that isn't really necessary at all'? If
you're asking to choose between: 1] a clean single line change and
boolean defcustom, and; 2] "a separate function/command (which will
require a bit of refactoring to share the common code, of course" ... uh
... I think I choose ... uh ... uh ...

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

bug#44023: dired-allow-duplicate-buffers

[Stefan Monnier]

>> I don't see this as a question of "choice vs no-choice" but a question
>> is: should this be decided globally over the whole Emacs session,
>
> Yes, yes.
>
>> or should it be decided one a case-by-case basis
>
> If you mean buffer-by-buffer, no, no, no. Think of how tedious that
> would be to need to manually set that for each navigation. Once a person
> sets a preference, it should become the default until changed.

That's the point I was asking: is it a user-preference, so the users
will either always want one or always want the other, or is it
a behavior that users will want in some cases and will not want in other
cases.  E.g. imagine users that use both "plain dired" sometimes and
diredc at other times: would they want to allow "duplicate dired" when
using "plain dired", or is it only desirable when used from diredc?

So maybe, the users should not even need to set the option a single time
and the behavior will simply depend on whether dired is invoked directly
or from diredc.

> Another way of saying 'more work that isn't really necessary at all'?

OK.  I'm not arguing either way, I was just raising the question because
my poor intuition got the impression that users might prefer not to set
any option at all and that the right behavior could be inferred
from context.


        Stefan