bug#81295: 32.0.50; dired-auto-toggle-b-switch problem with subdirectory

TL;DR for Eli and Sean: Should one of the attached patches for this
bug be installed in master now, or should we wait for Filipp's
alternative fix?  (Read on for more context.)

On Mon, 27 Jul 2026 16:36:28 +0300 Filipp Gunbin <[email protected]> wrote:

> On 26/07/2026 18:05 +0200, Stephen Berman wrote:
>
>> On Mon, 20 Jul 2026 17:14:38 +0300 Filipp Gunbin <[email protected]> wrote:
>>
>>> On 18/07/2026 12:01 +0200, Stephen Berman wrote:
>>>
>>>> On Sat, 18 Jul 2026 03:22:22 +0300 Filipp Gunbin <[email protected]> wrote:
>>>>
>>>>> On 17/07/2026 16:08 +0200, Stephen Berman wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>> - Typing `C-u i RET' calls `dired-maybe-insert-subdir' with argument
>>>>>>   SWITCHES set to the default value "-al".  This gets passed to
>>>>>>   `dired-insert-subdir', which in turn passes it to
>>>>>>   `dired-insert-subdir-validate', which return `(t t)' because neither
>>>>>>   SWITCHES nor `dired-actual-switches' includes either the "F" or "b"
>>>>>>   switch.  Hence, directory 2 gets inserted as a subdir.  Before
>>>>>>   `dired-insert-subdir' returns, the value of SWITCHES gets added to
>>>>>>   `dired-switches-alist'.
>>>>>>
>>>>>> - When `dired-insert-subdir' returns, `dired--toggle-b-switch' gets
>>>>>>   called from `post-command-hook' (since `dired-auto-toggle-b-switch' is
>>>>>>   t), and since directory 2 contains a file name with a newline
>>>>>>   `(dired--filename-with-newline-p)' returns t, and this results in the
>>>>>>   "b" switch getting added to `dired-actual-switches' and then
>>>>>>   `dired-revert' is called.
>>>>>
>>>>> I think the problem is that dired--toggle-b-switch only modifies
>>>>> dired-actual-switches, while it should also modify dired-switches-alist.
>>>>> Otherwise they go out of sync, may conflict, and lead to problematic
>>>>> cases like this.
>>>>>
>>>>> From your description it looks like that would naturally solve the
>>>>> current problem.
>>>>
>>>> I don't see a way to make `dired--toggle-b-switch' modify
>>>> `dired-switches-alist' that isn't much more complex than the solution I
>>>> proposed in patch 1.  If you do, can you post a patch?
>>>
>>> Alas, currently I don't have enough time, maybe later.  I've tried to
>>> explain my thoughts in more detail below.
>>>
>>>>> But it would also mean that any switches that user enters in C-u i
>>>>> should be modified with respect to dired-auto-toggle-b-switch.  It's
>>>>> "auto-toggle", after all.
>>>>
>>>> Right, that's what patch 1 does.
>>>
>>> I don't like this change:
>>>
>>> -    (and (not switches) cons (setq switches (cdr cons)))
>>> +    (and (not switches) (not (dired-switches-escape-p dired-actual-switches))
>>> +         cons (setq switches (cdr cons)))
>>>
>>> because it makes a non-obvious special case.  Currently it is: unless
>>> the switches are specified, take them from dired-switches-alist.  The
>>> patch makes an exclusion, a non-obvious one.
>>>
>>> The very fact that you're trying to escape validation doesn't make it
>>> better either: the validation is there for a reason, right?
>>
>> The validation is to prevent the situation where the Dired buffer uses
>> one or both of the -b or -F switches while the inserted subdirs do not,
>> or vice versa.  Your case demonstrates that validation is problematic
>> precisely when the -b switch gets added by dired--toggle-b-switch, so
>> circumventing validation in this case is a simple and effective way of
>> avoiding the problem, given the current handling of file names with
>> newlines.  No doubt a more intuitively appealing solution is possible,
>> but my impression is it would require fairly involved changes in the
>> current handling of file names with newlines.
>
> I think the constraints which validation imposes are reasonable, so I
> wouldn't want to circumvent it here.  Letting different combinations of
> -b/-F now will make it harder to do things "the right way" later.
>
> That said, I don't know the exact reasons why the validations were added
> originally.  But we should either keep them (and don't try to
> circumvent), or remove them altogether.
>
>>>>> And this brings one more thing.  It seems like you use
>>>>> dired-listing-switches as "the original" switches, for example in
>>>>> dired-internal-noselect:
>>>>>
>>>>>     (unless (or dired-auto-toggle-b-switch
>>>>>                 (dired-switches-escape-p dired-listing-switches)
>>>>>                 (dired-switches-escape-p dired-actual-switches))
>>>>>
>>>>> But C-u C-x d will ask you for your custom switches, and
>>>>> dired-listing-switches is only initial input there.  There's also code
>>>>> in dired--toggle-b-switch which uses dired-listing-switches in this way:
>>>>>
>>>>>       (unless (dired-switches-escape-p dired-listing-switches)
>>>>>         (when (dired-switches-escape-p dired-actual-switches)
>>>>>           (setq dired-actual-switches (dired--remove-b-switch))
>>>>>           (dired-revert))))))
>>>>>
>>>>> Perhaps we should have buffer-local variables which record the initial
>>>>> top-level and subdirectory switches, and consult them when deciding what
>>>>> to do in dired--toggle-b-switch.
>>>>
>>>> I don't follow you here; AFAIU the user option `dired-listing-switches'
>>>> can be overriden by `C-u C-x d', and the resulting switches are then the
>>>> value of `dired-actual-switches', otherwise, `dired-listing-switches'
>>>> and `dired-actual-switches' have the same value (unless something like
>>>> `dired--toggle-b-switch' changes the latter).  Can you elaborate on what
>>>> you're proposing (or considering) for `dired--toggle-b-switch'?
>>>
>>> "Then" branch currently adds the "-b" switch when we have newlines in
>>> files, and when the user option is enabled:
>>>
>>>     (if (and (dired--filename-with-newline-p) dired-auto-toggle-b-switch)
>>>         (unless (dired-switches-escape-p dired-actual-switches)
>>>           (setq dired-actual-switches (concat dired-actual-switches " -b"))
>>>           (dired-revert))
>>>
>>> But the logic in "else" branch is, I think, incomplete.
>>>
>>> Here we're checking that user hasn't specified -b explicitly by testing
>>> dired-listing-switches, and this doesn't account for the case of C-u C-x
>>> d, because dired-listing-switches then have nothing to do with the
>>> user's choice:
>>>
>>>       (unless (dired-switches-escape-p dired-listing-switches)
>>>         (when (dired-switches-escape-p dired-actual-switches)
>>>           (setq dired-actual-switches (dired--remove-b-switch))
>>>           (dired-revert))))))
>>>
>>> So I was thinking what the root cause of the problem is, and it appeared
>>> to me that we don't have enough variables currently:
>>>
>>> Top-level switches:
>>>
>>> (1) defcustom for default switches - dired-listing-switches
>>>
>>> (2) The value that user actually entered dired with: missing
>>
>> You mean with `C-u C-x d', right?
>
> Yes.
>
>> That value is assigned to dired-actual-switches in dired-mode:
>>
>> (setq-local dired-actual-switches
>>             (or switches (connection-local-value dired-listing-switches)))
>
> But with dired-auto-toggle-b-switch feature dired-actual-switches may be
> modified, and so we lose information about whether the user explicitly
> specified -b or not.  That's why I separated (2) and (3).
>
>>> (3) The value of actual ls invocation: dired-actual-switches
>>
>> As the code snippet from dired-mode shows, this is either (1) or (2).
>
> See above.
>
>>> Subdir switches:
>>>
>>> (4) defcustom for default switches - (or dired-subdir-switches
>>> dired-listing-switches)
>>>
>>> (5) The value that user actually entered dired with, per subdir:
>>> dired-subdir-switches alist
>>>
>>> (6) The value of actual ls invocation, per subdir: missing
>>
>> Here, too, it's dired-actual-switches, let-bound in
>> dired-insert-subdir-doinsert:
>>
>>       (let ((dired-actual-switches
>> 	     (or switches
>> 		 dired-subdir-switches
>> 		 (string-replace "R" "" dired-actual-switches))))
>
> This defaults to (adjusted) dired-actual-switches, yes, but otherwise
> the same reasoning as above applies.  The caller of this function,
> dired-insert-subdir, uses dired-switches-alist to store specific
> switches per subdir (5).
>
>>> The validation currently ensures that either all directories in dired
>>> have -b, or none.  In the current problematic case, we end up with:
>>>
>>> - "-alb" in dired-actual-switches.  This value is ok.
>>>
>>> - "-al" in dired-switches-alist.  This value is not ok, because we're
>>>   now in situtation which validation was trying to prevent.  IMO we
>>>   should have state for (6), and dired--toggle-b-switch should adjust it
>>>   by adding -b to dired-subdir-switches (5).  We should also store
>>>   dired-subdir-switches (5) as it is, as currently, because we need to
>>>   tell whether -b was present in initial user choice, to be able to
>>>   decide whether we need to remove it in "auto" case.
>>>
>>> Just trying to silence the validation IMO seems wrong.
>>>
>>> Not having the "full" state (current plus (2) and (6)) also makes for
>>> more moot points, like the extract from dired-internal-noselect I posted
>>> above.
>>>
>>> If no one currently can provide the full patch (if my logic is right, of
>>> course), then perhaps we better leave this unfixed for now.  You've
>>> fixed my original complaint in this bug, this is already an improvement.
>>
>> I probably need to see an implementation to really understand what
>> you're proposing.  But in the mean time, I think it would be better to
>> install the last patch I posted (with the change you don't like) than to
>> leave the `C-u i' case unhandled, because it's a serious problem (having
>> the subdir not appear at all), and the patch prevents that problem, even
>> if a better solution is possible.  We could certainly add a FIXME to the
>> code referring to this bug report, which can be left open.
>
> I still think I'm opposed to the fix, because 1) it surely will
> introduce more subtle bugs (I think I can come up with the cases
> easily),

