bug#17218: 24.3; Dired does not insert subdirs when used with switch "--group-directories-first"

bug#17218: 24.3; Dired does not insert subdirs when used with switch "--group-directories-first"

[Tu Do]

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

If I remove the switch "--group-directories-first", then Bookmark Jump
works as expected. However, as long as the switch is put in,
sub-directories won't be inserted again.

I found out the problem in the function "dired-insert-old-subdirs" at line
1367 in dired.el. In this function, it has an or:

(defun dired-insert-old-subdirs (old-subdir-alist)
"Try to insert all subdirs that were displayed before.
Do so according to the former subdir alist OLD-SUBDIR-ALIST."
(or (string-match "R" dired-actual-switches)
(let (elt dir)
(while old-subdir-alist
(setq elt (car old-subdir-alist)
old-subdir-alist (cdr old-subdir-alist)
dir (car elt))
(condition-case ()
(progn
(dired-uncache dir)
(dired-insert-subdir dir))
(error nil))))))


The functional call to string-match:

(string-match "R" dired-actual-switches)

is the root cause. If dired-actual-switches is "-lha", then string-match
returns nil, and the code for inserting subdirs is going to be executed.
However, with dired-actual-switches is "--group-directories-first -lha",
then string-match returns 3, making the or call returns true immediately
and subdirs are never inserted.

"ls -R" recursively list directories. But matching it this way is
incorrect, since we have listing switches that contain character "R" in
it (i.e. --group-directories-first).

Hope that help.

Regards,

Tu.


In GNU Emacs 24.3.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.6.4)
of 2014-04-01 on oldfriend
Windowing system distributor `The X.Org Foundation', version 11.0.11303000
System Description: Ubuntu 13.04

Configured using:
`configure '--with-x-toolkit=gtk''

Important settings:
value of $LC_MONETARY: en_US.UTF-8
value of $LC_NUMERIC: en_US.UTF-8
value of $LC_TIME: en_US.UTF-8
value of $LANG: en_US.UTF-8
value of $XMODIFIERS: @im=ibus
locale-coding-system: utf-8-unix
default enable-multibyte-characters: t

Major mode: Bookmark Menu

Minor modes in effect:
tooltip-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
buffer-read-only: t
line-number-mode: t
transient-mark-mode: t

Recent input:
C-v M-v M-v C-x k <return> e C-c C-c C-x o C-n C-n
C-n C-p k x C-x 1 C-x C-g C-h v d i r e d - l i s <tab>
<return> C-s c u s C-g C-x o C-s c u s C-f <return>
C-n C-n C-n C-n C-n C-n C-n C-n C-e SPC - - g r o u
p - d i r e c t o r i e s - f i r s t C-n C-a C-f C-f
C-f C-f <return> 0 q q C-x o C-x d . e m <tab> . d
<tab> <return> C-n C-n C-p i k C-n C-n C-n C-v M-v
C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n M-v
M-v M-v M-v C-v C-v C-v C-v C-p C-p C-p C-p C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-n C-n
C-n C-n C-n C-n C-n C-x 1 C-x r l C-x b <return> C-x
r m <return> C-x C-g C-x b <return> <return> C-v C-v
C-x k <return> <return> C-v C-v C-v M-v M-v M-v C-x
k <return> M-x e m <tab> a <tab> b u <tab> <backspace>
<backspace> r <tab> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> b u
g <tab> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
r e <tab> <tab> <backspace> <backspace> e m a <tab>
<tab> <tab> <backspace> <backspace> <backspace> <backspace>
<backspace> M-x M-x r e p o <tab> t <tab> <backspace>
r <tab> e <backspace> <return>

Recent messages:
Resetting customization items...done
Creating customization setup...done
Mark set
Killed 0 lines.
scroll-up-command: End of buffer [2 times]
scroll-down-command: Beginning of buffer [2 times]
Making completion list... [3 times]
completing-read-default: Command attempted to use minibuffer while in
minibuffer
Quit
Making completion list...

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils cus-edit cus-start cus-load wid-edit misearch
multi-isearch bookmark pp dired-aux help-mode easymenu help-fns dired
time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face macroexp files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty emacs)

[-- Attachment #2: Type: text/html, Size: 6505 bytes --]

bug#17218: 24.3; Dired does not insert subdirs when used with switch "--group-directories-first"

[Drew Adams]

> I found out the problem in the function "dired-insert-old-subdirs"
> at line 1367 in dired.el.

There is a similar problem in `dired-sort-R-check' and
`dired-build-subdir-alist'.

[There probably should be a function that checks for a given switch, and does not just use `string-match-p' in such a simple way.  (At least it should respect case etc.)]

bug#17218: 24.3; Dired does not insert subdirs when used with switch "--group-directories-first"

[Juri Linkov]

>> I found out the problem in the function "dired-insert-old-subdirs"
>> at line 1367 in dired.el.
>
> There is a similar problem in `dired-sort-R-check' and
> `dired-build-subdir-alist'.
>
> [There probably should be a function that checks for a given switch,
> and does not just use `string-match-p' in such a simple way.
> (At least it should respect case etc.)]

Dired could use something like `ange-ftp-switches-ok' that implements
a better check for the -R switch.

bug#17218: 24.3; Dired does not insert subdirs when used with switch "--group-directories-first"

[Juri Linkov]

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

>> I found out the problem in the function "dired-insert-old-subdirs"
>> at line 1367 in dired.el.
>
> There is a similar problem in `dired-sort-R-check' and
> `dired-build-subdir-alist'.
>
> [There probably should be a function that checks for a given switch,
> and does not just use `string-match-p' in such a simple way.
> (At least it should respect case etc.)]

