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

[Tino Calancha <tino.calancha@gmail.com>]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tino Calancha <tino.calancha@gmail.com>
>> Date: Tue, 30 Jan 2018 00:14:00 +0900
>> Cc: 30285@debbugs.gnu.org
>> 
>> > (Why doesn't it just complain "can't operate on" like it does for the
>> > third line, ".".)
>> Following patch just do nothing in these cases.  That's OK for me.
>> Do you prefer to inform the user in this case that there is no file
>> to change the mode?
>
> Yes, I think we should produce some message in these cases.
OK.  Then, we must adjust other siblings commands (dired-do-chgrp,
dired-do-chown);  otherwise they might become jealous.
I propose to add a new predicate
`dired-marked-files-or-file-at-point-p', and used it in all those commands.
--8<-----------------------------cut here---------------start------------->8---
commit a66d02f56f3b1019009c21997b0064f1fb26a03f
Author: tino calancha <tino.calancha@gmail.com>
Date:   Tue Jan 30 08:18:07 2018 +0900

    dired-do-chmod: Avoid unecessary prompt
    
    Prompt user only if there are any marked files or a
    file at point (Bug#30285).
    * lisp/dired.el (dired-marked-files-or-file-at-point-p): New defun.
    
    * lisp/dired-aux.el (dired-do-chmod)
    (dired-do-chmod, dired-do-chgrp, dired-do-chown): Use it.

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 55b68a372e..02febbe570 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -361,40 +361,42 @@ 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))
-	 ;; The source of default file attributes is the file at point.
-	 (default-file (dired-get-filename t t))
-	 (modestr (when default-file
-		    (nth 8 (file-attributes default-file))))
-	 (default
-	   (and (stringp modestr)
-		(string-match "^.\\(...\\)\\(...\\)\\(...\\)$" modestr)
-		(replace-regexp-in-string
-		 "-" ""
-		 (format "u=%s,g=%s,o=%s"
-			 (match-string 1 modestr)
-			 (match-string 2 modestr)
-			 (match-string 3 modestr)))))
-	 (modes (dired-mark-read-string
-		 "Change mode of %s to: "
-		 nil 'chmod arg files default))
-	 num-modes)
-    (cond ((or (equal modes "")
-	       ;; Use `eq' instead of `equal'
-	       ;; to detect empty input (bug#12399).
-	       (eq modes default))
-	   ;; We used to treat empty input as DEFAULT, but that is not
-	   ;; such a good idea (Bug#9361).
-	   (error "No file mode specified"))
-	  ((string-match-p "^[0-7]+" modes)
-	   (setq num-modes (string-to-number modes 8))))
-
-    (dolist (file files)
-      (set-file-modes
-       file
-       (if num-modes num-modes
-	 (file-modes-symbolic-to-number modes (file-modes file)))))
-    (dired-do-redisplay arg)))
+  (if (not (dired-marked-files-or-file-at-point-p))
+      (user-error "No marked files")
+    (let* ((files (dired-get-marked-files t arg))
+	   ;; The source of default file attributes is the file at point.
+	   (default-file (dired-get-filename t t))
+	   (modestr (when default-file
+		      (nth 8 (file-attributes default-file))))
+	   (default
+	     (and (stringp modestr)
+		  (string-match "^.\\(...\\)\\(...\\)\\(...\\)$" modestr)
+		  (replace-regexp-in-string
+		   "-" ""
+		   (format "u=%s,g=%s,o=%s"
+			   (match-string 1 modestr)
+			   (match-string 2 modestr)
+			   (match-string 3 modestr)))))
+	   (modes (dired-mark-read-string
+		   "Change mode of %s to: "
+		   nil 'chmod arg files default))
+	   num-modes)
+      (cond ((or (equal modes "")
+	         ;; Use `eq' instead of `equal'
+	         ;; to detect empty input (bug#12399).
+	         (eq modes default))
+	     ;; We used to treat empty input as DEFAULT, but that is not
+	     ;; such a good idea (Bug#9361).
+	     (error "No file mode specified"))
+	    ((string-match-p "^[0-7]+" modes)
+	     (setq num-modes (string-to-number modes 8))))
+
+      (dolist (file files)
+        (set-file-modes
+         file
+         (if num-modes num-modes
+	   (file-modes-symbolic-to-number modes (file-modes file)))))
+      (dired-do-redisplay arg))))
 
 ;;;###autoload
 (defun dired-do-chgrp (&optional arg)
@@ -404,7 +406,9 @@ dired-do-chgrp
   (interactive "P")
   (if (memq system-type '(ms-dos windows-nt))
       (error "chgrp not supported on this system"))
-  (dired-do-chxxx "Group" "chgrp" 'chgrp arg))
+  (if (not (dired-marked-files-or-file-at-point-p))
+      (user-error "No marked files")
+    (dired-do-chxxx "Group" "chgrp" 'chgrp arg)))
 
 ;;;###autoload
 (defun dired-do-chown (&optional arg)
@@ -414,7 +418,9 @@ dired-do-chown
   (interactive "P")
   (if (memq system-type '(ms-dos windows-nt))
       (error "chown not supported on this system"))
-  (dired-do-chxxx "Owner" dired-chown-program 'chown arg))
+  (if (not (dired-marked-files-or-file-at-point-p))
+      (user-error "No marked files")
+    (dired-do-chxxx "Owner" dired-chown-program 'chown arg)))
 
 ;;;###autoload
 (defun dired-do-touch (&optional arg)
@@ -423,7 +429,9 @@ dired-do-touch
 Type M-n to pull the file attributes of the file at point
 into the minibuffer."
   (interactive "P")
-  (dired-do-chxxx "Timestamp" dired-touch-program 'touch arg))
+  (if (not (dired-marked-files-or-file-at-point-p))
+      (user-error "No marked files")
+    (dired-do-chxxx "Timestamp" dired-touch-program 'touch arg)))
 
 ;; Process all the files in FILES in batches of a convenient size,
 ;; by means of (FUNCALL FUNCTION ARGS... SOME-FILES...).
diff --git a/lisp/dired.el b/lisp/dired.el
index eade11bc7f..79486c8b8e 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2385,6 +2385,11 @@ dired-get-filename
      (t
       (concat (dired-current-directory localp) file)))))
 
+(defun dired-marked-files-or-file-at-point-p ()
+  "Return non-nil if there are marked files or a file at point."
+  (and (or (cdr (dired-get-marked-files nil nil nil 'distinguish-1-marked))
+           (dired-get-filename nil 'no-error)) t))
+
 (defun dired-string-replace-match (regexp string newtext
                                    &optional literal global)
   "Replace first match of REGEXP in STRING with NEWTEXT.
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 27.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
Repository revision: 29abae3572090a86beedb66822ccf34356c8a00c