Dired subdirectories & the ls option time-style with the %R sequence

Dired subdirectories & the ls option time-style with the %R sequence

[Gautier Ponsinet]

Hello everyone,

I use dired with the variable `dired-listing-switches' containing the
--time-style option of ls (from the GNU coreutils), itself containing
the %R sequence. Precisely, I have:

(setq dired-listing-switches "-l -Fhv --group-directories-first \"--time-style=+%F %a %R\" ")

in my init.el file.

I have adapted the variable `directory-listing-before-filename-regexp'
accordingly and everything works fine, except when I insert a
subdirectory in dired using `dired-maybe-insert-subdir'. It seems that
the "%R" in the time-style option is interpreted as a -R option in
dired-insert-subdir and is removed (there is a (string-replace "R" ""
switches) somewhere in the function `dired-insert-subdir'). As a result,
the expected time is not printed and only a "%" appears, so my regexp
does not work and dired is confused.

Of course, I can also replace the %R by %H:%M in my config, which I do,
and I don't have this problem. Nevertheless, the bug seems there. It
took me some time to understand the problem, and I guess other people
might encounter the same problem.

Maybe `dired-insert-subdir' could search for "R" but exclude "%R"?

(emacs version: GNU Emacs 28.2 (build 1, x86_64-pc-linux-gnu, GTK+
 Version 3.24.36, cairo version 1.17.6) of 2023-01-03).

All the best,
Gautier

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Robert Pluim]

>>>>> On Wed, 24 May 2023 13:07:12 +0200, Gautier Ponsinet <gautier@gautierponsinet.xyz> said:

    Gautier> Hello everyone,
    Gautier> I use dired with the variable `dired-listing-switches' containing the
    Gautier> --time-style option of ls (from the GNU coreutils), itself containing
    Gautier> the %R sequence. Precisely, I have:

    Gautier> (setq dired-listing-switches "-l -Fhv --group-directories-first \"--time-style=+%F %a %R\" ")

    Gautier> in my init.el file.

    Gautier> I have adapted the variable `directory-listing-before-filename-regexp'
    Gautier> accordingly and everything works fine, except when I insert a
    Gautier> subdirectory in dired using `dired-maybe-insert-subdir'. It seems that
    Gautier> the "%R" in the time-style option is interpreted as a -R option in
    Gautier> dired-insert-subdir and is removed (there is a (string-replace "R" ""
    Gautier> switches) somewhere in the function `dired-insert-subdir'). As a result,
    Gautier> the expected time is not printed and only a "%" appears, so my regexp
    Gautier> does not work and dired is confused.

    Gautier> Of course, I can also replace the %R by %H:%M in my config, which I do,
    Gautier> and I don't have this problem. Nevertheless, the bug seems there. It
    Gautier> took me some time to understand the problem, and I guess other people
    Gautier> might encounter the same problem.

    Gautier> Maybe `dired-insert-subdir' could search for "R" but exclude "%R"?

Something like the following (untested) patch?

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index fc3f6f4f04d..dc558e13ac8 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -3142,7 +3142,7 @@ dired-insert-subdir
     (and (not 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)))
+    (if (setq switches-have-R (and switches (string-match-p "[^%]R" switches)))
 	;; avoid duplicated subdirs
 	(setq mark-alist (dired-kill-tree dirname t)))
     (if elt

Robert
-- 

Re: Dired subdirectories & the ls option time-style with the %R sequence

[gautier]

Hello Robert,

Robert Pluim <rpluim@gmail.com> (2023-05-24 13:36 +0200):
> Something like the following (untested) patch?
> 
> diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
> index fc3f6f4f04d..dc558e13ac8 100644
> --- a/lisp/dired-aux.el
> +++ b/lisp/dired-aux.el
> @@ -3142,7 +3142,7 @@ dired-insert-subdir
>      (and (not 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)))
> +    (if (setq switches-have-R (and switches (string-match-p "[^%]R" 
> switches)))
>  	;; avoid duplicated subdirs
>  	(setq mark-alist (dired-kill-tree dirname t)))
>      (if elt
> 
> Robert
> --

Thank you very much for the patch!

It seems that the exact same modification you did also need to be done 
in the
function: dired-insert-subdir-doinsert .

So, the following patch works for me:

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index a07406e4c0d..8d31efd4a1a 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -3142,7 +3142,7 @@ dired-insert-subdir
      (and (not 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)))
