Subject: bug#30285: dired-do-chmod vs. top line of dired

From: Juri Linkov <[email-masked]>
Date: Sun, 04 Feb 2018 23:44:09 +0200

>> Agreed.  Simplicity is the hallmark of Emacs.
>
> Wouldn't this
>
> -  (let* ((files (dired-get-marked-files t arg))
> +  (let* ((files (or (dired-get-marked-files t arg)
> +                    (user-error "No files specified")))
>
> call for an extra argument to 'dired-get-marked-files' to emit the
> user error right there?  If it's TRT in your cases, it might give
> coders a heads-up that it's TRT in their cases as well.

Usually we have an arg NOERROR to not signal an error when it's non-nil.
But since we should keep backward-compatibility, the arg should be
opt-in rather than opt-out here:

diff --git a/lisp/dired.el b/lisp/dired.el
index eade11b..ef069d2 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -645,7 +645,7 @@ dired-map-over-marks
      ;; save-excursion loses, again
      (dired-move-to-filename)))
 
-(defun dired-get-marked-files (&optional localp arg filter distinguish-one-marked)
+(defun dired-get-marked-files (&optional localp arg filter distinguish-one-marked error)
   "Return the marked files' names as list of strings.
 The list is in the same order as the buffer, that is, the car is the
   first marked file.
@@ -662,7 +662,10 @@ dired-get-marked-files
 
 If DISTINGUISH-ONE-MARKED is non-nil, then if we find just one marked file,
 return (t FILENAME) instead of (FILENAME).
-Don't use that together with FILTER."
+Don't use that together with FILTER.
+
+If ERROR is non-nil, signal an error when the list of found files is empty.
+ERROR can be a string with the error message."
   (let ((all-of-them
 	 (save-excursion
 	   (delq nil (dired-map-over-marks
@@ -672,13 +675,17 @@ dired-get-marked-files
     (when (equal all-of-them '(t))
       (setq all-of-them nil))
     (if (not filter)
-	(if (and distinguish-one-marked (eq (car all-of-them) t))
-	    all-of-them
-	  (nreverse all-of-them))
+	(setq result
+              (if (and distinguish-one-marked (eq (car all-of-them) t))
+	          all-of-them
+	        (nreverse all-of-them)))
       (dolist (file all-of-them)
 	(if (funcall filter file)
-	    (push file result)))
-      result)))
+	    (push file result))))
+    (when (and (null result) error)
+      (user-error (if (stringp error) error "No files specified")))
+    result))
+
 
 ;; The dired command
 
diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 55b68a3..6e3e336 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -301,7 +301,7 @@ dired-do-chxxx
   ;; PROGRAM is the program used to change the attribute.
   ;; OP-SYMBOL is the type of operation (for use in `dired-mark-pop-up').
   ;; ARG describes which files to use, as in `dired-get-marked-files'.
-  (let* ((files (dired-get-marked-files t arg))
+  (let* ((files (dired-get-marked-files t arg nil nil t))
 	 ;; The source of default file attributes is the file at point.
 	 (default-file (dired-get-filename t t))
 	 (default (when default-file
@@ -361,7 +361,7 @@ dired-do-chmod
 Type M-n to pull the file attributes of the file at point
 into the minibuffer."
   (interactive "P")
-  (let* ((files (dired-get-marked-files t arg))
+  (let* ((files (dired-get-marked-files t arg nil nil t))
 	 ;; The source of default file attributes is the file at point.
 	 (default-file (dired-get-filename t t))
 	 (modestr (when default-file
@@ -476,7 +476,7 @@ dired-do-print
 `lpr-switches' as default."
   (interactive "P")
   (require 'lpr)
-  (let* ((file-list (dired-get-marked-files t arg))
+  (let* ((file-list (dired-get-marked-files t arg nil nil t))
 	 (lpr-switches
 	  (if (and (stringp printer-name)
 		   (string< "" printer-name))
@@ -666,7 +666,7 @@ dired-do-async-shell-command
 
 The output appears in the buffer `*Async Shell Command*'."
   (interactive
-   (let ((files (dired-get-marked-files t current-prefix-arg)))
+   (let ((files (dired-get-marked-files t current-prefix-arg nil nil t)))
      (list
       ;; Want to give feedback whether this file or marked files are used:
       (dired-read-shell-command "& on %s: " current-prefix-arg files)
@@ -727,7 +727,7 @@ dired-do-shell-command
 ;;Functions dired-run-shell-command and dired-shell-stuff-it do the
 ;;actual work and can be redefined for customization.
   (interactive
-   (let ((files (dired-get-marked-files t current-prefix-arg)))
+   (let ((files (dired-get-marked-files t current-prefix-arg nil nil t)))
      (list
       ;; Want to give feedback whether this file or marked files are used:
       (dired-read-shell-command "! on %s: " current-prefix-arg files)
@@ -1030,7 +1030,7 @@ dired-do-compress-to
 Choose the archiving command based on the archive file-name extension
 and `dired-compress-files-alist'."
   (interactive)
-  (let* ((in-files (dired-get-marked-files))
+  (let* ((in-files (dired-get-marked-files nil nil nil nil t))
          (out-file (expand-file-name (read-file-name "Compress to: ")))
          (rule (cl-find-if
                 (lambda (x)
@@ -1153,7 +1153,7 @@ dired-mark-confirm
       ;; Pass t for DISTINGUISH-ONE-MARKED so that a single file which
       ;; is marked pops up a window.  That will help the user see
       ;; it isn't the current line file.
-      (let ((files (dired-get-marked-files t arg nil t))
+      (let ((files (dired-get-marked-files t arg nil t t))
 	    (string (if (eq op-symbol 'compress) "Compress or uncompress"
 		      (capitalize (symbol-name op-symbol)))))
 	(dired-mark-pop-up nil op-symbol files #'y-or-n-p
@@ -1845,7 +1845,7 @@ dired-do-create-files
       The rest of into-dir are optional arguments.
    For any other return value, TARGET is treated as a directory."
   (or op1 (setq op1 operation))
-  (let* ((fn-list (dired-get-marked-files nil arg))
+  (let* ((fn-list (dired-get-marked-files nil arg nil nil t))
 	 (rfn-list (mapcar #'dired-make-relative fn-list))
 	 (dired-one-file	; fluid variable inside dired-create-files
 	  (and (consp fn-list) (null (cdr fn-list)) (car fn-list)))
@@ -2799,14 +2799,14 @@ dired-do-isearch
   "Search for a string through all marked files using Isearch."
   (interactive)
   (multi-isearch-files
-   (dired-get-marked-files nil nil 'dired-nondirectory-p)))
+   (dired-get-marked-files nil nil 'dired-nondirectory-p nil t)))
 
 ;;;###autoload
 (defun dired-do-isearch-regexp ()
   "Search for a regexp through all marked files using Isearch."
   (interactive)
   (multi-isearch-files-regexp
-   (dired-get-marked-files nil nil 'dired-nondirectory-p)))
+   (dired-get-marked-files nil nil 'dired-nondirectory-p nil t)))
 
 ;;;###autoload
 (defun dired-do-search (regexp)
@@ -2827,7 +2827,7 @@ dired-do-query-replace-regexp
 	  (query-replace-read-args
 	   "Query replace regexp in marked files" t t)))
      (list (nth 0 common) (nth 1 common) (nth 2 common))))
-  (dolist (file (dired-get-marked-files nil nil 'dired-nondirectory-p))
+  (dolist (file (dired-get-marked-files nil nil 'dired-nondirectory-p nil t))
     (let ((buffer (get-file-buffer file)))
       (if (and buffer (with-current-buffer buffer
 			buffer-read-only))
@@ -2851,7 +2851,7 @@ dired-do-find-regexp
   (require 'grep)
   (defvar grep-find-ignored-files)
   (defvar grep-find-ignored-directories)
-  (let* ((files (dired-get-marked-files))
+  (let* ((files (dired-get-marked-files nil nil nil nil t))
          (ignores (nconc (mapcar
                           (lambda (s) (concat s "/"))
                           grep-find-ignored-directories)
diff --git a/lisp/dired-x.el b/lisp/dired-x.el
index a90f1f4..fa36083 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -1335,7 +1335,8 @@ dired-do-find-marked-files
 To keep Dired buffer displayed, type \\[split-window-below] first.
 To display just marked files, type \\[delete-other-windows] first."
   (interactive "P")
-  (dired-simultaneous-find-file (dired-get-marked-files) noselect))
+  (dired-simultaneous-find-file (dired-get-marked-files nil nil nil nil t)
+                                noselect))
 
 (defun dired-simultaneous-find-file (file-list noselect)
   "Visit all files in FILE-LIST and display them simultaneously.