bug#41250: 28.0.50; Dired displays unconditionally ls-switches on modeline

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

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Cc: Drew Adams <drew.adams@oracle.com>,  41250@debbugs.gnu.org,
>>   juri@linkov.net
>> Date: Fri, 15 May 2020 23:08:46 +0200
>> 
>> After I saw Drews mail and patch, and answered, I was thinking
>> additionally, and I am actually now wondering, why is it assumed that
>> Dired will show sorting order on modeline by default? I mean other modes
>> does not do similar. Say, cc-mode does not show which current identation
>> scheme I use, or something similar.
>
> Yes, it does: the CC Mode shows the comment style in use and the minor
> mode.
>
>> Why is it assumed for Dired? I don't have historical insight so I
>> don't know why original author(s) decided to make it so?
>
> The sorting order was just one letter originally, so it sounds like a
> good idea to have an indication of why the order is this and not
> another.
>
>> If Dired show just, word "Dired" as it's lighter only, as other modes do,
>> then maybe Drews idea to have a format string is maybe the most flexible
>> one?
>
> Not IMO.  Using format strings and functions is "advanced usage",
> which is normally barred for newbies and relatively inexperienced
> Emacs users.  Popular options should IMO be exposed though easier
> customization values.

Ok, what about this strategy:

I have introduced dired-mode-line-hook, which is a usual thing in Emacs,
which is ment as a list of hooks that user can set. Each hook should
return a string that will be concatenated to the lighter. So users can
print whatever they want to that string (number of files, dirs etc).

I have introduced also another function that will just iterate through
hooks concat stuff and update the modeline and refactored some code to
call this function instead of old dired-sort-set-modeline. Also
dired-sort-set-modeline is changed to work as a mentioned hook and is
used as default value for dired-mode-line-hook.

If user does not prefer to see any aditional info on modeline then it is
just to set dired-mode-line-hook to nil. Obs, that can probably be coded
more elegantly, me & elisp are maybe not best friends (yet :-)). I have
built and tested emacs with the patch, but I might have missed
something.

While I was looking through the code to set myself into dired, I have
also noticed lots of '^L' chars, I took the freedom to clean it up where
I saw them, there are probably more.


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