Are you referring just to the change that skips validation or to the
entire change to dired-aux.el?  If the former, and you demonstrate that
it introduces bugs at least as serious as the one it fixes, then I
certainly have no objection to omitting that part of the patch.

>          2) it will be harder to clean up later.  A FIXME is IMO better
> for now, given that we don't have bug reports on this other than mine.
>
> I'll try to come up with an implementation of what I proposed, but
> later.

When you wrote above "perhaps we better leave this unfixed for now.
You've fixed my original complaint in this bug, this is already an
improvement.", it seemed clear you were referring just to the problem I
"fixed" by skipping validation.  But your proposal to add more
buffer-local variables would AFAIU also entail different changes than
the rest of what my patch for dired-aux.el contains; but without those
changes, newlines in filenames in Dired subdirs would be left unhandled,
and the change to `dired--filename-with-newline-p' would be useless.

It's not clear to me how to proceed at this point, so I'm asking the
maintainers for their opinion and will defer to that.  AFAIU the choice
is between my patch for handling newlines in filenames in Dired subdirs,
either including the case of `C-u i' (attached patch 1) or leaving this
case unfixed for now (attached patch 2), or waiting for your alternative
implementation for handling newlines in filenames in Dired subdirs.

Steve Berman

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 50287a67129..57bf0d5eb4f 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -3356,7 +3356,12 @@ dired-insert-subdir
 	(modflag (buffer-modified-p))
 	(old-switches switches)
 	switches-have-R mark-alist case-fold-search buffer-read-only)