+    (if (setq switches-have-R (and switches (string-match-p "[^%]R" 
switches)))
  	;; avoid duplicated subdirs
  	(setq mark-alist (dired-kill-tree dirname t)))
      (if elt
@@ -3258,7 +3258,7 @@ dired-insert-subdir-doinsert
        (let ((dired-actual-switches
  	     (or switches
  		 dired-subdir-switches
-		 (string-replace "R" "" dired-actual-switches))))
+		 (string-replace "[^%]R" "" dired-actual-switches))))
  	(if (equal dirname (car (car (last dired-subdir-alist))))
  	    ;; If doing the top level directory of the buffer,
  	    ;; redo it as specified in dired-directory.


However, I don't know if it has some unexpected consequences on dired.

All the best,
Gautier.

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Yuri Khan]

On Wed, 24 May 2023 at 18:36, Robert Pluim <rpluim@gmail.com> wrote:
>
>     Gautier> Maybe `dired-insert-subdir' could search for "R" but exclude "%R"?
>
> Something like the following (untested) patch?
>
> -    (if (setq switches-have-R (and switches (string-match-p "R" switches)))
> +    (if (setq switches-have-R (and switches (string-match-p "[^%]R" switches)))

This looks like a kludge. Will any occurrences of ‘R’ also be
discarded in values for options such as ‘--hide’, ‘-I’|‘--ignore’, and
any new options introduced later that take an arbitrary user-supplied
string?

Also, will a ‘--recursive’ affect ‘dired-insert-subdir’ in the same
way that removing ‘R’ is designed to prevent?

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Eli Zaretskii]

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Wed, 24 May 2023 23:41:37 +0700
> Cc: Gautier Ponsinet <gautier@gautierponsinet.xyz>, emacs-devel@gnu.org
> 
> On Wed, 24 May 2023 at 18:36, Robert Pluim <rpluim@gmail.com> wrote:
> >
> >     Gautier> Maybe `dired-insert-subdir' could search for "R" but exclude "%R"?
> >
> > Something like the following (untested) patch?
> >
> > -    (if (setq switches-have-R (and switches (string-match-p "R" switches)))
> > +    (if (setq switches-have-R (and switches (string-match-p "[^%]R" switches)))
> 
> This looks like a kludge. Will any occurrences of ‘R’ also be
> discarded in values for options such as ‘--hide’, ‘-I’|‘--ignore’, and
> any new options introduced later that take an arbitrary user-supplied
> string?
> 
> Also, will a ‘--recursive’ affect ‘dired-insert-subdir’ in the same
> way that removing ‘R’ is designed to prevent?

We sometimes install kludgey solutions if no better ideas are
available.  Do you have better ideas for fixing this?

Thanks.

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Yuri Khan]

On Wed, 24 May 2023 at 23:58, Eli Zaretskii <eliz@gnu.org> wrote:

> We sometimes install kludgey solutions if no better ideas are
> available.  Do you have better ideas for fixing this?

I do not have a sufficiently deep understanding of the original
problem removing ‘R’s solves. If it’s intended to build an ‘ls’
invocation with the same switches as used in the buffer but
non-recursive, I’m afraid my idea would be to mimic full-fledged
switches parsing, including the cases of ‘-R’, ‘--recursive’ (remove
the whole switch), bunched single-character switches (remove the R but
stop at the first I, T or w because what follows those is no longer
switches but the argument value to I, T or w), and stopping at the
‘--’. Even that is brittle wrt addition of new short options taking
arguments. Please feel free to disregard.

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Robert Pluim]