--- dired.el	2020-05-14 03:06:34.046112281 +0200
+++ lisp/dired.el	2020-05-17 04:36:20.727942621 +0200
@@ -73,9 +73,9 @@
 (defcustom dired-subdir-switches nil
   "If non-nil, switches passed to `ls' for inserting subdirectories.
 If nil, `dired-listing-switches' is used."
-   :group 'dired
-   :type '(choice (const :tag "Use dired-listing-switches" nil)
-                  (string :tag "Switches")))
+  :group 'dired
+  :type '(choice (const :tag "Use dired-listing-switches" nil)
+                 (string :tag "Switches")))
 
 (defcustom dired-chown-program
   (purecopy (cond ((executable-find "chown") "chown")
@@ -238,6 +238,14 @@
   :group 'dired
   :type 'hook)
 
+(defcustom dired-mode-line-hook 'dired-sort-set-mode-line
+  "Run when dired is displaying it's info on modeline. Default hook is
+dired-set-sort-mode-line, which displays sorting order used in current
+  dired buffer. Every hook in the list should return a string that
+  will be appended to dired info already shown on modeline."
+  :group 'dired
+  :type 'hook)
+
 (defcustom dired-before-readin-hook nil
   "This hook is run before a Dired buffer is read in (created or reverted)."
   :group 'dired
@@ -634,7 +642,7 @@
   "Additional expressions to highlight in Dired mode.")
 
 (defvar dnd-protocol-alist)
-\f
+
 ;;; Macros must be defined before they are used, for the byte compiler.
 
 (defmacro dired-mark-if (predicate msg)
@@ -649,57 +657,57 @@
 
 Return value is the number of files marked, or nil if none were marked."
   `(let* ((inhibit-read-only t) count
-         (use-region-p (and dired-mark-region
-                            (region-active-p)
-                            (> (region-end) (region-beginning))))
-         (beg (if use-region-p
-                  (save-excursion
-                    (goto-char (region-beginning))
-                    (line-beginning-position))
-                (point-min)))
-         (end (if use-region-p
-                  (save-excursion
-                    (goto-char (region-end))
-                    (if (if (eq dired-mark-region 'line)
-                            (not (bolp))
-                          (get-text-property (1- (point)) 'dired-filename))
-                        (line-end-position)
-                      (line-beginning-position)))
-                (point-max))))
-    (save-excursion
-      (setq count 0)
-      (when ,msg
-	(message "%s %ss%s%s..."
-		 (cond ((eq dired-marker-char ?\s) "Unmarking")
-		       ((eq dired-del-marker dired-marker-char)
-			"Flagging")
-		       (t "Marking"))
-		 ,msg
-		 (if (eq dired-del-marker dired-marker-char)
-		     " for deletion"
-		   "")
-                 (if use-region-p
-                     " in region"
-                   "")))
-      (goto-char beg)
-      (while (< (point) end)
-        (when ,predicate
-          (unless (= (following-char) dired-marker-char)
-            (delete-char 1)
-            (insert dired-marker-char)
-            (setq count (1+ count))))
-        (forward-line 1))
-      (when ,msg (message "%s %s%s %s%s%s"
-                        count
-                        ,msg
-                        (dired-plural-s count)
-                        (if (eq dired-marker-char ?\s) "un" "")
-                        (if (eq dired-marker-char dired-del-marker)
-                            "flagged" "marked")
-                        (if use-region-p
-                            " in region"
-                          ""))))
-    (and (> count 0) count)))
+          (use-region-p (and dired-mark-region
+                             (region-active-p)
+                             (> (region-end) (region-beginning))))
+          (beg (if use-region-p
+                   (save-excursion
+                     (goto-char (region-beginning))
+                     (line-beginning-position))
+                 (point-min)))
+          (end (if use-region-p
+                   (save-excursion
+                     (goto-char (region-end))
+                     (if (if (eq dired-mark-region 'line)
+                             (not (bolp))
+                           (get-text-property (1- (point)) 'dired-filename))
+                         (line-end-position)
+                       (line-beginning-position)))
+                 (point-max))))
+     (save-excursion
+       (setq count 0)
+       (when ,msg
+	 (message "%s %ss%s%s..."
+		  (cond ((eq dired-marker-char ?\s) "Unmarking")
+		        ((eq dired-del-marker dired-marker-char)
+			 "Flagging")
+		        (t "Marking"))
+		  ,msg
+		  (if (eq dired-del-marker dired-marker-char)
+		      " for deletion"
+		    "")
+                  (if use-region-p
+                      " in region"
+                    "")))
+       (goto-char beg)
+       (while (< (point) end)
+         (when ,predicate
+           (unless (= (following-char) dired-marker-char)
+             (delete-char 1)
+             (insert dired-marker-char)
+             (setq count (1+ count))))
+         (forward-line 1))
+       (when ,msg (message "%s %s%s %s%s%s"
+                           count
+                           ,msg
+                           (dired-plural-s count)
+                           (if (eq dired-marker-char ?\s) "un" "")
+                           (if (eq dired-marker-char dired-del-marker)
+                               "flagged" "marked")
+                           (if use-region-p
+                               " in region"
+                             ""))))
+     (and (> count 0) count)))
 
 (defmacro dired-map-over-marks (body arg &optional show-progress
 				     distinguish-one-marked)
@@ -817,7 +825,6 @@
       (user-error (if (stringp error) error "No files specified")))
     result))
 
-\f
 ;; The dired command
 
 (defun dired-read-dir-and-switches (str)
@@ -1197,7 +1204,6 @@
               (setq blist (cdr blist))))))
       found)))
 
