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

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

[積丹尼 Dan Jacobson]

M runs the command dired-do-chmod.
If used on the very top line of dired
(the directory name,) it says:
"Change mode of * [0 files] to: "
which doesn't make a lot of sense.
emacs-version "25.2.2"

Same problem if used on the second line, (total...).

(Why doesn't it just complain "can't operate on" like it does for the
third line, ".".)

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

[Tino Calancha]

積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:

Thank you for your report.
> M runs the command dired-do-chmod.
> If used on the very top line of dired
> (the directory name,) it says:
> "Change mode of * [0 files] to: "
> which doesn't make a lot of sense.
Agreed.
> emacs-version "25.2.2"
>
> Same problem if used on the second line, (total...).
>
> (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?

--8<-----------------------------cut here---------------start------------->8---
commit 9b10f6ef6fbb3e6c50ffb04b97fc5b0b86d3e559
Author: tino calancha <tino.calancha@gmail.com>
Date:   Tue Jan 30 00:02:00 2018 +0900

    dired-do-chmod: Avoid unecessary prompt
    
    * lisp/dired-aux.el (dired-do-chmod): Offer to change mode just if
    there are any marked files or there is a file at point (Bug#30285).

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 55b68a372e..3868efe0c1 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)))
+  (when (or (cdr (dired-get-marked-files nil nil nil 'distinguish-1-marked))
+            (dired-get-filename t t))
+    (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)

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 27.0.50 (build 7, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
Repository revision: ea8c0e1b9eaa6651919fb4e039e3fcb5a1fa73db

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

[積丹尼 Dan Jacobson]

>>>>> "TC" == Tino Calancha <tino.calancha@gmail.com> writes:
TC> Do you prefer to inform the user in this case that there is no file
TC> to change the mode?

(All I know is I found it odd I couldn't change . or .. but OK you must
have your reasons. Maybe say "use a prefix argument to really change . "...)

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

[Eli Zaretskii]

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

Thanks.

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

[Tino Calancha]

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



On Mon, 29 Jan 2018, 積丹尼 Dan Jacobson wrote:

>>>>>> "TC" == Tino Calancha <tino.calancha@gmail.com> writes:
> TC> Do you prefer to inform the user in this case that there is no file
> TC> to change the mode?
>
> (All I know is I found it odd I couldn't change . or .. but OK you must
> have your reasons. Maybe say "use a prefix argument to really change . "...)

I'd rather prefer fix the case when:
* there is no marked files
** We are in a line without file at point.

Whether if an user should be able to `chmod' '.' or '..' seems
a bit orthogonal to *, ** above; maybe better to discuss such
issue in a separated bug report.

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

[Tino Calancha]

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

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

[Drew Adams]

> >> > (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.

Please don't do any such thing.

Yes, it makes sense for such commands to do nothing or to show an
error message when on the "top line of dired", as described in the
bug report.

No, we don't need a function `dired-marked-files-or-file-at-point-p',
for that or anything else.  The `dired-do-*' commands already DTRT
wrt the marked-files-or-file-at-point.

And no, it doesn't make sense to act that way on `.' - it's OK to
change the properties of the current directory (provided you have
the necessary permissions).  And that's not even part of this bug
report, is it?

The question of `..' is arguable, but I'd say the same thing for
it as for `.': It's OK to change its properties, provided you have
permission to do so.  After all, `..' is just a (unique) directory.

How did this bug report move from being about behavior on the top
line (and the second, "total" etc. line) to being also about the
lines for `.' and `..'?

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

[Tino Calancha]

On Mon, 29 Jan 2018, Drew Adams wrote:

>>>>> (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.
>
> Please don't do any such thing.
>
> Yes, it makes sense for such commands to do nothing or to show an
> error message when on the "top line of dired", as described in the
> bug report.
OK, I see you agree with Eli and me.  the rest I believe is just 
funny misunderstanding :-)

> No, we don't need a function `dired-marked-files-or-file-at-point-p',
> for that or anything else.
Probably not, but it looks tidy in my patch to add one to reinforce DRY.
> The `dired-do-*' commands already DTRT wrt the marked-files-or-file-at-point.
No, they don't.  They annoying users asking a useless prompt, like:

Change mode of * [0 files] to:
;; Just to notify the user after his input:
No file on this line

This is like if I ask my gf:

Dear, do you want I change diapers to [0 of our children]?
;; After she answer...
Ohhh, I just remembered we have no kids!!!
;; After that silly question probably I will not have gf either...

> And no, it doesn't make sense to act that way on `.' - it's OK to
> change the properties of the current directory (provided you have
> the necessary permissions).
Indeed, I don't want to change that and I agree with you.  I just offered
the OP to open another bug report with this topic if he likes.  This bug
report is just about the unnecessary prompt in the top line.

> The question of `..' is arguable, but I'd say the same thing for
> it as for `.': It's OK to change its properties, provided you have
> permission to do so.  After all, `..' is just a (unique) directory.
Nobody said the opposite :-D
>
> How did this bug report move from being about behavior on the top
> line (and the second, "total" etc. line) to being also about the
> lines for `.' and `..'?
No idea... maybe it just happened in your mind, or you had a dream
last night about it ;-)

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

[Drew Adams]

> > The `dired-do-*' commands already DTRT wrt the marked-files-or-file-
> > at-point.
>
> No, they don't.  They annoying users asking a useless prompt, like:
> 
> Change mode of * [0 files] to:
> ;; Just to notify the user after his input:
> No file on this line

I see that only for the reported places (this bug).
Dunno if that's what you meant.

Otherwise I don't see that at all.  With point on a file
or dir line (even on `.' or `..') and no files marked I
get this prompt, where THE-FILE-OR-DIR is the name of
the file or directory:

 Change mode of `THE-FILE-OR-DIR' to: 

IOW, except for the bug case it works fine, and there
is no function `dired-marked-files-or-file-at-point-p'.
Likewise, for other `dired-do-*' commands.

The marked-files-or-file-at-point behavior is done by
`dired-get-marked-files'.  That's exactly what it does.

(defun dired-do-chmod (&optional arg)
  "..."
  (interactive "P")
  (let* ((files (dired-get-marked-files t arg)) ; <======
    ...))

All that's needed is to test for FILES = nil.

> This bug report is just about the unnecessary prompt in the top line.

OK.  We agree that the bug is only about the top line (or
top two lines, if details are not hidden).

Well, not really.  It's really about point on any line where
there is no file or dir.  That includes:

1. The empty line at the end of the buffer.
2. The empty line before each inserted subdir listing.
3. The first two lines of the main dir and of each inserted
   subdir - that is, the highlighted line with the full
   (sub)dir name and the "total..." information line that
   follows it.

The right thing to do is to test first for whether point
is on a file or dir line.  There are several ways to do
that.

One is shown above: (dired-get-marked-files t arg), and
that's already used by commands such as `dired-do-chmod'.
We just need to test its return value.

Another is to use (dired-move-to-filename).  One of the
main uses of that simple function is to test whether or
not point is on a file or dir line.

A third way, more costly, is to use `dired-get-filename',

Some of these ways give you a choice of whether to raise
an error or just return nil when point is not on a file
or dir line, i.e., when we are in one of the cases where
we don't want to try to act on the indicated file(s),
because there are none.

For `dired-do-chmod' and similar, I'd say just test the
return value of `dired-get-marked-files' which is already
called at the beginning of the function.  If, in some
other `dired-do-*' command that function is not called at
the beginning anyway, just use `dired-move-to-filename': 

(defun dired-do-whatever (&optional arg)
  "..."
  (interactive (progn (dired-move-to-filename 'RAISE-ERROR)
                      (list current-prefix-arg))))
  ...)

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

[Drew Adams]

Sorry; I realize that my last msg was probably not clear
about the possibility of using `dired-move-to-filename'
or `dired-get-filename'.

Obviously, for `dired-do-*' commands we still need to
handle the case where files or dirs are marked.  And
in that case the command should DTRT even if called from
a non-file line, i.e., one of the problematic places
we've been discussing - it should not raise an error
in that context.

For `dired-do-*' commands that already call
`dired-get-marked-files' the clear solution, I think,
is to just test that return value and raise an error
if it is nil.

If there is are `dired-do-*' commands that do not call
`dired-get-marked-files' then we have a choice how to
solve the problem.  But it might well be that even then
the best solution is to use `dired-get-marked-files'.

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

[Tino Calancha]

On Tue, 30 Jan 2018, Drew Adams wrote:

> Obviously, for `dired-do-*' commands we still need to
> handle the case where files or dirs are marked.  And
> in that case the command should DTRT even if called from
> a non-file line, i.e., one of the problematic places
> we've been discussing - it should not raise an error
> in that context.
IIRC, my latest patch handle those situations pretty well.
Could you tested it and provide feedback about how to
improve it?

> For `dired-do-*' commands that already call
> `dired-get-marked-files' the clear solution, I think,
> is to just test that return value and raise an error
> if it is nil.
>
> If there is are `dired-do-*' commands that do not call
> `dired-get-marked-files' then we have a choice how to
> solve the problem.  But it might well be that even then
> the best solution is to use `dired-get-marked-files'.
Since you are reluctant to the addition of the new predicate.  That's 
fine.
May I ask you to provide an alternative  patch to compare
with mine? Then, people here might do further feedback based
on those 2 alternatives.

Thank you very much!

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

[Drew Adams]

> > Obviously, for `dired-do-*' commands we still need to
> > handle the case where files or dirs are marked.  And
> > in that case the command should DTRT even if called from
> > a non-file line, i.e., one of the problematic places
> > we've been discussing - it should not raise an error
> > in that context.
>
> IIRC, my latest patch handle those situations pretty well.
> Could you tested it and provide feedback about how to
> improve it?
> 
> > For `dired-do-*' commands that already call
> > `dired-get-marked-files' the clear solution, I think,
> > is to just test that return value and raise an error
> > if it is nil.
> >
> > If there are `dired-do-*' commands that do not call
> > `dired-get-marked-files' then we have a choice how to
> > solve the problem.  But it might well be that even then
> > the best solution is to use `dired-get-marked-files'.
>
> Since you are reluctant to the addition of the new predicate.
> That's fine.
>
> May I ask you to provide an alternative  patch to compare
> with mine? Then, people here might do further feedback based
> on those 2 alternatives.

Sorry; I don't have time to work on this.  I've already
provided my suggestions - hope they help. Whatever you
decide is fine by me.  Thx.

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

[Juri Linkov]

>> I propose to add a new predicate
>> `dired-marked-files-or-file-at-point-p', and used it in all those
>> commands.
>
> Please don't do any such thing.
>
> Yes, it makes sense for such commands to do nothing or to show an
> error message when on the "top line of dired", as described in the
> bug report.

Instead of doing nothing or showing an error message, how about
doing a more useful thing: when on the top line, ‘dired-do-chmod’
could do chmod on all files in the dir.

This is exactly what other Dired commands already do: e.g. typing ‘m’
on the top line or on any other subdir headerline, they perform
their actions on all files.

For example, see the docstring of ‘dired-mark’:

  “If on a subdir headerline, mark all its files except `.' and `..'.”

> No, we don't need a function `dired-marked-files-or-file-at-point-p',
> for that or anything else.  The `dired-do-*' commands already DTRT
> wrt the marked-files-or-file-at-point.

I agree that it's better to check the ‘files’ returned from
‘dired-get-marked-files’.

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

[Drew Adams]

> > Yes, it makes sense for such commands to do nothing or to show an
> > error message when on the "top line of dired", as described in the
> > bug report.
> 
> Instead of doing nothing or showing an error message, how about
> doing a more useful thing: when on the top line, ‘dired-do-chmod’
> could do chmod on all files in the dir.

Not a good idea, IMO.  Too easy to do accidentally.  At the
very least it would need to ask for confirmation specially.

`m', `d', and `w', which are the keys that you are talking
about, to not do anything to the files in question.  They
affect only the Dired listing or the `kill-ring'.  That's
quite different from something like `chmod'.

And in any case, such shortcut behavior is not needed at all
(see next).

> This is exactly what other Dired commands already do: e.g. typing ‘m’
> on the top line or on any other subdir headerline, they perform
> their actions on all files.

Which is why what you propose isn't needed.  Just `m' then `M'.

If someone is going to act on lots of files, s?he had better
be aware of that.  Best is that they are first marked before
acting on them.

But yes, it's true that not needing to change marks can be
handy.  I.e., you have some files marked for a given reason
already, and you want to keep those markings.  (You could
use `* c' to temporarily change marks, but that's a bit
roundabout.)

That handiness (not losing existing markings) is the reason
why in my Dired+ code, for commands that normally act on the
marked files (or the N next files, with numeric prefix arg N),
you can use multiple plain `C-u' to act on all files, ignoring
markings:

Just what "all" files means changes with the number of `C-u',
as follows:

 `C-u C-u'         - Use all files present, but no directories.
 `C-u C-u C-u'     - Use all files and dirs except `.' and `..'.
 `C-u C-u C-u C-u' - use all files and dirs, `.' and `..'.

 (More than four `C-u' act the same as two.)

But I don't think that's a good idea in the current context.
I'd suggest that we just let someone use `m' (on that
non-file line) followed by `M' etc.

I think that this bug should be handled by doing what Dired
usually does when point is on a non-file line (_anywhere_,
not just on a directory header line or its "total..." line) -
as I said earlier: just raise a `user-error'.

You'll note too that `m' on a non-file line other than
the dir header line does _not_ do what you describe.

* On the "total..." line it marks the first file line
  (which is now `..' in Windows but used to be `.').

* On the blank line before an inserted subdir header it
  does the same thing: it marks the first file line of
  that subdir listing.

* On any other non-file line, such as the blank line at
  the end of the buffer, it does nothing at all.

So the `m' (`dired-mark') behavior is quite variable, even
if it can be useful.

Note too that the other two commands that act specially on
a (sub)dir header line do not do the same thing as `dired-mark'.

* `dired-copy-filename-as-kill' does not act similarly at all.
  It copies the subdir name instead of names of any files in
  its listing.

* `dired-flag-file-deletion' does act somewhat similarly to
  `dired-mark': it flags all of the files (other than `.' and
  `..') in the subdir listing - except if the region is active.
  In the latter case it flags the files in the region.  And that
  need not mean any files in the subdir listing - it could be
  just files in the previous listing.  Or it could be files in
  the subdir listing plus files in other, subsequent subdir
  listings.

> For example, see the docstring of ‘dired-mark’:
>   “If on a subdir headerline, mark all its files except `.'
>    and `..'.”
> 
> > No, we don't need a function `dired-marked-files-or-file-at-point-p',
> > for that or anything else.  The `dired-do-*' commands already DTRT
> > wrt the marked-files-or-file-at-point.
> 
> I agree that it's better to check the ‘files’ returned from
> ‘dired-get-marked-files’.

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

[Tino Calancha]

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



On Wed, 31 Jan 2018, Juri Linkov wrote:

>>> I propose to add a new predicate
>>> `dired-marked-files-or-file-at-point-p', and used it in all those
>>> commands.
>>
>> Please don't do any such thing.
>>
>> Yes, it makes sense for such commands to do nothing or to show an
>> error message when on the "top line of dired", as described in the
>> bug report.
>
> Instead of doing nothing or showing an error message, how about
> doing a more useful thing: when on the top line, ‘dired-do-chmod’
> could do chmod on all files in the dir.
>
> This is exactly what other Dired commands already do: e.g. typing ‘m’
> on the top line or on any other subdir headerline, they perform
> their actions on all files.
>
> For example, see the docstring of ‘dired-mark’:
>
>  “If on a subdir headerline, mark all its files except `.' and `..'.”
Yeah, that's another possibility (not my preference).

IMO marking commands are at at different level than commands that operate
on marked files; we don't need to mimic such feature of the `dired-mark'. 
Indeed, if the user want to operate on all files, she can easily do 
`dired-mark' followed by the command in question;  I tend to think calling 
`dired-do...' things without marked files from the top line as an user mistake.

I would like all `dired-do...' commands behave the same under the
'X condition':
* called from the top line
** no marked files.


>> No, we don't need a function `dired-marked-files-or-file-at-point-p',
>> for that or anything else.  The `dired-do-*' commands already DTRT
>> wrt the marked-files-or-file-at-point.
>
> I agree that it's better to check the ‘files’ returned from
> ‘dired-get-marked-files’.

Today I took a deeper look in the train and I saw there are several more
commands that don't protect against X.  Some even breaks
(e.g., dired-do-shell-command, dired-do-async-shell-command).

Below patch introduce a macro to systematically handle the 'X condition',
what do you think?

--8<-----------------------------cut here---------------start------------->8---
commit 193ba8fe6225093a0fc96e4bea7eec21a1643d4b
Author: tino calancha <tino.calancha@gmail.com>
Date:   Thu Feb 1 10:32:30 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.
     (dired-with-dired-do): New macro.

     * lisp/dired-aux.el (dired-do-chmod)
     (dired-do-chmod, dired-do-chgrp, dired-do-chown)
     (dired-do-touch, dired-do-print, dired-do-async-shell-command)
     (dired-do-shell-command, dired-query, dired-byte-compile)
     (dired-load, dired-do-copy, dired-do-symlink, dired-do-hardlink)
     (dired-do-rename, dired-isearch-filenames-regexp, dired-do-find-regexp)
     (dired-do-find-regexp-and-replace)
     * lisp/dired-x.el (dired-do-relsymlink, dired-do-find-marked-files):
     Use it.

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 55b68a372e..f5f3311ead 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -361,40 +361,41 @@ 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)))
+  (dired-with-dired-do
+    (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 +405,8 @@ 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))
+  (dired-with-dired-do
+    (dired-do-chxxx "Group" "chgrp" 'chgrp arg)))

  ;;;###autoload
  (defun dired-do-chown (&optional arg)
@@ -414,7 +416,8 @@ 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))
+  (dired-with-dired-do
+    (dired-do-chxxx "Owner" dired-chown-program 'chown arg)))

  ;;;###autoload
  (defun dired-do-touch (&optional arg)
@@ -423,7 +426,8 @@ 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))
+  (dired-with-dired-do
+    (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...).
@@ -476,23 +480,24 @@ dired-do-print
  `lpr-switches' as default."
    (interactive "P")
    (require 'lpr)
-  (let* ((file-list (dired-get-marked-files t arg))
-	 (lpr-switches
-	  (if (and (stringp printer-name)
-		   (string< "" printer-name))
-	      (cons (concat lpr-printer-switch printer-name)
-		    lpr-switches)
-	    lpr-switches))
-	 (command (dired-mark-read-string
-		   "Print %s with: "
- 		   (mapconcat 'identity
-			      (cons lpr-command
-				    (if (stringp lpr-switches)
-					(list lpr-switches)
-				      lpr-switches))
-			      " ")
-		   'print arg file-list)))
-    (dired-run-shell-command (dired-shell-stuff-it command file-list nil))))
+  (dired-with-dired-do
+    (let* ((file-list (dired-get-marked-files t arg))
+	   (lpr-switches
+	    (if (and (stringp printer-name)
+		     (string< "" printer-name))
+	        (cons (concat lpr-printer-switch printer-name)
+		      lpr-switches)
+	      lpr-switches))
+	   (command (dired-mark-read-string
+		     "Print %s with: "
+		     (mapconcat 'identity
+			        (cons lpr-command
+				      (if (stringp lpr-switches)
+					  (list lpr-switches)
+				        lpr-switches))
+			        " ")
+		     'print arg file-list)))
+      (dired-run-shell-command (dired-shell-stuff-it command file-list nil)))))

  (defun dired-mark-read-string (prompt initial op-symbol arg files
  			       &optional default-value collection)
@@ -666,7 +671,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)))
+   (dired-with-dired-do
       (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 +732,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)))
+   (dired-with-dired-do
       (list
        ;; Want to give feedback whether this file or marked files are used:
        (dired-read-shell-command "! on %s: " current-prefix-arg files)
@@ -1224,7 +1229,8 @@ dired-query
  (defun dired-do-compress (&optional arg)
    "Compress or uncompress marked (or next ARG) files."
    (interactive "P")
-  (dired-map-over-marks-check #'dired-compress arg 'compress t))
+  (dired-with-dired-do
+    (dired-map-over-marks-check #'dired-compress arg 'compress t)))

  ;; Commands for Emacs Lisp files - load and byte compile

@@ -1252,7 +1258,8 @@ dired-byte-compile
  (defun dired-do-byte-compile (&optional arg)
    "Byte compile marked (or next ARG) Emacs Lisp files."
    (interactive "P")
-  (dired-map-over-marks-check #'dired-byte-compile arg 'byte-compile t))
+  (dired-with-dired-do
+    (dired-map-over-marks-check #'dired-byte-compile arg 'byte-compile t)))

  (defun dired-load ()
    ;; Return nil for success, offending file name else.
@@ -1269,7 +1276,8 @@ dired-load
  (defun dired-do-load (&optional arg)
    "Load the marked (or next ARG) Emacs Lisp files."
    (interactive "P")
-  (dired-map-over-marks-check #'dired-load arg 'load t))
+  (dired-with-dired-do
+    (dired-map-over-marks-check #'dired-load arg 'load t)))

  ;;;###autoload
  (defun dired-do-redisplay (&optional arg test-for-subdir)
@@ -2042,11 +2050,12 @@ dired-do-copy
  This command copies symbolic links by creating new ones, similar
  to the \"-d\" option for the \"cp\" shell command."
    (interactive "P")
-  (let ((dired-recursive-copies dired-recursive-copies))
-    (dired-do-create-files 'copy #'dired-copy-file
-			   "Copy"
-			   arg dired-keep-marker-copy
-			   nil dired-copy-how-to-fn)))
+  (dired-with-dired-do
+    (let ((dired-recursive-copies dired-recursive-copies))
+      (dired-do-create-files 'copy #'dired-copy-file
+			     "Copy"
+			     arg dired-keep-marker-copy
+			     nil dired-copy-how-to-fn))))

  ;;;###autoload
  (defun dired-do-symlink (&optional arg)
@@ -2060,8 +2069,9 @@ dired-do-symlink

  For relative symlinks, use \\[dired-do-relsymlink]."
    (interactive "P")
-  (dired-do-create-files 'symlink #'make-symbolic-link
-			   "Symlink" arg dired-keep-marker-symlink))
+  (dired-with-dired-do
+    (dired-do-create-files 'symlink #'make-symbolic-link
+			   "Symlink" arg dired-keep-marker-symlink)))

  ;;;###autoload
  (defun dired-do-hardlink (&optional arg)
@@ -2073,8 +2083,9 @@ dired-do-hardlink
  suggested for the target directory depends on the value of
  `dired-dwim-target', which see."
    (interactive "P")
-  (dired-do-create-files 'hardlink #'dired-hardlink
-			   "Hardlink" arg dired-keep-marker-hardlink))
+  (dired-with-dired-do
+    (dired-do-create-files 'hardlink #'dired-hardlink
+			   "Hardlink" arg dired-keep-marker-hardlink)))

  (defun dired-hardlink (file newname &optional ok-if-already-exists)
    (dired-handle-overwrite newname)
@@ -2092,8 +2103,9 @@ dired-do-rename
  The default suggested for the target directory depends on the value
  of `dired-dwim-target', which see."
    (interactive "P")
-  (dired-do-create-files 'move #'dired-rename-file
-			 "Move" arg dired-keep-marker-rename "Rename"))
+  (dired-with-dired-do
+    (dired-do-create-files 'move #'dired-rename-file
+			   "Move" arg dired-keep-marker-rename "Rename")))
  ;;;###end dired-cp.el

  ;;; 5K
@@ -2798,15 +2810,17 @@ dired-isearch-filenames-regexp
  (defun 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-with-dired-do
+    (multi-isearch-files
+     (dired-get-marked-files nil nil 'dired-nondirectory-p))))

  ;;;###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-with-dired-do
+    (multi-isearch-files-regexp
+     (dired-get-marked-files nil nil 'dired-nondirectory-p))))

  ;;;###autoload
  (defun dired-do-search (regexp)
@@ -2847,7 +2861,9 @@ dired-do-find-regexp
  directories.

  REGEXP should use constructs supported by your local `grep' command."
-  (interactive "sSearch marked files (regexp): ")
+  ;; (interactive "sSearch marked files (regexp): ")
+  (interactive
+   (dired-with-dired-do (list (read-string "Search marked files (regexp): "))))
    (require 'grep)
    (defvar grep-find-ignored-files)
    (defvar grep-find-ignored-directories)
@@ -2877,8 +2893,9 @@ dired-do-find-regexp-and-replace
  REGEXP should use constructs supported by your local `grep' command."
    (interactive
     (let ((common
-          (query-replace-read-args
-           "Query replace regexp in marked files" t t)))
+          (dired-with-dired-do
+            (query-replace-read-args
+             "Query replace regexp in marked files" t t))))
       (list (nth 0 common) (nth 1 common))))
    (with-current-buffer (dired-do-find-regexp from)
      (xref-query-replace-in-results from to)))
diff --git a/lisp/dired-x.el b/lisp/dired-x.el
index a90f1f4adc..f9aacc97b3 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -1282,8 +1282,9 @@ dired-do-relsymlink

  For absolute symlinks, use \\[dired-do-symlink]."
    (interactive "P")
-  (dired-do-create-files 'relsymlink #'dired-make-relative-symlink
-                           "RelSymLink" arg dired-keep-marker-relsymlink))
+  (dired-with-dired-do
+    (dired-do-create-files 'relsymlink #'dired-make-relative-symlink
+                           "RelSymLink" arg dired-keep-marker-relsymlink)))

  (autoload 'dired-mark-read-regexp "dired-aux")
  (autoload 'dired-do-create-files-regexp "dired-aux")
@@ -1335,7 +1336,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-with-dired-do
+    (dired-simultaneous-find-file (dired-get-marked-files) noselect)))

  (defun dired-simultaneous-find-file (file-list noselect)
    "Visit all files in FILE-LIST and display them simultaneously.
diff --git a/lisp/dired.el b/lisp/dired.el
index eade11bc7f..649214612b 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2385,6 +2385,22 @@ 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))
+
+;; Use this macro on `dired-do-' commands that accept as
+;; input the marked file or the file at point.
+(defmacro dired-with-dired-do (&rest body)
+  "Run BODY if there are marked files or a file at point.
+Signal an error if there is neither marked files nor a file at point.
+Return value of the last evaluated form in BODY."
+  (declare (debug (&body)) (indent 0))
+  `(if (null (dired-marked-files-or-file-at-point-p))
+       (user-error "No file on this line")
+     ,@body))
+
  (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 17, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
  of 2018-01-30 built on calancha-pc
Repository revision: 29abae3572090a86beedb66822ccf34356c8a00c

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

[Tino Calancha]

Tino Calancha <tino.calancha@gmail.com> writes:

Below the lines

-   (let ((files (dired-get-marked-files t current-prefix-arg)))

must not be deleted (its a typo), otherwise `files' is unbound.
Sorry for the typo (the train was really crowdy!) :-|

  (defun dired-mark-read-string (prompt initial op-symbol arg files
  			       &optional default-value collection)
@@ -666,7 +671,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)))
+   (dired-with-dired-do
       (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 +732,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)))
+   (dired-with-dired-do
       (list
        ;; Want to give feedback whether this file or marked files are used:
        (dired-read-shell-command "! on %s: " current-prefix-arg files)

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

[Drew Adams]

> I would like all `dired-do...' commands behave the same under the
> 'X condition': * called from the top line ** no marked files.

I've already said that it's not only about the top line.
It's about the ordinary Dired situation of not being on a
file line.  Plenty of Dired code already deals (simply)
with this "X condition".

The important things about a command that acts on the
marked files are that (1) it acts on the N next files,
if given a numeric prefix arg, (2) if no prefix arg,
it acts on the marked files, if any, (3) if not prefix
arg and no marked files it acts on the file of the
current line, and (4) it doesn't do something confusing
if there is no prefix arg, there are no marked files,
and there is no file on the current line. 

This bug is only about case (4) - a corner case.  The
solution is to just let the user know that s?he is not
on a file line.

> >> No, we don't need a function `dired-marked-files-or-file-at-point-p',
> >> for that or anything else.  The `dired-do-*' commands already DTRT
> >> wrt the marked-files-or-file-at-point.
> >
> > I agree that it's better to check the ‘files’ returned from
> > ‘dired-get-marked-files’.
> 
> Today I took a deeper look in the train and I saw there are
> several more commands that don't protect against X.  Some
> even breaks (e.g., dired-do-shell-command,
> dired-do-async-shell-command).
> 
> Below patch introduce a macro to systematically handle the 'X
> condition', what do you think?

Sorry, Tino, but I don't have the time or the will to
check your large patch.

My impression is that you guys are going overboard.

This bug is a _trivial_ usability bug.  There is nothing
really wrong with the existing behavior.  But yes, it
is unnecessary, and it could be slightly confusing, to
prompt the user about acting on zero files.

The trivial bug should be fixed; sure.  But it's not
a big deal.

The fix is also trivial.  Dired already provides what
is needed, as has been pointed out: just check the
return value of `dired-get-marked-files'.

If no files are marked then handle use of the command
as a `user-error': inform the user that there is no
file on the current line and no files are marked.
End of story.

This is trivial to do, and the resulting (corner case)
behavior will be simple, consistent, and clear to all
users.

If some `dired-do-*' command does not already use
`dired-get-marked-files' up front, and if it is not
appropriate for it to use it, then there are at least
two other alternatives that Dired offers to detect
this corner case.  I already mentioned them.  Using
either of them to fix the bug is also trivial.

In sum: trivial problem, trivial solution.

I'm not happy seeing big changes made to Dired code
gratuitously.  I don't have the time to argue about
it or provide alternative patches.  But I hope you
will not go down the road you seem to be going down.
Just one opinion.

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

[Juri Linkov]

> IMO marking commands are at at different level than commands that operate
> on marked files; we don't need to mimic such feature of the `dired-mark'.
> Indeed, if the user want to operate on all files, she can easily do
> `dired-mark' followed by the command in question;  I tend to think calling
> `dired-do...' things without marked files from the top line as an user mistake.

Since `dired-mark' from the top line followed by the command in question
is not obvious for users, we could provide a hint in the error message,
i.e. mention the availability of ‘m’ on the top line with such message:
“You can type `m' here to mark all files for this operation”.

>>> No, we don't need a function `dired-marked-files-or-file-at-point-p',
>>> for that or anything else.  The `dired-do-*' commands already DTRT
>>> wrt the marked-files-or-file-at-point.
>>
>> I agree that it's better to check the ‘files’ returned from
>> ‘dired-get-marked-files’.
>
> Today I took a deeper look in the train and I saw there are several more
> commands that don't protect against X.  Some even breaks
> (e.g., dired-do-shell-command, dired-do-async-shell-command).
>
> Below patch introduce a macro to systematically handle the 'X condition',
> what do you think?

I agree with Drew that better to use existing functions, and not to
duplicate them.  Non sunt multiplicanda entia sine necessitate.

Moreover, we should not change the old semantic of Dired commands:
if users have a habit of operating on the first files by going to the top
line and typing e.g. ‘M-< C-5 M’ to change modes of the first 5 files,
this is just fine, we should not prohobit this behaviour now.

So what we need to do is just check if the list of files returned from
‘dired-get-marked-files’ is nil, and show some message in this case in
all places that you found where the prompt with [0 files] makes no sense.

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

[Drew Adams]

> Since `dired-mark' from the top line followed by the command in question
> is not obvious for users, we could provide a hint in the error message,
> i.e. mention the availability of ‘m’ on the top line with such message:
> “You can type `m' here to mark all files for this operation”.

Not sure how helpful or necessary that is.  It's liable to
not be helpful (that use case needs no special advertising).
And it might even confuse things.  I think it just gets in
the way of the message, which is, "You are not on a file line."

I think we should show the same error message from any non-file
line - not just the top line or top-two lines.  And I think it
should just say that point is not on a file line.

Users who are interested or who wonder about the error message
can consult the doc string of the command.

That's the place, if any is needed, where such info as you
mention should be conveyed.  Such info is not part of the
message we're trying to get across here, which is just that
this command cannot be used if point is not on a file line.

Dired lets you do all kinds of things.  There are lots of ways
top mark files and lots of ways to act on marked files.  This
command should not be advertising any such ways in its error
message.

> I agree with Drew that better to use existing functions, and not to
> duplicate them.  Non sunt multiplicanda entia sine necessitate.

;-)

> Moreover, we should not change the old semantic of Dired commands:
> if users have a habit of operating on the first files by going to the
> top
> line and typing e.g. ‘M-< C-5 M’ to change modes of the first 5 files,
> this is just fine, we should not prohobit this behaviour now.
> 
> So what we need to do is just check if the list of files returned from
> ‘dired-get-marked-files’ is nil, and show some message in this case in
> all places that you found where the prompt with [0 files] makes no
> sense.

Yes.  Just a simple fix for this minor usability bug, please.

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

[Juri Linkov]

>> Since `dired-mark' from the top line followed by the command in question
>> is not obvious for users, we could provide a hint in the error message,
>> i.e. mention the availability of ‘m’ on the top line with such message:
>> “You can type `m' here to mark all files for this operation”.
>
> Not sure how helpful or necessary that is.  It's liable to
> not be helpful (that use case needs no special advertising).
> And it might even confuse things.  I think it just gets in
> the way of the message, which is, "You are not on a file line."

This message is absolutely wrong, it doesn't describe the state that causes
the error message.  It has a bigger scope than just file lines, it works
with marked files, etc.  So more correct message would be like this:

  “No files selected.”

Oh, and I discovered that the current state is much worse than I thought:

1. load dired-x
2. type ‘M-<’ to go to the first line
3. type ‘!’ (dired-do-shell-command)

  “Wrong type argument: stringp, nil”

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

[Drew Adams]

> > Not sure how helpful or necessary that is.  It's liable to
> > not be helpful (that use case needs no special advertising).
> > And it might even confuse things.  I think it just gets in
> > the way of the message, which is, "You are not on a file line."
> 
> This message is absolutely wrong, it doesn't describe the state that
> causes the error message.  It has a bigger scope than just file lines,
> it works with marked files, etc.

You're right about that.

> So more correct message would be like this:
> 
>   “No files selected.”

That's better, but it risks confusion over the notion of
"selection", especially in the context of marking or using
marks (a "selection" of files).

This might be better: "No files specified"
Or this: "No files chosen"

> Oh, and I discovered that the current state is much worse than I
> thought:
> 
> 1. load dired-x
> 2. type ‘M-<’ to go to the first line
> 3. type ‘!’ (dired-do-shell-command)
> 
>   “Wrong type argument: stringp, nil”

Meme combat - same thing.

The return value of `dired-get-marked-files' needs to be tested as
soon as it's available.  When it is nil we must not call
`dired-read-shell-command' or do anything else - just raise a
`user-error'.

This is a general change that needs to be looked for and made
wherever appropriate.

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

[Juri Linkov]

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

> This might be better: "No files specified"
> Or this: "No files chosen"
> [...]
> The return value of `dired-get-marked-files' needs to be tested as
> soon as it's available.  When it is nil we must not call
> `dired-read-shell-command' or do anything else - just raise a
> `user-error'.
>
> This is a general change that needs to be looked for and made
> wherever appropriate.

Agreed.  Simplicity is the hallmark of Emacs.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dired-get-marked-files-user-error.patch --]
[-- Type: text/x-diff, Size: 6386 bytes --]

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 55b68a3..f5caa2a 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -301,7 +301,8 @@ 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 (or (dired-get-marked-files t arg)
+                    (user-error "No files specified")))
 	 ;; The source of default file attributes is the file at point.
 	 (default-file (dired-get-filename t t))
 	 (default (when default-file
@@ -361,7 +362,8 @@ 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 (or (dired-get-marked-files t arg)
+                    (user-error "No files specified")))
 	 ;; The source of default file attributes is the file at point.
 	 (default-file (dired-get-filename t t))
 	 (modestr (when default-file
@@ -476,7 +478,8 @@ dired-do-print
 `lpr-switches' as default."
   (interactive "P")
   (require 'lpr)
-  (let* ((file-list (dired-get-marked-files t arg))
+  (let* ((file-list (or (dired-get-marked-files t arg)
+                        (user-error "No files specified")))
 	 (lpr-switches
 	  (if (and (stringp printer-name)
 		   (string< "" printer-name))
@@ -666,7 +669,8 @@ 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 (or (dired-get-marked-files t current-prefix-arg)
+                    (user-error "No files specified"))))
      (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 +731,8 @@ 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 (or (dired-get-marked-files t current-prefix-arg)
+                    (user-error "No files specified"))))
      (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 +1035,8 @@ 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 (or (dired-get-marked-files)
+                       (user-error "No files specified")))
          (out-file (expand-file-name (read-file-name "Compress to: ")))
          (rule (cl-find-if
                 (lambda (x)
@@ -1153,7 +1159,8 @@ 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 (or (dired-get-marked-files t arg nil t)
+                       (user-error "No files specified")))
 	    (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 +1852,8 @@ 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 (or (dired-get-marked-files nil arg)
+                      (user-error "No files specified")))
 	 (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 +2807,16 @@ 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)))
+   (or (dired-get-marked-files nil nil 'dired-nondirectory-p)
+       (user-error "No files specified"))))
 
 ;;;###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)))
+   (or (dired-get-marked-files nil nil 'dired-nondirectory-p)
+       (user-error "No files specified"))))
 
 ;;;###autoload
 (defun dired-do-search (regexp)
@@ -2827,7 +2837,8 @@ 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 (or (dired-get-marked-files nil nil 'dired-nondirectory-p)
+                    (user-error "No files specified")))
     (let ((buffer (get-file-buffer file)))
       (if (and buffer (with-current-buffer buffer
 			buffer-read-only))
@@ -2851,7 +2862,8 @@ dired-do-find-regexp
   (require 'grep)
   (defvar grep-find-ignored-files)
   (defvar grep-find-ignored-directories)
-  (let* ((files (dired-get-marked-files))
+  (let* ((files (or (dired-get-marked-files)
+                    (user-error "No files specified")))
          (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..1beeafe 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -1335,7 +1335,9 @@ 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 (or (dired-get-marked-files)
+                                    (user-error "No files specified"))
+                                noselect))
 
 (defun dired-simultaneous-find-file (file-list noselect)
   "Visit all files in FILE-LIST and display them simultaneously.

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

[martin rudalics]

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

martin

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

[Juri Linkov]

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dired-get-marked-files-user-error.2.patch --]
[-- Type: text/x-diff, Size: 7404 bytes --]

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))
+
 \f
 ;; 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.

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

[Tino Calancha]

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



On Thu, 1 Feb 2018, Juri Linkov wrote:

> Oh, and I discovered that the current state is much worse than I thought:
>
> 1. load dired-x
> 2. type ‘M-<’ to go to the first line
> 3. type ‘!’ (dired-do-shell-command)
>
>  “Wrong type argument: stringp, nil”
Is it too much ask you to read my commit messages in case you dont
have time/motivation to test my patches?  Then, you dont need to
rediscover things already reported by others.  Thanks.

Some people say here they dont have time to tes my patches (even
to read my commit messages), but at the same time they write
the Bible in their emails (by the way, not refering to you now).

I do sleep 3 hours every day, more busy that all of you together
and I still tested your patch and read (entirely) others emails.

PD: By the way, I have tested your patch, and it looks good to me.  It 
fixes the issue in a quite simple way.

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

[Tino Calancha]

On Thu, 1 Feb 2018, Drew Adams wrote:

>> I would like all `dired-do...' commands behave the same under the
>> 'X condition': * called from the top line ** no marked files.
>
> I've already said that it's not only about the top line.
> It's about the ordinary Dired situation of not being on a
> file line.  Plenty of Dired code already deals (simply)
> with this "X condition".
Sorry for the confussion:  I thought it was prety obvious
that 'X condition' was akind of summary of what the patch was doing.

It would be as easy as to read my commit message to realize that; or take 
a quick look in the diff I provided (dont need even to test it).  Then,
you would be talking about my work, not about what you guess it
is my work.  For the future, please try to at least read my commit
messages before make lot of observations about one patch that you didn't 
even read at all.  Thanks! :-)

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

[Drew Adams]

> >> I would like all `dired-do...' commands behave the same under the
> >> 'X condition': * called from the top line ** no marked files.
> >
> > I've already said that it's not only about the top line.
> > It's about the ordinary Dired situation of not being on a
> > file line.  Plenty of Dired code already deals (simply)
> > with this "X condition".
>
> Sorry for the confussion:  I thought it was prety obvious
> that 'X condition' was akind of summary of what the patch
> was doing.

I don't think there was any confusion there.  It was clear
what you meant by "X condition".  You said it meant:

  "* called from the top line [and] ** no marked files"

My point was that Dired code already deals, in various
places, with the condition of not being on a file line
and no files being marked.  That condition is easy to
deal with.

And I think that's the only problem that this bug
report needs to fix - in the case of the commands,
like `dired-do-chmod', that don't yet deal with it.

> It would be as easy as to read my commit message to realize that; or
> take
> a quick look in the diff I provided (dont need even to test it).  Then,
> you would be talking about my work, not about what you guess it
> is my work.  For the future, please try to at least read my commit
> messages before make lot of observations about one patch that you didn't
> even read at all.  Thanks! :-)

I guess you are angry or frustrated.  Sorry if I caused that.

I had already said, a day or two earlier in reply to your
request that I provide an alternative patch, that I'm OK
with whatever you decide.  I offered suggestions about this
bug, and I made clear that it's up to you and I wouldn't be
getting into the implementation details:

   > May I ask you to provide an alternative  patch to compare
   > with mine? Then, people here might do further feedback
   > based on those 2 alternatives.

   Sorry; I don't have time to work on this.  I've already
   provided my suggestions - hope they help. Whatever you
   decide is fine by me.  Thx.

I consider this bug to be trivial, and I hope for a simple
fix that doesn't complicate Dired generally.  But whatever
fix you provide is OK by me.

Sorry if my suggestions made you feel like I was discounting
your efforts.  I too have already spent more time on this
bug than I can afford to.  Please fix this bug as you see
best.  And thanks for your work on Dired and other Emacs
features.

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

[Juri Linkov]

>> Oh, and I discovered that the current state is much worse than I thought:
>>
>> 1. load dired-x
>> 2. type ‘M-<’ to go to the first line
>> 3. type ‘!’ (dired-do-shell-command)
>>
>>  “Wrong type argument: stringp, nil”
> Is it too much ask you to read my commit messages in case you dont
> have time/motivation to test my patches?  Then, you dont need to
> rediscover things already reported by others.  Thanks.

Where did you get the idea that I don't read your patches? This is
not true.  Now I re-read again your previous emails, and see nowhere
a mention of the error “Wrong type argument: stringp, nil”.
This is what I pointed out that unlike a useless message “[0 files]”
discussed before, the current code is actually worse because it contains
a plain bug.

> Some people say here they dont have time to tes my patches (even
> to read my commit messages), but at the same time they write
> the Bible in their emails (by the way, not refering to you now).

I'm always trying to write my emails as concise as possible to save time
of people who will read it.

> I do sleep 3 hours every day, more busy that all of you together
> and I still tested your patch and read (entirely) others emails.

I really appreciate all fresh ideas that you bring into development, and
your tireless efforts to implement them.  Thanks for all your contributions!

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

[Drew Adams]

> > Is it too much ask you to read my commit messages in case you dont
> > have time/motivation to test my patches?  Then, you dont need to
> > rediscover things already reported by others.  Thanks.
> 
> Where did you get the idea that I don't read your patches? This is
> not true.  

I'm guessing that Tino replied to you after getting angry
by my message saying that I didn't have time to read his
patch etc.  I imagine his frustration was not really directed
at you.  If so, that's understandable, even if you might not
have deserved it.  Just a guess.

> I really appreciate all fresh ideas that you bring into development, and
> your tireless efforts to implement them.  Thanks for all your
> contributions!

Ditto - for both of you, in fact.

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

[Juri Linkov]

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

This is now pushed to master and closed.