Below is the implementation of such a function:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2777 bytes --]

=== modified file 'lisp/dired.el'
--- lisp/dired.el	2014-04-22 18:17:17 +0000
+++ lisp/dired.el	2014-04-27 08:28:10 +0000
@@ -1142,10 +1142,20 @@ (defun dired-align-file (beg end)
 
 (defvar ls-lisp-use-insert-directory-program)
 
+(defun dired-switches-check (switches long short)
+  "Return non-nil if the string SWITCHES matches LONG or SHORT format."
+  (let (case-fold-search)
+    (string-match-p (concat "\\(\\`\\| \\)-[[:alnum:]]*" short
+			    "\\|--" long "\\>") switches)))
+
 (defun dired-switches-escape-p (switches)
   "Return non-nil if the string SWITCHES contains -b or --escape."
   ;; Do not match things like "--block-size" that happen to contain "b".
-  (string-match-p "\\(\\`\\| \\)-[[:alnum:]]*b\\|--escape\\>" switches))
+  (dired-switches-check switches "escape" "b"))
+
+(defun dired-switches-recursive-p (switches)
+  "Return non-nil if the string SWITCHES contains -R or --recursive."
+  (dired-switches-check switches "recursive" "R"))
 
 (defun dired-insert-directory (dir switches &optional file-list wildcard hdr)
   "Insert a directory listing of DIR, Dired style.
@@ -1402,7 +1412,7 @@ (defun dired-remember-hidden ()
 (defun dired-insert-old-subdirs (old-subdir-alist)
   "Try to insert all subdirs that were displayed before.
 Do so according to the former subdir alist OLD-SUBDIR-ALIST."
-  (or (string-match-p "R" dired-actual-switches)
+  (or (dired-switches-recursive-p dired-actual-switches)
       (let (elt dir)
 	(while old-subdir-alist
 	  (setq elt (car old-subdir-alist)
@@ -2613,7 +2623,7 @@ (defun dired-build-subdir-alist (&option
 	   (R-ftp-base-dir-regex
 	    ;; Used to expand subdirectory names correctly in recursive
 	    ;; ange-ftp listings.
-	    (and (string-match-p "R" switches)
+	    (and (dired-switches-recursive-p switches)
 		 (string-match "\\`/.*:\\(/.*\\)" default-directory)
 		 (concat "\\`" (match-string 1 default-directory)))))
       (goto-char (point-min))
@@ -3688,12 +3698,12 @@ (defun dired-sort-R-check (switches)
 minus any directories explicitly deleted when R is cleared.
 To be called first in body of `dired-sort-other', etc."
   (cond
-   ((and (string-match-p "R" switches)
-	 (not (string-match-p "R" dired-actual-switches)))
+   ((and (dired-switches-recursive-p switches)
+	 (not (dired-switches-recursive-p dired-actual-switches)))
     ;; Adding -R to ls switches -- save `dired-subdir-alist':
     (setq dired-subdir-alist-pre-R dired-subdir-alist))
-   ((and (string-match-p "R" dired-actual-switches)
-	 (not (string-match-p "R" switches)))
+   ((and (dired-switches-recursive-p dired-actual-switches)
+	 (not (dired-switches-recursive-p switches)))
     ;; Deleting -R from ls switches -- revert to pre-R subdirs
     ;; that are still present:
     (setq dired-subdir-alist

bug#17218: 24.3; Dired does not insert subdirs when used with switch "--group-directories-first"

[Juri Linkov]

Version: 24.5

>>> I found out the problem in the function "dired-insert-old-subdirs"
>>> at line 1367 in dired.el.
>>
>> There is a similar problem in `dired-sort-R-check' and
>> `dired-build-subdir-alist'.
>>
>> [There probably should be a function that checks for a given switch,
>> and does not just use `string-match-p' in such a simple way.
>> (At least it should respect case etc.)]
>
> Below is the implementation of such a function:

Installed to trunk since this is not a regression.

bug#17218: 24.3; Dired does not insert subdirs when used with switch "--group-directories-first"

[Glenn Morris]

Juri Linkov wrote:

> Installed to trunk since this is not a regression.

Note that emacs-24 is not yet in "regression fix only" mode
(and likely won't be for some time AFAICS).

bug#17218: 24.3; Dired does not insert subdirs when used with switch "--group-directories-first"

[Juri Linkov]

>> Installed to trunk since this is not a regression.
>
> Note that emacs-24 is not yet in "regression fix only" mode
> (and likely won't be for some time AFAICS).

Thanks for the clarification.  Then perhaps this change could be
backported to emacs-24.

bug#17218: 24.3; Dired does not insert subdirs when used with switch "--group-directories-first"

[Glenn Morris]

Juri Linkov wrote:

> Then perhaps this change could be backported to emacs-24.

I think it should be, yes.