>>>>> On Wed, 24 May 2023 17:03:49 +0200, gautier@gautierponsinet.xyz said:
    Gautier> It seems that the exact same modification you did also need to be done
    Gautier> in the
    Gautier> function: dired-insert-subdir-doinsert .

    Gautier> So, the following patch works for me:

    Gautier> diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
    Gautier> index a07406e4c0d..8d31efd4a1a 100644
    Gautier> --- a/lisp/dired-aux.el
    Gautier> +++ b/lisp/dired-aux.el
    Gautier> @@ -3142,7 +3142,7 @@ dired-insert-subdir
    Gautier>      (and (not switches) cons (setq switches (cdr cons)))
    Gautier>      (dired-insert-subdir-validate dirname switches)
    Gautier>      ;; case-fold-search is nil now, so we can test for capital `R':
    Gautier> -    (if (setq switches-have-R (and switches (string-match-p "R"
    Gautier>      switches)))
    Gautier> +    (if (setq switches-have-R (and switches (string-match-p "[^%]R"
    Gautier> switches)))
    Gautier>  	;; avoid duplicated subdirs
    Gautier>  	(setq mark-alist (dired-kill-tree dirname t)))
    Gautier>      (if elt
    Gautier> @@ -3258,7 +3258,7 @@ dired-insert-subdir-doinsert
    Gautier>        (let ((dired-actual-switches
    Gautier>  	     (or switches
    Gautier>  		 dired-subdir-switches
    Gautier> -		 (string-replace "R" "" dired-actual-switches))))
    Gautier> +		 (string-replace "[^%]R" "" dired-actual-switches))))
    Gautier>  	(if (equal dirname (car (car (last dired-subdir-alist))))
    Gautier>  	    ;; If doing the top level directory of the buffer,
    Gautier>  	    ;; redo it as specified in dired-directory.

That doesnʼt seem right. If you have "-lR", that would result in "-"

Robert
-- 

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Gautier Ponsinet]

Robert Pluim <rpluim@gmail.com> (2023-05-25 08:45 +0200):
> That doesnʼt seem right. If you have "-lR", that would result in "-"

Indeed.

Do you think another solution could be to expand '%R' into '%H:%M' early 
on in dired-insert-subdir then?

All the best,
Gautier.

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Robert Pluim]

>>>>> On Thu, 25 May 2023 16:31:52 +0200, Gautier Ponsinet <gautier@gautierponsinet.xyz> said:

    Gautier> Robert Pluim <rpluim@gmail.com> (2023-05-25 08:45 +0200):
    >> That doesnʼt seem right. If you have "-lR", that would result in "-"

    Gautier> Indeed.

    Gautier> Do you think another solution could be to expand '%R' into '%H:%M'
    Gautier> early on in dired-insert-subdir then?

Thatʼs another option, perhaps slightly less kludgy than my original
patch :-)

Robert
-- 

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Gautier Ponsinet]

Robert Pluim <rpluim@gmail.com> (2023-05-25 18:21 +0200):
>     Gautier> Do you think another solution could be to expand '%R' into '%H:%M'
>     Gautier> early on in dired-insert-subdir then?
>
> Thatʼs another option, perhaps slightly less kludgy than my original
> patch :-)

So, something like this?


diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index a07406e4c0d..967bdbd3e44 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -3142,7 +3142,7 @@ dired-insert-subdir
     (and (not 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)))
+    (if (setq switches-have-R (and switches (string-match-p "R" (string-replate "%R" "%H:%M" switches))))
 	;; avoid duplicated subdirs
 	(setq mark-alist (dired-kill-tree dirname t)))
     (if elt
@@ -3157,7 +3157,7 @@ dired-insert-subdir
 	(push (cons dirname switches) dired-switches-alist)))
     (when switches-have-R
       (dired-build-subdir-alist switches)
-      (setq switches (string-replace "R" "" switches))
+      (setq switches (string-replace "R" "" (string-replace "%R" "%H:%M" switches)))
       (dolist (cur-ass dired-subdir-alist)
 	(let ((cur-dir (car cur-ass)))
 	  (and (dired-in-this-tree-p cur-dir dirname)
@@ -3258,7 +3258,7 @@ dired-insert-subdir-doinsert
       (let ((dired-actual-switches
 	     (or switches
 		 dired-subdir-switches
-		 (string-replace "R" "" dired-actual-switches))))
+		 (string-replace "R" "" (string-replace "%R" "%H:%M" dired-actual-switches)))))
 	(if (equal dirname (car (car (last dired-subdir-alist))))
 	    ;; If doing the top level directory of the buffer,
 	    ;; redo it as specified in dired-directory.



All the best,
Gautier.

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Gautier Ponsinet]

There is a typo in my previous patch.
Here is the corrected version:

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index a07406e4c0d..967bdbd3e44 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -3142,7 +3142,7 @@ dired-insert-subdir
     (and (not 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)))
+    (if (setq switches-have-R (and switches (string-match-p "R" (string-replace "%R" "%H:%M" switches))))
 	;; avoid duplicated subdirs
 	(setq mark-alist (dired-kill-tree dirname t)))
     (if elt
@@ -3157,7 +3157,7 @@ dired-insert-subdir
 	(push (cons dirname switches) dired-switches-alist)))
     (when switches-have-R
       (dired-build-subdir-alist switches)
-      (setq switches (string-replace "R" "" switches))
+      (setq switches (string-replace "R" "" (string-replace "%R" "%H:%M" switches)))
       (dolist (cur-ass dired-subdir-alist)
 	(let ((cur-dir (car cur-ass)))
 	  (and (dired-in-this-tree-p cur-dir dirname)
@@ -3258,7 +3258,7 @@ dired-insert-subdir-doinsert
       (let ((dired-actual-switches
 	     (or switches
 		 dired-subdir-switches
-		 (string-replace "R" "" dired-actual-switches))))
+		 (string-replace "R" "" (string-replace "%R" "%H:%M" dired-actual-switches)))))
 	(if (equal dirname (car (car (last dired-subdir-alist))))
 	    ;; If doing the top level directory of the buffer,
 	    ;; redo it as specified in dired-directory.

Re: Dired subdirectories & the ls option time-style with the %R sequence

[Gautier Ponsinet]

This is just a gentle reminder.

All the best,
Gautier.