-    (and (not switches) cons (setq switches (cdr cons)))
+    ;; If `dired-actual-switches' now includes "-b" due to
+    ;; `dired--toggle-b-switch', we don't want to override this by
+    ;; passing incompatible switches via `C-u i', so we skip validation
+    ;; in this case.  FIXME: Is there a better way?  See bug#81295.
+    (and (not switches) (not (dired-switches-escape-p dired-actual-switches))
+         cons (setq switches (cdr cons)))
     (dired-insert-subdir-validate dirname switches)
     ;; case-fold-search is nil now, so we can test for capital `R':
     (if (setq switches-have-R (and switches (string-match-p "R" switches)))
@@ -3368,6 +3373,16 @@ dired-insert-subdir
       (dired-insert-subdir-newpos dirname)) ; else compute new position
     (dired-insert-subdir-doupdate
      dirname elt (dired-insert-subdir-doinsert dirname switches))
+    ;; Since we insert subdirs without calling `dired-internal-noselect'
+    ;; we also have to test here whether the subdir contains a file name
+    ;; with a newline, and if so, pop up a warning.  FIXME: Is there a
+    ;; cleaner way to do this?  See bug#81295.
+    (unless (or dired-auto-toggle-b-switch
+                (dired-switches-escape-p dired-listing-switches)
+                (dired-switches-escape-p dired-actual-switches))
+      (when (and (dired--filename-with-newline-p)
+                 (dired--ls-accept-b-switch-p))
+        (dired--display-filename-with-newline-warning (current-buffer))))
     (when old-switches
       (if cons
 	  (setcdr cons switches)
diff --git a/lisp/dired.el b/lisp/dired.el
index 53ec6779061..a4bfd81b5b9 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -1487,7 +1487,9 @@ dired-internal-noselect
     ;; and `ls' can take the `b' switch.  We do this here in order to
     ;; get the warning not only when interactively invoking `dired' on a
     ;; directory, but also e.g. when passing the directory name as a
-    ;; command line argument when starting Emacs from the shell.
+    ;; command line argument when starting Emacs from the shell (but
+    ;; inserting subdirs does not call `dired-internal-noselect' so we
+    ;; also have to run this test in `dired-insert-subdir').
     (unless (or dired-auto-toggle-b-switch
                 (dired-switches-escape-p dired-listing-switches)
                 (dired-switches-escape-p dired-actual-switches))
@@ -4060,11 +4062,18 @@ dired-buffer-more-recently-used-p
        (not (memq buffer1 (memq buffer2 (buffer-list))))))
 
 (defun dired--filename-with-newline-p ()
-  "Check whether a file name in this directory has a newline.
-Return non-nil if at least one file name in this directory contains a
-newline character (regardless of whether Dired displays the character as
-a literal newline or as \"\\n\")."
-  (directory-files default-directory nil "\n"))
+  "Check whether a file name in this Dired buffer has a newline.
+Return non-nil if at least one file name in this Dired buffer (including
+any Dired subdirectories in the buffer) contains a newline character
+(regardless of whether Dired displays the character as a literal newline
+or as \"\\n\")."
+  (if (length> dired-subdir-alist 1)
+      (let (res)
+        (catch 'found
+          (dolist (subdir dired-subdir-alist res)
+            (setq res (directory-files (car subdir) nil "\n"))
+            (and res (throw 'found res)))))
+    (directory-files default-directory nil "\n")))
 
 (defun dired--ls-accept-b-switch-p ()
   "Return non-nil if the `ls' used by Dired accepts the `b' switch."

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 50287a67129..7a43e75d1a0 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -3368,6 +3368,16 @@ dired-insert-subdir
       (dired-insert-subdir-newpos dirname)) ; else compute new position
     (dired-insert-subdir-doupdate
      dirname elt (dired-insert-subdir-doinsert dirname switches))