-\f
 ;; Read in a new dired buffer
 
 (defun dired-readin ()
@@ -1231,16 +1237,16 @@
       ;; same performance advantages without the problem of breaking
       ;; users of after/before-change-functions.
       (combine-change-calls (point-min) (point-max)
-	(let ((inhibit-read-only t)
-	      ;; Don't make undo entries for readin.
-	      (buffer-undo-list t))
-	  (erase-buffer)
-	  (dired-readin-insert))
-	(goto-char (point-min))
-	;; Must first make alist buffer local and set it to nil because
-	;; dired-build-subdir-alist will call dired-clear-alist first
-	(setq-local dired-subdir-alist nil)
-	(dired-build-subdir-alist))
+	                    (let ((inhibit-read-only t)
+	                          ;; Don't make undo entries for readin.
+	                          (buffer-undo-list t))
+	                      (erase-buffer)
+	                      (dired-readin-insert))
+	                    (goto-char (point-min))
+	                    ;; Must first make alist buffer local and set it to nil because
+	                    ;; dired-build-subdir-alist will call dired-clear-alist first
+	                    (setq-local dired-subdir-alist nil)
+	                    (dired-build-subdir-alist))
       (let ((attributes (file-attributes dirname)))
 	(if (eq (car attributes) t)
 	    (set-visited-file-modtime (file-attribute-modification-time
@@ -1556,7 +1562,7 @@
 	    (put-text-property (+ (point) 4) (line-end-position)
 			       'invisible 'dired-hide-details-link))))
       (forward-line 1))))
