Proposal: ls-lisp.el handle --group-directories-first flag

Proposal: ls-lisp.el handle --group-directories-first flag

[Arthur Miller]

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

Just a minor 'out-of-the-box' quality of life improvement mostly for
MS Windows users. ls-lisp.el is default on MS Windows, and some users
might share their init configuration between their Gnu/Linux and MS
Windows systems, it might be nice to have ls-lisp.el recognize
`--group-directories-first' flag of Gnu ls program.

I have just piggy-back on existing code here; I am not sure if I can
actually set `ls-lisp-dirs-firs' var to `t' when
`--group-directories-first' is found, so I am just converting this flag
to `y' so we can per-use existing machinery. But I would happily skip
`y' if it is acceptable to change users choice for `ls-lisp-dirs-first'
variable when `--group-directories-first' is present.

If this would be acceptable, I can also add some notice to docs and
news. 


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

--- ../emacs/lisp/ls-lisp.el	2021-06-06 23:11:38.317648694 +0200
+++ ./ls-lisp.el	2021-07-15 11:08:02.233434792 +0200
@@ -291,6 +291,10 @@
 	;; Remove --dired switch
 	(if (string-match "--dired " switches)
 	    (setq switches (replace-match "" nil nil switches)))
+        ;; Convert --group-directories-first to `y' (`y' and `Y' does not seem to
+        ;; be used by any ls prog currently so we can per-use it here)
+        (if (string-match "--group-directories-first " switches)
+            (setq switches (replace-match "y" nil nil switches)))
 	;; Convert SWITCHES to a list of characters.
 	(setq switches (delete ?\  (delete ?- (append switches nil))))
 	;; Sometimes we get ".../foo*/" as FILE.  While the shell and
@@ -682,7 +686,7 @@
 	       (ding) (sit-for 2))))	; to show user the message!
   (if (memq ?F switches)		; classify switch
       (setq file-alist (mapcar 'ls-lisp-classify file-alist)))
-  (if ls-lisp-dirs-first
+  (if (or ls-lisp-dirs-first (memq ?y switches))
   ;; Re-sort directories first, without otherwise changing the
   ;; ordering, and reverse whole list.  cadr of each element of
   ;; `file-alist' is t for directory, string (name linked to) for

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Date: Thu, 15 Jul 2021 11:19:52 +0200
> 
> Just a minor 'out-of-the-box' quality of life improvement mostly for
> MS Windows users. ls-lisp.el is default on MS Windows, and some users
> might share their init configuration between their Gnu/Linux and MS
> Windows systems, it might be nice to have ls-lisp.el recognize
> `--group-directories-first' flag of Gnu ls program.
> 
> I have just piggy-back on existing code here; I am not sure if I can
> actually set `ls-lisp-dirs-firs' var to `t' when
> `--group-directories-first' is found, so I am just converting this flag
> to `y' so we can per-use existing machinery. But I would happily skip
> `y' if it is acceptable to change users choice for `ls-lisp-dirs-first'
> variable when `--group-directories-first' is present.

This is not future-proof: what if 'ls' in some future release will
have the -y option, and we'd want to support that in ls-lisp.el?

Doesn't it work to let-bind ls-lisp-dirs-first?

Thanks.

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Arthur Miller]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Date: Thu, 15 Jul 2021 11:19:52 +0200
>> 
>> Just a minor 'out-of-the-box' quality of life improvement mostly for
>> MS Windows users. ls-lisp.el is default on MS Windows, and some users
>> might share their init configuration between their Gnu/Linux and MS
>> Windows systems, it might be nice to have ls-lisp.el recognize
>> `--group-directories-first' flag of Gnu ls program.
>> 
>> I have just piggy-back on existing code here; I am not sure if I can
>> actually set `ls-lisp-dirs-firs' var to `t' when
>> `--group-directories-first' is found, so I am just converting this flag
>> to `y' so we can per-use existing machinery. But I would happily skip
>> `y' if it is acceptable to change users choice for `ls-lisp-dirs-first'
>> variable when `--group-directories-first' is present.
>
> This is not future-proof: what if 'ls' in some future release will
> have the -y option, and we'd want to support that in ls-lisp.el?
Yes, I knowm I just thought that world does not directly see an explosion of
'ls' applications development. Yes, I had an idea you wouldn't like the
hacky patch, it was nore of a joke.

> Doesn't it work to let-bind ls-lisp-dirs-first?

I don't know about that one. ls-lisp--insert-directory effectively
ensures that switches are passed further the line as a string of
chars. The flag is lost in the first step and can't be passed further
on. I am not sure it's worth the trouble to modify that behaviour. Or
maybe I don't know what you mean. 

Anyway, see if this is acceptable. It adds one extra variable, so it's still
pretty cheap.


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

--- ../emacs/lisp/ls-lisp.el	2021-06-06 23:11:38.317648694 +0200
+++ ./ls-lisp.el	2021-07-16 15:00:29.000632720 +0200
@@ -253,6 +253,9 @@
   "Format to display float file sizes.")
 (defvar ls-lisp-filesize-b-fmt " %.0f"
   "Format to display file sizes in blocks (for the -s switch).")
+(defvar ls-lisp--really-dirs-first nil
+  "`t' if either ls-lisp-dirs-first is set to `t' or GNU ls flag
+  \"--group-directory-first\" is passed in switches to ls-lisp.")
 \f
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
@@ -291,6 +294,9 @@
 	;; Remove --dired switch
 	(if (string-match "--dired " switches)
 	    (setq switches (replace-match "" nil nil switches)))