+    ;; Since we insert subdirs without calling `dired-internal-noselect'
+    ;; we also have to test here whether the subdir contains a file name
+    ;; with a newline, and if so, pop up a warning.  FIXME: Is there a
+    ;; cleaner way to do this?  See bug#81295.
+    (unless (or dired-auto-toggle-b-switch
+                (dired-switches-escape-p dired-listing-switches)
+                (dired-switches-escape-p dired-actual-switches))
+      (when (and (dired--filename-with-newline-p)
+                 (dired--ls-accept-b-switch-p))
+        (dired--display-filename-with-newline-warning (current-buffer))))
     (when old-switches
       (if cons
 	  (setcdr cons switches)
diff --git a/lisp/dired.el b/lisp/dired.el
index 53ec6779061..a4bfd81b5b9 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -1487,7 +1487,9 @@ dired-internal-noselect
     ;; and `ls' can take the `b' switch.  We do this here in order to
     ;; get the warning not only when interactively invoking `dired' on a
     ;; directory, but also e.g. when passing the directory name as a
-    ;; command line argument when starting Emacs from the shell.
+    ;; command line argument when starting Emacs from the shell (but
+    ;; inserting subdirs does not call `dired-internal-noselect' so we
+    ;; also have to run this test in `dired-insert-subdir').
     (unless (or dired-auto-toggle-b-switch
                 (dired-switches-escape-p dired-listing-switches)
                 (dired-switches-escape-p dired-actual-switches))
@@ -4060,11 +4062,18 @@ dired-buffer-more-recently-used-p
        (not (memq buffer1 (memq buffer2 (buffer-list))))))
 
 (defun dired--filename-with-newline-p ()
-  "Check whether a file name in this directory has a newline.
-Return non-nil if at least one file name in this directory contains a
-newline character (regardless of whether Dired displays the character as
-a literal newline or as \"\\n\")."
-  (directory-files default-directory nil "\n"))
+  "Check whether a file name in this Dired buffer has a newline.
+Return non-nil if at least one file name in this Dired buffer (including
+any Dired subdirectories in the buffer) contains a newline character
+(regardless of whether Dired displays the character as a literal newline
+or as \"\\n\")."
+  (if (length> dired-subdir-alist 1)
+      (let (res)
+        (catch 'found
+          (dolist (subdir dired-subdir-alist res)
+            (setq res (directory-files (car subdir) nil "\n"))
+            (and res (throw 'found res)))))
+    (directory-files default-directory nil "\n")))
 
 (defun dired--ls-accept-b-switch-p ()
   "Return non-nil if the `ls' used by Dired accepts the `b' switch."