-\f
+
 ;; Reverting a dired buffer
 
 (defun dired-revert (&optional _arg _noconfirm)
@@ -1899,7 +1905,7 @@
     ;; No need to do this, now that top-level items are fewer.
     ;;;;
     ;; Get rid of the Edit menu bar item to save space.
-    ;(define-key map [menu-bar edit] 'undefined)
+    ;(define-key map [menu-bar edit] 'undefined)
 
     (define-key map [menu-bar subdir]
       (cons "Subdir" (make-sparse-keymap "Subdir")))
@@ -2209,7 +2215,7 @@
 
     map)
   "Local keymap for Dired mode buffers.")
-\f
+
 ;; Dired mode is suitable only for specially formatted data.
 (put 'dired-mode 'mode-class 'special)
 
@@ -2317,14 +2323,14 @@
   (add-hook 'file-name-at-point-functions #'dired-file-name-at-point nil t)
   (add-hook 'isearch-mode-hook #'dired-isearch-filenames-setup nil t)
   (run-mode-hooks 'dired-mode-hook))
-\f
+
 ;; Idiosyncratic dired commands that don't deal with marks.
 
 (defun dired-summary ()
   "Summarize basic Dired commands and show recent Dired errors."
   (interactive)
   (dired-why)
-  ;>> this should check the key-bindings and use substitute-command-keys if non-standard
+  ;>> this should check the key-bindings and use substitute-command-keys if non-standard
   (message
    "d-elete, u-ndelete, x-punge, f-ind, o-ther window, R-ename, C-opy, h-elp"))
 
@@ -2631,7 +2637,7 @@
   (if (string-match (concat "^" (regexp-quote dir)) file)
       (substring file (match-end 0))
     file))
-\f
+
 (define-minor-mode dired-hide-details-mode
   "Toggle visibility of detailed information in current Dired buffer.
 When this minor mode is enabled, details such as file ownership and
@@ -2668,7 +2674,7 @@
 	       'add-to-invisibility-spec
 	     'remove-from-invisibility-spec)
 	   'dired-hide-details-link))
-\f
+
 ;;; Functions to hide/unhide text
 
 (defun dired--find-hidden-pos (start end)
@@ -3553,7 +3559,7 @@
   (let ((beg (point)))
     (completion--insert-strings files)
     (put-text-property beg (point) 'mouse-face nil)))
-\f
+
 ;; Commands to mark or flag file(s) at or near current line.
 
 (defun dired-repeat-over-lines (arg function)
@@ -4014,7 +4020,7 @@
       (message (if (= count 1) "1 mark removed"
 		 "%d marks removed")
 	       count))))
-\f
+
 ;; Logging failures operating on files, and showing the results.
 
 (defvar dired-log-buffer "*Dired log*")
@@ -4080,7 +4086,7 @@
   ;; Log a summary describing a bunch of errors.
   (dired-log (concat "\n" string "\n"))
   (dired-log t))
-\f
+
 ;;; Sorting
 
 ;; Most ls can only sort by name or by date (with -t), nothing else.
@@ -4114,23 +4120,39 @@
   "Non-nil means the Dired sort command is disabled.
 The idea is to set this buffer-locally in special Dired buffers.")
 
+(defun dired-set-mode-line ()
+  ;; Flush dired info to mode-line (eval all dired-mode-line-hook)
+  ;; If dired-mode-line-hook is nil, it means user has manually
+  ;; disabled displaying of Dired info on mode-line, so let's respect
+  ;; the user decision.
+  (when (eq major-mode 'dired-mode)
+    (if dired-mode-line-hook
+        (progn
+          (let ((mode-info ""))
+            (dolist (hook dired-mode-line-hook)
+              (setq mode-info (concat " " (funcall hook))))
+            (setq mode-name (concat mode-name mode-info))))
+      (setq mode-name "Dired")) ;; reset name if dired-mode-line-hook is nil
+    (force-mode-line-update)))
+
 (defun dired-sort-set-mode-line ()
   ;; Set mode line display according to dired-actual-switches.
   ;; Mode line display of "by name" or "by date" guarantees the user a
   ;; match with the corresponding regexps.  Non-matching switches are
-  ;; shown literally.
+  ;; shown literally if user has not disabled displaying them by
+  ;; customizing dired-display-listing-switches variable.
   (when (eq major-mode 'dired-mode)
-    (setq mode-name
-	  (let (case-fold-search)
-	    (cond ((string-match-p
-		    dired-sort-by-name-regexp dired-actual-switches)
-		   "Dired by name")
-		  ((string-match-p
-		    dired-sort-by-date-regexp dired-actual-switches)
-		   "Dired by date")
-		  (t
-		   (concat "Dired " dired-actual-switches)))))
-    (force-mode-line-update)))
+    (let* ((mode-line-info)
+           (case-fold-search))
+      (cond ((string-match-p
+	      dired-sort-by-name-regexp dired-actual-switches)
+	     (setq mode-line-info " by name"))
+	    ((string-match-p
+	      dired-sort-by-date-regexp dired-actual-switches)
+	     (setq mode-line-info " by date"))
+	    (t
+	     (setq mode-line-info (concat " " dired-actual-switches))))
+      mode-line-info)))
 
 (define-obsolete-function-alias 'dired-sort-set-modeline
   #'dired-sort-set-mode-line "24.3")
@@ -4174,7 +4196,7 @@
                                         dired-actual-switches)
                         "t"
                       " -t")))))
-  (dired-sort-set-mode-line)
+  (dired-set-mode-line)
   (revert-buffer))
 
 ;; Some user code loads dired especially for this.
@@ -4197,7 +4219,7 @@
 With optional second arg NO-REVERT, don't refresh the listing afterwards."
   (dired-sort-R-check switches)
   (setq dired-actual-switches switches)
-  (dired-sort-set-mode-line)
+  (dired-set-mode-line)
   (or no-revert (revert-buffer)))
 
 (defvar-local dired-subdir-alist-pre-R nil
@@ -4233,7 +4255,6 @@
 	    ;; No pre-R subdir alist, so revert to main directory
 	    ;; listing:
 	    (list (car (reverse dired-subdir-alist))))))))
-\f
 
 ;;;;  Drag and drop support
 
@@ -4337,7 +4358,6 @@
   (let ((local-file (dnd-get-local-file-uri uri)))
     (if local-file (dired-dnd-handle-local-file local-file action)
       nil)))
-\f
 
 ;;;;  Desktop support