+        (setq ls-lisp--really-dirs-first
+              (or ls-lisp-dirs-first
+                  (string-match "--group-directories-first" switches)))
 	;; Convert SWITCHES to a list of characters.
 	(setq switches (delete ?\  (delete ?- (append switches nil))))
 	;; Sometimes we get ".../foo*/" as FILE.  While the shell and
@@ -682,7 +688,7 @@
 	       (ding) (sit-for 2))))	; to show user the message!
   (if (memq ?F switches)		; classify switch
       (setq file-alist (mapcar 'ls-lisp-classify file-alist)))
-  (if ls-lisp-dirs-first
+  (if ls-lisp--really-dirs-first
   ;; Re-sort directories first, without otherwise changing the
   ;; ordering, and reverse whole list.  cadr of each element of
   ;; `file-alist' is t for directory, string (name linked to) for

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 16 Jul 2021 15:20:58 +0200
> 
> > Doesn't it work to let-bind ls-lisp-dirs-first?
> 
> I don't know about that one. ls-lisp--insert-directory effectively
> ensures that switches are passed further the line as a string of
> chars. The flag is lost in the first step and can't be passed further
> on.

Which flag is lost and why?

> Anyway, see if this is acceptable. It adds one extra variable, so it's still
> pretty cheap.

I'm still missing something, because I don't see how this is different
from just let-binding ls-lisp-dirs-first.

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Arthur Miller]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 16 Jul 2021 15:20:58 +0200
>> 
>> > Doesn't it work to let-bind ls-lisp-dirs-first?
>> 
>> I don't know about that one. ls-lisp--insert-directory effectively
>> ensures that switches are passed further the line as a string of
>> chars. The flag is lost in the first step and can't be passed further
>> on.
>
> Which flag is lost and why?

Ok, I take my words back, it is actually not lost. I was to fast didn't
reallize --group-directories-first was just converted with rest to a
list of chars.

Doesn't it mean that ls-lisp could potentialy recieve flags that user
didn't ment to use?

> I'm still missing something, because I don't see how this is different
> from just let-binding ls-lisp-dirs-first.

Since --group-directories-first is still there, then let bind works fine
:-).


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

--- ../emacs/lisp/ls-lisp.el	2021-06-06 23:11:38.317648694 +0200
+++ ./ls-lisp.el	2021-07-16 16:54:51.413276991 +0200
@@ -682,24 +682,27 @@
 	       (ding) (sit-for 2))))	; to show user the message!
   (if (memq ?F switches)		; classify switch
       (setq file-alist (mapcar 'ls-lisp-classify file-alist)))
-  (if ls-lisp-dirs-first
-  ;; Re-sort directories first, without otherwise changing the
-  ;; ordering, and reverse whole list.  cadr of each element of
-  ;; `file-alist' is t for directory, string (name linked to) for
-  ;; symbolic link, or nil.
-      (let (el dirs files)
-	(while file-alist
-	  (if (or (eq (cadr (setq el (car file-alist))) t) ; directory
-                  (and (stringp (cadr el))
-                       (file-directory-p (cadr el)))) ; symlink to a directory
-	      (setq dirs (cons el dirs))
-	    (setq files (cons el files)))
-	  (setq file-alist (cdr file-alist)))
-	(setq file-alist
-	      (if (memq ?U switches)	; unsorted order is reversed
-		  (nconc dirs files)
-		(nconc files dirs)
-		))))
+  (let ((dirs-first (or ls-lisp-dirs-first
+                        (string-match "groupdirectoriesfirst" (concat
+                                                               switches))))) 
+    (if dirs-first
+        ;; Re-sort directories first, without otherwise changing the
+        ;; ordering, and reverse whole list.  cadr of each element of
+        ;; `file-alist' is t for directory, string (name linked to) for
+        ;; symbolic link, or nil.
+        (let (el dirs files)
+	  (while file-alist
+	    (if (or (eq (cadr (setq el (car file-alist))) t) ; directory
+                    (and (stringp (cadr el))
+                         (file-directory-p (cadr el)))) ; symlink to a directory
+	        (setq dirs (cons el dirs))
+	      (setq files (cons el files)))
+	    (setq file-alist (cdr file-alist)))
+	  (setq file-alist
+	        (if (memq ?U switches)	; unsorted order is reversed
+		    (nconc dirs files)
+		  (nconc files dirs)
+		  )))))
   ;; Finally reverse file alist if necessary.
   ;; (eq below MUST compare `(not (memq ...))' to force comparison of
   ;; t or nil, rather than list tails!)

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 16 Jul 2021 16:58:16 +0200
> 
> Ok, I take my words back, it is actually not lost. I was to fast didn't
> reallize --group-directories-first was just converted with rest to a
> list of chars.
> 
> Doesn't it mean that ls-lisp could potentialy recieve flags that user
> didn't ment to use?

Yes, you need to remove that option before the options get converted.

> Since --group-directories-first is still there, then let bind works fine
> :-).

Great.  I think this can be installed, as soon as you add the removal
of that option from the options' list.

Thanks.

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Arthur Miller]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 16 Jul 2021 16:58:16 +0200
>> 
>> Ok, I take my words back, it is actually not lost. I was to fast didn't
>> reallize --group-directories-first was just converted with rest to a
>> list of chars.
>> 
>> Doesn't it mean that ls-lisp could potentialy recieve flags that user
>> didn't ment to use?
>
> Yes, you need to remove that option before the options get converted.

Yes, and possibly even other long options that an ignorant, unawary user
like myself can pass in dired-listing-switches. The original code just
removes "--dired" flag. Everything else is converted into character
list, so if I pass something like --time-style=long-iso, everything but
dashes makes it into list with switches.

Manual says which switches are supported by ls-lisp, but now how it
behaves when passed flags it does not support:

https://www.gnu.org/software/emacs/manual/html_node/emacs/ls-in-Lisp.html

Personally I expect it to just igonre those, now I have learned it does
not. Bug or not, does not matter, I think it is more convenient to have
one version of dired-listing-switches, then to keep two different, so I
suggest to "sanitize switches" before they are processed. Check included
ls-lisp-p2.patch and see if that is acceptable.

>> Since --group-directories-first is still there, then let bind works fine
>> :-).
>
> Great.  I think this can be installed, as soon as you add the removal
> of that option from the options' list.

Yepp, included patch ls-lisp-p1.patch does it.

A think to note is that -U will be ignored if --group-directories-first
is passed (so does gls).


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

--- ../emacs/lisp/ls-lisp.el	2021-06-06 23:11:38.317648694 +0200
+++ ./ls-lisp.el	2021-07-19 22:07:19.977128780 +0200
@@ -284,13 +284,19 @@
     (let ((handler (find-file-name-handler (expand-file-name file)
 					   'insert-directory))
 	  (orig-file file)
-	  wildcard-regexp)
+	  wildcard-regexp
+          (ls-lisp-dirs-first (or ls-lisp-dirs-first
+                                 (string-match "--group-directories-first" switches))))
       (if handler
 	  (funcall handler 'insert-directory file switches
 		   wildcard full-directory-p)
 	;; Remove --dired switch
 	(if (string-match "--dired " switches)
 	    (setq switches (replace-match "" nil nil switches)))
+        (if (string-match "--group-directories-first" switches)
+            ;; if ls-lisp-dirs-first is nil, dirs are grouped but come out in
+            ;; reverse order, so add -U flag to fix that
+            (setq switches (concat "-U " (replace-match "" nil nil switches))))
 	;; Convert SWITCHES to a list of characters.
 	(setq switches (delete ?\  (delete ?- (append switches nil))))
 	;; Sometimes we get ".../foo*/" as FILE.  While the shell and

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: ls-lisp-p2.patch --]
[-- Type: text/x-patch, Size: 3946 bytes --]

--- ../emacs/lisp/ls-lisp.el	2021-06-06 23:11:38.317648694 +0200
+++ ./ls-lisp.el	2021-07-19 22:34:46.464966628 +0200
@@ -284,13 +284,18 @@
     (let ((handler (find-file-name-handler (expand-file-name file)
 					   'insert-directory))
 	  (orig-file file)
-	  wildcard-regexp)
+	  wildcard-regexp
+          (ls-lisp-dirs-first (or ls-lisp-dirs-first
+                                 (string-match "--group-directories-first" switches))))
       (if handler
 	  (funcall handler 'insert-directory file switches
 		   wildcard full-directory-p)
-	;; Remove --dired switch
-	(if (string-match "--dired " switches)
-	    (setq switches (replace-match "" nil nil switches)))
+        (if (string-match "--group-directories-first" switches)
+            ;; if ls-lisp-dirs-first is nil, dirs are grouped but come out in
+            ;; reverse order, so add -U flag to fix that
+            (setq switches (concat "-U " (replace-match "" nil nil switches))))
+	;; Remove unrecognized ls long flags, and convert recognized to short
+        (setq switches (ls-lisp--sanitize-switches switches))
 	;; Convert SWITCHES to a list of characters.
 	(setq switches (delete ?\  (delete ?- (append switches nil))))
 	;; Sometimes we get ".../foo*/" as FILE.  While the shell and
@@ -890,6 +895,60 @@
   ;; Continue standard unloading.
   nil)
 
+(defun ls-lisp--sanitize-switches (switches)
+  "Convert long opt flags to short.  Conversion is done only for flags supported
+by ls-lisp.  Long options not supported by ls-lisp are removed.  Supported
+options are: A a B C c F G g h i n R r S s t U u v X. The l switch is assumed to
+be always present and cannot be turned off.  --As listed in docs for
+ls-lisp--insert-directory function." 
+  (let ((lsflags '(("-a" . "--all")
+                   ("-A" . "--almost-all")
+                   ("-B" . "--ignore-backups")
+                   ("-C" . "--color")
+                   ("-F" . "--classify")
+                   ("-G" . "--no-group")
+                   ("-h" . "--human-readable")
+                   ("-H" . "--dereference-command-line")
+                   ("-i" . "--inode")
+                   ("-n" . "--numeric-uid-gid")
+                   ("-r" . "--reverse")
+                   ("-R" . "--recursive")
+                   ("-s" . "--size")
+                   ("-S" . "--sort.*[ \\\t]")
+                   (""   . "--group-directories-first")
+                   (""   . "--author")
+                   (""   . "--escape")
+                   (""   . "--directory")
+                   (""   . "--dired")
+                   (""   . "--file-type")
+                   (""   . "--format")
+                   (""   . "--full-time")
+                   (""   . "--si")
+                   (""   . "--dereference-command-line-symlink-to-dir")
+                   (""   . "--hide")
+                   (""   . "--hyperlink")
+                   (""   . "--ignore")
+                   (""   . "--kibibytes")
+                   (""   . "--dereference")
+                   (""   . "--literal")
+                   (""   . "--hide-control-chars")
+                   (""   . "--show-control-chars")
+                   (""   . "--quote-name")
+                   (""   . "--context")
+                   (""   . "--help")
+                   ;; (""   . "--indicator-style.*[ \\\t]")
+                   ;; (""   . "--quoting-style.*[ \t\\]")
+                   ;; (""   . "--time.*[ \\\t]")
+                   ;; (""   . "--time-style.*[ \\\t]")
+                   ;; (""   . "--tabsize.*[ \\\t]")
+                   ;; (""   . "--width.*[ \\\t]")
+                   (""   . "--.*=.*[ \\\t\n]?") ;; catch all with '=' sign in
+                   (""   . "--version"))))
+    (dolist (f lsflags)
+      (if (string-match (cdr f) switches)
+          (setq switches (replace-match (car f) nil nil switches))))
+    (string-trim switches)))
+
 (provide 'ls-lisp)
 
 ;;; ls-lisp.el ends here

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Cc: emacs-devel@gnu.org
> Date: Mon, 19 Jul 2021 22:59:13 +0200
> 
> >> Since --group-directories-first is still there, then let bind works fine
> >> :-).
> >
> > Great.  I think this can be installed, as soon as you add the removal
> > of that option from the options' list.
> 
> Yepp, included patch ls-lisp-p1.patch does it.
> 
> A think to note is that -U will be ignored if --group-directories-first
> is passed (so does gls).

Hmm... bother: the need to add -U could be a problem, because users
don't necessarily want the list to come out unsorted.

If adding -U is to fix the order of the directories, then how about
using nreverse on the list of directories instead?

(Sorry about my slow responses, and thanks for working on this.)

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Eli Zaretskii]

> Date: Sat, 24 Jul 2021 14:13:09 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > A think to note is that -U will be ignored if --group-directories-first
> > is passed (so does gls).
> 
> Hmm... bother: the need to add -U could be a problem, because users
> don't necessarily want the list to come out unsorted.
> 
> If adding -U is to fix the order of the directories, then how about
> using nreverse on the list of directories instead?

Btw, in my testing, with ls-lisp-dirs-first bound non-nil, the
directories are shown in their correct alphabetically sorted order.
So I'm unsure what problems you saw and in which scenario.  Can you
tell more about this issue?

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 24 Jul 2021 14:13:09 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: emacs-devel@gnu.org
>> 
>> > A think to note is that -U will be ignored if --group-directories-first
>> > is passed (so does gls).
>> 
>> Hmm... bother: the need to add -U could be a problem, because users
>> don't necessarily want the list to come out unsorted.
>> 
>> If adding -U is to fix the order of the directories, then how about
>> using nreverse on the list of directories instead?

True, I don't know why I didn't thought of it :-).

> Btw, in my testing, with ls-lisp-dirs-first bound non-nil, the
> directories are shown in their correct alphabetically sorted order.
> So I'm unsure what problems you saw and in which scenario.  Can you
> tell more about this issue?

Ok, I'll test more.

> (Sorry about my slow responses, and thanks for working on this.)
No rush, it's not like this was something people complain about, and I
have seen all the tree-sitter work going on.

Have you seen the other patch, where flags are sanitized, that would
make ls-lisp accept all the flags as gnu ls, but ignore those it does
not undrstand. Provided that gnu ls does not use same flags as ls-lisp
with different meaning.

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 24 Jul 2021 13:54:49 +0200
> 
> > Btw, in my testing, with ls-lisp-dirs-first bound non-nil, the
> > directories are shown in their correct alphabetically sorted order.
> > So I'm unsure what problems you saw and in which scenario.  Can you
> > tell more about this issue?
> 
> Ok, I'll test more.

Thanks.

> Have you seen the other patch, where flags are sanitized, that would
> make ls-lisp accept all the flags as gnu ls, but ignore those it does
> not undrstand.

Yes, it looks OK to me.

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Arthur Miller]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sat, 24 Jul 2021 13:54:49 +0200
>> 
>> > Btw, in my testing, with ls-lisp-dirs-first bound non-nil, the
>> > directories are shown in their correct alphabetically sorted order.
>> > So I'm unsure what problems you saw and in which scenario.  Can you
>> > tell more about this issue?

When I was testing, if ls-lisp-dirs-first is nil, and
--group-directories-first is specified, dirs come out in reverse order
in new buffer. Just reverting the buffer didn't show this. I don't know
why it is so. I just tested with emacs built last night from the
master (though only on Arch Linux). Either kill the dired buffer and
open again, or just navigate to another dir.

>> Ok, I'll test more.
>
> Thanks.

I remember now what was going on: when --group-directories-first is
specified, the -U option does not have effect according to gnu ls
manual. Anyway, since we let-bind the ls-lisp-dirs-first, I can just set
that one to 't and get same effect.

>> Have you seen the other patch, where flags are sanitized, that would
>> make ls-lisp accept all the flags as gnu ls, but ignore those it does
>> not undrstand.
>
> Yes, it looks OK to me.

If you are OK with the patch2, then here is one without -U on top of
that one.


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

--- ../emacs/lisp/ls-lisp.el	2021-06-06 23:11:38.317648694 +0200
+++ ./ls-lisp.el	2021-07-24 16:51:38.433573800 +0200
@@ -284,13 +284,19 @@
     (let ((handler (find-file-name-handler (expand-file-name file)
 					   'insert-directory))
 	  (orig-file file)
-	  wildcard-regexp)
+	  wildcard-regexp
+          (ls-lisp-dirs-first (or ls-lisp-dirs-first
+                                 (string-match "--group-directories-first" switches))))
       (if handler
 	  (funcall handler 'insert-directory file switches
 		   wildcard full-directory-p)
-	;; Remove --dired switch
-	(if (string-match "--dired " switches)
-	    (setq switches (replace-match "" nil nil switches)))
+        (when (string-match "--group-directories-first" switches)
+            ;; if ls-lisp-dirs-first is nil, dirs are grouped but come out in
+            ;; reverse order:
+            (setq ls-lisp-dirs-first t)
+            (setq switches (replace-match "" nil nil switches)))
+	;; Remove unrecognized ls long flags, and convert recognized to short
+        (setq switches (ls-lisp--sanitize-switches switches))
 	;; Convert SWITCHES to a list of characters.
 	(setq switches (delete ?\  (delete ?- (append switches nil))))
 	;; Sometimes we get ".../foo*/" as FILE.  While the shell and
@@ -890,6 +896,60 @@
   ;; Continue standard unloading.
   nil)
 
+(defun ls-lisp--sanitize-switches (switches)
+  "Convert long opt flags to short.  Conversion is done only for flags supported
+by ls-lisp.  Long options not supported by ls-lisp are removed.  Supported
+options are: A a B C c F G g h i n R r S s t U u v X. The l switch is assumed to
+be always present and cannot be turned off.  --As listed in docs for
+ls-lisp--insert-directory function." 
+  (let ((lsflags '(("-a" . "--all")
+                   ("-A" . "--almost-all")
+                   ("-B" . "--ignore-backups")
+                   ("-C" . "--color")
+                   ("-F" . "--classify")
+                   ("-G" . "--no-group")
+                   ("-h" . "--human-readable")
+                   ("-H" . "--dereference-command-line")
+                   ("-i" . "--inode")
+                   ("-n" . "--numeric-uid-gid")
+                   ("-r" . "--reverse")
+                   ("-R" . "--recursive")
+                   ("-s" . "--size")
+                   ("-S" . "--sort.*[ \\\t]")
+                   (""   . "--group-directories-first")
+                   (""   . "--author")
+                   (""   . "--escape")
+                   (""   . "--directory")
+                   (""   . "--dired")
+                   (""   . "--file-type")
+                   (""   . "--format")
+                   (""   . "--full-time")
+                   (""   . "--si")
+                   (""   . "--dereference-command-line-symlink-to-dir")
+                   (""   . "--hide")
+                   (""   . "--hyperlink")
+                   (""   . "--ignore")
+                   (""   . "--kibibytes")
+                   (""   . "--dereference")
+                   (""   . "--literal")
+                   (""   . "--hide-control-chars")
+                   (""   . "--show-control-chars")
+                   (""   . "--quote-name")
+                   (""   . "--context")
+                   (""   . "--help")
+                   ;; (""   . "--indicator-style.*[ \\\t]")
+                   ;; (""   . "--quoting-style.*[ \t\\]")
+                   ;; (""   . "--time.*[ \\\t]")
+                   ;; (""   . "--time-style.*[ \\\t]")
+                   ;; (""   . "--tabsize.*[ \\\t]")
+                   ;; (""   . "--width.*[ \\\t]")
+                   (""   . "--.*=.*[ \\\t\n]?") ;; catch all with '=' sign in
+                   (""   . "--version"))))
+    (dolist (f lsflags)
+      (if (string-match (cdr f) switches)
+          (setq switches (replace-match (car f) nil nil switches))))
+    (string-trim switches)))
+
 (provide 'ls-lisp)
 
 ;;; ls-lisp.el ends here

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 24 Jul 2021 16:58:08 +0200
> 
> >> > Btw, in my testing, with ls-lisp-dirs-first bound non-nil, the
> >> > directories are shown in their correct alphabetically sorted order.
> >> > So I'm unsure what problems you saw and in which scenario.  Can you
> >> > tell more about this issue?
> 
> When I was testing, if ls-lisp-dirs-first is nil, and
> --group-directories-first is specified, dirs come out in reverse order
> in new buffer. Just reverting the buffer didn't show this. I don't know
> why it is so.

Because --group-directories-first includes 'r'?

> If you are OK with the patch2, then here is one without -U on top of
> that one.

I take it that the problem with the reverse order doesn't exist?  Or
does it still exist and needs to be debugged?

Thanks.

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sat, 24 Jul 2021 16:58:08 +0200
>> 
>> >> > Btw, in my testing, with ls-lisp-dirs-first bound non-nil, the
>> >> > directories are shown in their correct alphabetically sorted order.
>> >> > So I'm unsure what problems you saw and in which scenario.  Can you
>> >> > tell more about this issue?
>> 
>> When I was testing, if ls-lisp-dirs-first is nil, and
>> --group-directories-first is specified, dirs come out in reverse order
>> in new buffer. Just reverting the buffer didn't show this. I don't know
>> why it is so.
>
> Because --group-directories-first includes 'r'?
No, it is removed from switches before sorting happends, in this patch.

>> If you are OK with the patch2, then here is one without -U on top of
>> that one.
>
> I take it that the problem with the reverse order doesn't exist?  Or
> does it still exist and needs to be debugged?

No longer, the last patch fixes it by setting let-bound value of
ls-lisp-dirs-first to 't. I hope somebody else test it too,
beside me.

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 24 Jul 2021 21:02:56 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Arthur Miller <arthur.miller@live.com>
> >> Cc: emacs-devel@gnu.org
> >> Date: Sat, 24 Jul 2021 16:58:08 +0200
> >> 
> >> >> > Btw, in my testing, with ls-lisp-dirs-first bound non-nil, the
> >> >> > directories are shown in their correct alphabetically sorted order.
> >> >> > So I'm unsure what problems you saw and in which scenario.  Can you
> >> >> > tell more about this issue?
> >> 
> >> When I was testing, if ls-lisp-dirs-first is nil, and
> >> --group-directories-first is specified, dirs come out in reverse order
> >> in new buffer. Just reverting the buffer didn't show this. I don't know
> >> why it is so.
> >
> > Because --group-directories-first includes 'r'?
> No, it is removed from switches before sorting happends, in this patch.
> 
> >> If you are OK with the patch2, then here is one without -U on top of
> >> that one.
> >
> > I take it that the problem with the reverse order doesn't exist?  Or
> > does it still exist and needs to be debugged?
> 
> No longer, the last patch fixes it by setting let-bound value of
> ls-lisp-dirs-first to 't. I hope somebody else test it too,
> beside me.

Thanks, I installed it, with some minimal changes.

Btw, I don't see a copyright assignment for your contributions on
file.  Would you like to start the legal paperwork rolling, so we
could accept further contributions from you in the future?

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Arthur Miller]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sat, 24 Jul 2021 21:02:56 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Arthur Miller <arthur.miller@live.com>
>> >> Cc: emacs-devel@gnu.org
>> >> Date: Sat, 24 Jul 2021 16:58:08 +0200
>> >> 
>> >> >> > Btw, in my testing, with ls-lisp-dirs-first bound non-nil, the
>> >> >> > directories are shown in their correct alphabetically sorted order.
>> >> >> > So I'm unsure what problems you saw and in which scenario.  Can you
>> >> >> > tell more about this issue?
>> >> 
>> >> When I was testing, if ls-lisp-dirs-first is nil, and
>> >> --group-directories-first is specified, dirs come out in reverse order
>> >> in new buffer. Just reverting the buffer didn't show this. I don't know
>> >> why it is so.
>> >
>> > Because --group-directories-first includes 'r'?
>> No, it is removed from switches before sorting happends, in this patch.
>> 
>> >> If you are OK with the patch2, then here is one without -U on top of
>> >> that one.
>> >
>> > I take it that the problem with the reverse order doesn't exist?  Or
>> > does it still exist and needs to be debugged?
>> 
>> No longer, the last patch fixes it by setting let-bound value of
>> ls-lisp-dirs-first to 't. I hope somebody else test it too,
>> beside me.
>
> Thanks, I installed it, with some minimal changes.

Cool. Thanks!

> Btw, I don't see a copyright assignment for your contributions on
> file.  Would you like to start the legal paperwork rolling, so we
> could accept further contributions from you in the future?

I have already signed it. No idea why you can't see it.

I already helped with patch to dired.c with lots of help from Michael
A, if you remember directory-empty-p, you suggested to add count to
return to cut off unnecessary iteration and with wdired not re-reading
entire buffer when switching from dired wich proffeseur SM made into a
working patch.


[-- Attachment #2: arthur-fsf.jpg --]
[-- Type: image/jpeg, Size: 1101095 bytes --]

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Cc: emacs-devel@gnu.org
> Date: Sun, 25 Jul 2021 10:29:39 +0200
> 
> > Btw, I don't see a copyright assignment for your contributions on
> > file.  Would you like to start the legal paperwork rolling, so we
> > could accept further contributions from you in the future?
> 
> I have already signed it. No idea why you can't see it.

Sorry, my bad.  I guess I need new glasses.

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sun, 25 Jul 2021 10:29:39 +0200
>> 
>> > Btw, I don't see a copyright assignment for your contributions on
>> > file.  Would you like to start the legal paperwork rolling, so we
>> > could accept further contributions from you in the future?
>> 
>> I have already signed it. No idea why you can't see it.
>
> Sorry, my bad.  I guess I need new glasses.

No, you work too much! Just delegate more work :-)

Re: Proposal: ls-lisp.el handle --group-directories-first flag

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

> I guess I need new glasses.

Send pics! :-)

Best regards, Michael.