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

[Arthur Miller <arthur.miller@live.com>]

[-- 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