GNU bug report logs - #29465
25.3; Confusing message for dired-do-shell-command substitution

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29465 in the body.
You can then email your comments to 29465 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.3; Confusing message for dired-do-shell-command substitution
Date: Sun, 26 Nov 2017 23:16:59 -0800

When you use * or ? in dired-do-shell-command without the intent for
Dired to replace it,  you are prompted with

  Confirm--do you mean to use `*' as a wildcard?

This message is confusing, because there are lots of ways for * to be
passed to the shell without globbing.  I am also more familiar with
the term globbing than wildcard, which makes it doubly confusing, for
example if I run

  find ? -name '*.txt'

I get the message:

  Confirm--do you mean to use `*' as a wildcard?

What the question is really asking is, should * be passed to the shell
directly, as whether or not it is interpreted as a glob is determined
by the shell and the quoting rules in question.

I think this confirmation message should be removed entirely.

1. The edge case that it is trying to protect against is not very common.
2. There is no reasonable behavior that the user could expect from
this edge case.
3. The documentation string clearly describes how * and ? are interpreted.
4. The confirmation message is not very informative and is possible misleading.
5. This confirmation message shows up every time an advanced user
wants to run any command containing * or ?, e.g. for find, grep, sed,
or many other tools.
6. The confirmation message is not even shown consistently.  For
example it is shown for

  find ? -name '*.txt'

but not for

  find * -name '*.txt'

Thus, it isn't even useful for protecting against some hypothetical
unwanted behavior.

In GNU Emacs 25.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.19)
 of 2017-09-16 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.11905000
Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong
 -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Message #8 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: 29465 <at> debbugs.gnu.org
Subject: RE: 25.3; Confusing message for dired-do-shell-command substitution
Date: Sun, 26 Nov 2017 23:34:33 -0800

I have included two patches.  The first is to fix the documentation
string which I encountered while reading the code, the second is for
my proposed solution of removing the confirmation message.

Subject: [PATCH 1/2] Clarify dired-do-shell-command docstring

The docstring seemed to imply that if * and ? were used together, *
would take priority.  However, it is explicitly checked that * and ?
are not used together.

* lisp/dired-aux.el (dired-do-shell-command): Fix docstring
---
 lisp/dired-aux.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index f1f7cf0b0e..57eb216231 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -686,13 +686,15 @@ dired-do-shell-command
 If there is a `*' in COMMAND, surrounded by whitespace, this runs
 COMMAND just once with the entire file list substituted there.

-If there is no `*', but there is a `?' in COMMAND, surrounded by
-whitespace, or a `\\=`?\\=`' this runs COMMAND on each file
-individually with the file name substituted for `?' or `\\=`?\\=`'.
+If there is a `?' in COMMAND, surrounded by whitespace, or a
+`\\=`?\\=`' this runs COMMAND on each file individually with the
+file name substituted for `?' or `\\=`?\\=`'.

 Otherwise, this runs COMMAND on each file individually with the
 file name added at the end of COMMAND (separated by a space).

+`*' and `?' cannot be used together.
+
 `*' and `?' when not surrounded by whitespace nor `\\=`' have no special
 significance for `dired-do-shell-command', and are passed through
 normally to the shell, but you must confirm first.
-- 
2.15.0

Subject: [PATCH 2/2] Remove confirmation when using * or ? for the shell

These confirmation messages are misleading, do not trigger for all
cases, and obstruct many commands that use * or ?, like find, sed,
grep, etc.

* lisp/dired-aux.el (dired-do-shell-command): Remove substitution mark
  confirmation
---
 lisp/dired-aux.el | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 57eb216231..15bb3173b7 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -744,20 +744,10 @@ dired-do-shell-command
                            (dired--star-or-qmark-p res str))
                  (setq res (replace-match "" t t res 2)))
                (string-match regexp res))))
-  (let* ((on-each (not (dired--star-or-qmark-p command "*" 'keep)))
- (no-subst (not (dired--star-or-qmark-p command "?" 'keep)))
-         ;; Get confirmation for wildcards that may have been meant
-         ;; to control substitution of a file name or the file name list.
-         (ok (cond ((not (or on-each no-subst))
-             (error "You can not combine `*' and `?' substitution marks"))
-            ((need-confirm-p command "*")
-             (y-or-n-p (format-message
-        "Confirm--do you mean to use `*' as a wildcard? ")))
-            ((need-confirm-p command "?")
-             (y-or-n-p (format-message
-        "Confirm--do you mean to use `?' as a wildcard? ")))
-            (t))))
-    (when ok
+    (let* ((on-each (not (dired--star-or-qmark-p command "*" 'keep)))
+           (no-subst (not (dired--star-or-qmark-p command "?" 'keep))))
+      (unless (or on-each no-subst)
+        (error "You can not combine `*' and `?' substitution marks"))
       (if on-each
    (dired-bunch-files (- 10000 (length command))
                       (lambda (&rest files)
@@ -766,7 +756,7 @@ dired-do-shell-command
                       nil file-list)
  ;; execute the shell command
  (dired-run-shell-command
- (dired-shell-stuff-it command file-list nil arg)))))))
+ (dired-shell-stuff-it command file-list nil arg))))))

 ;; Might use {,} for bash or csh:
 (defvar dired-mark-prefix ""
-- 
2.15.0

Message #11 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Allen Li <vianchielfaura <at> gmail.com>
Cc: 29465 <at> debbugs.gnu.org
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Mon, 27 Nov 2017 10:07:20 +0100

Allen Li <vianchielfaura <at> gmail.com> writes:

> I have included two patches.  The first is to fix the documentation
> string which I encountered while reading the code, the second is for
> my proposed solution of removing the confirmation message.

I think this would also solve Bug#28969 that I had reported some time
ago.  I wanted to try your patch but...

> diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
> index 57eb216231..15bb3173b7 100644
> --- a/lisp/dired-aux.el
> +++ b/lisp/dired-aux.el
> @@ -744,20 +744,10 @@ dired-do-shell-command
>                             (dired--star-or-qmark-p res str))
>                   (setq res (replace-match "" t t res 2)))
>                 (string-match regexp res))))
> -  (let* ((on-each (not (dired--star-or-qmark-p command "*" 'keep)))
> - (no-subst (not (dired--star-or-qmark-p command "?" 'keep)))

Here seems to be a problem with leading whitespace in the first line -
applying this patch fails for me.


Michael.

Message #14 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Allen Li <vianchielfaura <at> gmail.com>,
 Tino Calancha <tino.calancha <at> gmail.com>
Cc: 29465 <at> debbugs.gnu.org
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Mon, 27 Nov 2017 17:58:04 +0200

> From: Allen Li <vianchielfaura <at> gmail.com>
> Date: Sun, 26 Nov 2017 23:16:59 -0800
> 
> When you use * or ? in dired-do-shell-command without the intent for
> Dired to replace it,  you are prompted with
> 
>   Confirm--do you mean to use `*' as a wildcard?
> 
> This message is confusing, because there are lots of ways for * to be
> passed to the shell without globbing.  I am also more familiar with
> the term globbing than wildcard, which makes it doubly confusing, for
> example if I run
> 
>   find ? -name '*.txt'
> 
> I get the message:
> 
>   Confirm--do you mean to use `*' as a wildcard?
> 
> What the question is really asking is, should * be passed to the shell
> directly, as whether or not it is interpreted as a glob is determined
> by the shell and the quoting rules in question.

Actually, AFAICT the command wants to ask this:

  Are you sure you want `*' to be passed to the shell?

(as opposed to letting dired-do-shell-command interpret `*' as
described in the doc string).

> I think this confirmation message should be removed entirely.
> 
> 1. The edge case that it is trying to protect against is not very common.
> 2. There is no reasonable behavior that the user could expect from
> this edge case.
> 3. The documentation string clearly describes how * and ? are interpreted.
> 4. The confirmation message is not very informative and is possible misleading.
> 5. This confirmation message shows up every time an advanced user
> wants to run any command containing * or ?, e.g. for find, grep, sed,
> or many other tools.
> 6. The confirmation message is not even shown consistently.  For
> example it is shown for
> 
>   find ? -name '*.txt'
> 
> but not for
> 
>   find * -name '*.txt'
> 
> Thus, it isn't even useful for protecting against some hypothetical
> unwanted behavior.

Tino added this confirmation last July, so I will let him defend his
change.

If we want to remove this confirmation, now is the time, because it
wasn't yet released with any Emacs version.  Once this confirmation is
out at large, it will be much harder to remove it, as that would be an
incompatible change.

Message #17 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Allen Li <vianchielfaura <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29465 <at> debbugs.gnu.org, rms <at> gnu.org
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Tue, 28 Nov 2017 12:50:52 +0900

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Allen Li <vianchielfaura <at> gmail.com>
>> 
>>   find ? -name '*.txt'
>> 
>> but not for
>> 
>>   find * -name '*.txt'
>> 
>> Thus, it isn't even useful for protecting against some hypothetical
>> unwanted behavior.
>
> Tino added this confirmation last July, so I will let him defend his
> change.
>
> If we want to remove this confirmation, now is the time, because it
> wasn't yet released with any Emacs version.  Once this confirmation is
> out at large, it will be much harder to remove it, as that would be an
> incompatible change.
Thanks for your report Allen!

FWIW the confirmation was added by RMS in 2002
(eab9ed67eb50bab4fc736058a799508d544606a0).
See also commits:
commit e52c37fad057b29d68c51cf3a70b2e0d94f656cb
commit edb8d73e62552cf2f95cbf871050913862dc5f18

My commit "Ask confirmation for all suspicious wildcards"
(6e39940adba7b96dfe520caa52a1b92a1a84a84f)
extends that confirmation to cover Bug#27496.

Before 6e39940adb, not all `?' or `*' were checked for
being between white spaces:
! echo ./? RET ; This ask confirmation
! echo ./? ? RET ; This doesn't

After 6e39940adb, all occurrences of `?' or `*' are checked for
being between white spaces; the user is asked confirmation if
any of them are not surrounded by whites.
I made this change for consistency: I thought it has sense to
check all `?' `*' occurrences, not just one.

In fact, this change causes that you are prompted in your snippet:
! find * -name '*.txt' RET 
;; After (before) 6e39940adb you are (not) prompted.

Compare with the following:
! find ? -name '*.txt' RET 
;; After and before 6e39940adb you are prompted.

Your second patch disables the confirmation prompts that have
being around since 2002.  Since the source of this bug report
seems to be 6e39940adb, I would rather revert just this commit.

Message #20 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29465 <at> debbugs.gnu.org, rms <at> gnu.org
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Tue, 28 Nov 2017 00:25:17 -0800

On Mon, Nov 27, 2017 at 7:50 PM, Tino Calancha <tino.calancha <at> gmail.com> wrote:
> FWIW the confirmation was added by RMS in 2002
> (eab9ed67eb50bab4fc736058a799508d544606a0).
> See also commits:
> commit e52c37fad057b29d68c51cf3a70b2e0d94f656cb
> commit edb8d73e62552cf2f95cbf871050913862dc5f18
>
> My commit "Ask confirmation for all suspicious wildcards"
> (6e39940adba7b96dfe520caa52a1b92a1a84a84f)
> extends that confirmation to cover Bug#27496.
>
> Before 6e39940adb, not all `?' or `*' were checked for
> being between white spaces:
> ! echo ./? RET ; This ask confirmation
> ! echo ./? ? RET ; This doesn't
>
> After 6e39940adb, all occurrences of `?' or `*' are checked for
> being between white spaces; the user is asked confirmation if
> any of them are not surrounded by whites.
> I made this change for consistency: I thought it has sense to
> check all `?' `*' occurrences, not just one.
>
> In fact, this change causes that you are prompted in your snippet:
> ! find * -name '*.txt' RET
> ;; After (before) 6e39940adb you are (not) prompted.
>
> Compare with the following:
> ! find ? -name '*.txt' RET
> ;; After and before 6e39940adb you are prompted.
>
> Your second patch disables the confirmation prompts that have
> being around since 2002.  Since the source of this bug report
> seems to be 6e39940adb, I would rather revert just this commit.

I see.  I actually do not have your commit yet (on 25.3.1), so I am
encountering the inconsistent behavior that you fixed.

However, your commit doesn't solve the problem that the message is
misleading; unfortunately by making the prompt appear consistently the
message is now even more confusing.

Before (right now on 25.3.1), this did not show a prompt:

  find * -name '*.txt'

Now it does show a prompt:

  Confirm--do you mean to use `*' as a wildcard?

I would have no idea what this means.  Does it mean that Dired is
going to replace the *?  Does it mean that Dired is going to replace
the ’*.txt’?  Or perhaps both?  What am I confirming exactly?

The second issue is that the prompt is very annoying for advanced
users; I have filed a second bug for this (#29190).

Since your commit fixes the inconsistency problem, that's one less
reason for my advocating to remove the confirmation.  If we can make
the message less confusing and add an option to disable the prompt, I
would be happy.

However, I think writing a useful confirmation prompt for this is
hard; hopefully someone has a good idea.

One idea would be what Eli suggested:

  Are you sure you want `*' to be passed to the shell?

However, what if the command contains both `*' and `?'?

  find * -name ’*.txt’ -o -name ’x??’
  Are you sure you want `*' to be passed to the shell?
  (Does that include the `?' or not?)

What if the user typos the intended substitution character?

  find *x -name ’*.txt’ -o -name ’x??’
  Are you sure you want `*' to be passed to the shell?
  (Which one?)

Since I was not confident that a good message could be written, I
suggested removing the confirmation.

Also, I am not sure what this is supposed to be protecting against.
It seems more useful to confirm when dired-do-shell-command is going
to replace * or ? rather than when it is not.  If the user did not
read the documentation string, the user would most likely expect these
characters to be passed to the shell.  If the user did read the
documentation string, the prompt would only be an annoyance.

The original commit by RMS (eab9ed67eb50bab4fc736058a799508d544606a0)
does not provide a reason for the confirmation.  If I were to hazard a
guess, the behavior of the command was changed, so the prompt was
added to warn users accustomed to the old behavior.  However, it is
now 15 years since; I don’t think there’s any value keeping the
confirmation around for its original (?) purpose.

Message #23 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: vianchielfaura <at> gmail.com, 29465 <at> debbugs.gnu.org
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Tue, 28 Nov 2017 18:15:15 +0200

> From: Tino Calancha <tino.calancha <at> gmail.com>
> Cc: 29465 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, rms <at> gnu.org
> Date: Tue, 28 Nov 2017 12:50:52 +0900
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Allen Li <vianchielfaura <at> gmail.com>
> >> 
> >>   find ? -name '*.txt'
> >> 
> >> but not for
> >> 
> >>   find * -name '*.txt'
> >> 
> >> Thus, it isn't even useful for protecting against some hypothetical
> >> unwanted behavior.
> >
> > Tino added this confirmation last July, so I will let him defend his
> > change.
> >
> > If we want to remove this confirmation, now is the time, because it
> > wasn't yet released with any Emacs version.  Once this confirmation is
> > out at large, it will be much harder to remove it, as that would be an
> > incompatible change.
> Thanks for your report Allen!
> 
> FWIW the confirmation was added by RMS in 2002
> (eab9ed67eb50bab4fc736058a799508d544606a0).
> See also commits:
> commit e52c37fad057b29d68c51cf3a70b2e0d94f656cb
> commit edb8d73e62552cf2f95cbf871050913862dc5f18
> 
> My commit "Ask confirmation for all suspicious wildcards"
> (6e39940adba7b96dfe520caa52a1b92a1a84a84f)
> extends that confirmation to cover Bug#27496.

Oops, I apologize for my faulty forensic job.

Message #26 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Allen Li <vianchielfaura <at> gmail.com>
Cc: 29465 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Tue, 28 Nov 2017 18:26:52 +0200

> From: Allen Li <vianchielfaura <at> gmail.com>
> Date: Tue, 28 Nov 2017 00:25:17 -0800
> Cc: 29465 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, rms <at> gnu.org
> 
> Since your commit fixes the inconsistency problem, that's one less
> reason for my advocating to remove the confirmation.  If we can make
> the message less confusing and add an option to disable the prompt, I
> would be happy.

I think clarifying the prompt and adding an option is indeed the way
forward.

> However, I think writing a useful confirmation prompt for this is
> hard; hopefully someone has a good idea.
> 
> One idea would be what Eli suggested:
> 
>   Are you sure you want `*' to be passed to the shell?
> 
> However, what if the command contains both `*' and `?'?

Then the prompt should say that:

   Are you sure you want `*' and `?' to be passed to the shell?

> Since I was not confident that a good message could be written, I
> suggested removing the confirmation.

I think we should try to make the prompt more clear, it cannot be that
hard.  Removing the prompt will introduce backward incompatibility
with what Emacs was doing for the past 15 years, so it's a worse
alternative, IMO.

> Also, I am not sure what this is supposed to be protecting against.
> It seems more useful to confirm when dired-do-shell-command is going
> to replace * or ? rather than when it is not.  If the user did not
> read the documentation string, the user would most likely expect these
> characters to be passed to the shell.  If the user did read the
> documentation string, the prompt would only be an annoyance.
> 
> The original commit by RMS (eab9ed67eb50bab4fc736058a799508d544606a0)
> does not provide a reason for the confirmation.

You need to look up relevant discussions on Emacs mailing lists around
the date of the commit.  In this case, read this thread:

  http://lists.gnu.org/archive/html/emacs-devel/2002-01/msg00233.html

and also the original bug report and its followups:

  http://lists.gnu.org/archive/html/bug-gnu-emacs/2002-01/msg00230.html

> If I were to hazard a guess, the behavior of the command was
> changed, so the prompt was added to warn users accustomed to the old
> behavior.

No, it was a bug report about a potentially risky feature, where a
user mistyping a command could have their files wiped out or cause
some other grave accident.

> However, it is now 15 years since; I don’t think there’s any value
> keeping the confirmation around for its original (?) purpose.

The syntax of the shell commands supported by dired-do-shell-command
and its features regarding '*' and '?' are still very complicated, as
they were back then.  Just the doc string describing the behavior is
so long it can scare.  So I don't see how the time that has passed is
of relevance here.

Message #29 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29465 <at> debbugs.gnu.org, Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Tue, 28 Nov 2017 12:13:11 -0800

On Tue, Nov 28, 2017 at 8:26 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> I think we should try to make the prompt more clear, it cannot be that
> hard.  Removing the prompt will introduce backward incompatibility
> with what Emacs was doing for the past 15 years, so it's a worse
> alternative, IMO.

I don't care personally, if there's an option to disable it, but I am
not convinced for the sake of the betterment of Emacs.  I don't see
how removing the prompt introduces backward incompatibility issues
unless someone has a keyboard macro saved that includes a "y" to
handle this case (which I suppose is a possible use case, but I think
we have to draw the line somewhere; would improving Emacs performance
be a breaking change because someone was relying on a certain function
call taking at least a certain amount of time?).  This is changing
interactive usage, not API usage.

> You need to look up relevant discussions on Emacs mailing lists around
> the date of the commit.  In this case, read this thread:
>
>   http://lists.gnu.org/archive/html/emacs-devel/2002-01/msg00233.html
>
> and also the original bug report and its followups:
>
>   http://lists.gnu.org/archive/html/bug-gnu-emacs/2002-01/msg00230.html
>
>> If I were to hazard a guess, the behavior of the command was
>> changed, so the prompt was added to warn users accustomed to the old
>> behavior.
>
> No, it was a bug report about a potentially risky feature, where a
> user mistyping a command could have their files wiped out or cause
> some other grave accident.

Thanks for that.  But the confirmation prompt doesn't actually address
said bug.  Mistyping `M-! rm *' instead of `! rm *' will still wipe
out your files with no confirmation.  I don't accept that `! rm *""',
the case covered by the confirmation prompt, is a common typo.

Basically, I'm not convinced that this is protecting against a
common case of user error.  I would appreciate it if someone
could provide a concrete example of such a user error (or ideally
two or three examples).

> The syntax of the shell commands supported by dired-do-shell-command
> and its features regarding '*' and '?' are still very complicated, as
> they were back then.  Just the doc string describing the behavior is
> so long it can scare.  So I don't see how the time that has passed is
> of relevance here.

If dired-do-shell-command is complicated enough to scare, as you
say, I think presenting the user with an ambiguous confirmation
prompt scares even more.  The confirmation prompt implies that
the user is doing something dangerous, when in all likelihood the
user is doing nothing of the sort.  Again, I would like to see a
concrete example of a user error this prompt actually protects
against.

With all of that said, I am fine with adding an option, since I can
just set it and be on my merry way.  But I think Emacs would benefit
from not having this prompt (and I would be happy to be convinced
otherwise).

Message #32 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Allen Li <vianchielfaura <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 29465 <at> debbugs.gnu.org, Tino Calancha <tino.calancha <at> gmail.com>
Subject: RE: bug#29465: 25.3; Confusing message for dired-do-shell-command
 substitution
Date: Tue, 28 Nov 2017 20:20:38 -0800 (PST)

I have not been following this thread, so apologies if
this doesn't help or isn't very relevant.

IF we feel it helps a user to prompt about something,
and IF we feel there is a possibility that some users
might not understand the prompt, in spite of our best
efforts to come up with a good prompt, and IF we feel
that understanding the prompt is important, THEN the
doc string should make clear whatever it is that it
is important that users understand about that prompting.

It's quite possible for a user not to understand even
a good prompt.  S?he should be able to get the point
by doing `C-h f', in that case.

Message #35 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: vianchielfaura <at> gmail.com, 29465 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#29465: 25.3; Confusing message for dired-do-shell-command
 substitution
Date: Fri, 01 Dec 2017 10:36:18 +0200

> Date: Tue, 28 Nov 2017 20:20:38 -0800 (PST)
> From: Drew Adams <drew.adams <at> oracle.com>
> Cc: 29465 <at> debbugs.gnu.org, Tino Calancha <tino.calancha <at> gmail.com>
> 
> IF we feel it helps a user to prompt about something,
> and IF we feel there is a possibility that some users
> might not understand the prompt, in spite of our best
> efforts to come up with a good prompt, and IF we feel
> that understanding the prompt is important, THEN the
> doc string should make clear whatever it is that it
> is important that users understand about that prompting.
> 
> It's quite possible for a user not to understand even
> a good prompt.  S?he should be able to get the point
> by doing `C-h f', in that case.

The doc string already attempts to do that:

  `*' and `?' when not surrounded by whitespace nor `\\=`' have no special
  significance for `dired-do-shell-command', and are passed through
  normally to the shell, but you must confirm first.

We could make the intent of the confirmation even more clear, e.g.

  `*' and `?' when not surrounded by whitespace nor `\\=`' have no special
  significance for `dired-do-shell-command', and are passed through
  normally to the shell, but you must confirm first, to avoid
  inadvertently passing a wildcard to a shell command, which would cause
  that command to act on more files than you intended.

Is anything else needed to make this prompt's intent more clear?

Message #38 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Drew Adams <drew.adams <at> oracle.com>
Cc: vianchielfaura <at> gmail.com, 29465 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: RE: bug#29465: 25.3; Confusing message for dired-do-shell-command
 substitution
Date: Fri, 1 Dec 2017 07:42:44 -0800 (PST)

> > IF we feel it helps a user to prompt about something,
> > and IF we feel there is a possibility that some users
> > might not understand the prompt, in spite of our best
> > efforts to come up with a good prompt, and IF we feel
> > that understanding the prompt is important, THEN the
> > doc string should make clear whatever it is that it
> > is important that users understand about that prompting.
> >
> > It's quite possible for a user not to understand even
> > a good prompt.  S?he should be able to get the point
> > by doing `C-h f', in that case.
> 
> The doc string already attempts to do that:
> 
> `*' and `?' when not surrounded by whitespace nor `\\=`' have...

  `*' and `?', unless surrounded by whitespace or `\\=', have...

is easier to understand, I think.

> We could make the intent of the confirmation even more clear, e.g.
> 
>   `*' and `?' when not surrounded by whitespace nor `\\=`' have no
>   special
>   significance for `dired-do-shell-command', and are passed through
>   normally to the shell, but you must confirm first, to avoid
>   inadvertently passing a wildcard to a shell command, which would
>   cause that command to act on more files than you intended.

Please consider splitting that in two: "...to the shell.  But..."

> Is anything else needed to make this prompt's intent more clear?

That seems good enough for the doc string.  I don't have
a suggestion for the prompt itself.  (I don't think it's
super clear, though.)

Message #41 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 29465 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Fri, 1 Dec 2017 22:31:03 -0800

[Message part 1 (text/plain, inline)]

On Fri, Dec 1, 2017 at 12:36 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> Date: Tue, 28 Nov 2017 20:20:38 -0800 (PST)
>> From: Drew Adams <drew.adams <at> oracle.com>
>> Cc: 29465 <at> debbugs.gnu.org, Tino Calancha <tino.calancha <at> gmail.com>
>>
>> IF we feel it helps a user to prompt about something,
>> and IF we feel there is a possibility that some users
>> might not understand the prompt, in spite of our best
>> efforts to come up with a good prompt, and IF we feel
>> that understanding the prompt is important, THEN the
>> doc string should make clear whatever it is that it
>> is important that users understand about that prompting.
>>
>> It's quite possible for a user not to understand even
>> a good prompt.  S?he should be able to get the point
>> by doing `C-h f', in that case.
>
> The doc string already attempts to do that:
>
>   `*' and `?' when not surrounded by whitespace nor `\\=`' have no special
>   significance for `dired-do-shell-command', and are passed through
>   normally to the shell, but you must confirm first.
>
> We could make the intent of the confirmation even more clear, e.g.
>
>   `*' and `?' when not surrounded by whitespace nor `\\=`' have no special
>   significance for `dired-do-shell-command', and are passed through
>   normally to the shell, but you must confirm first, to avoid
>   inadvertently passing a wildcard to a shell command, which would cause
>   that command to act on more files than you intended.
>
> Is anything else needed to make this prompt's intent more clear?

I made some small changes to the docstring and I added an option for
disabling the prompt, in two separate patches against master.  I have
attached the patches.

Since I don't have a good idea for the prompt text itself, I fixed
these two issues first.

Aside: is there a recommended way of formatting and sending patches?
What's easiest for me is using git format-patch and then attaching the
files, but I don't know if Emacs maintainers prefer anything specific
(e.g. mail readers that don't support MIME attachments?)

[0001-Clarify-dired-do-shell-command-docstring.patch (text/x-patch, attachment)]

[0002-Add-option-for-controlling-dired-do-shell-command-pr.patch (text/x-patch, attachment)]

Message #44 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Allen Li <vianchielfaura <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29465 <at> debbugs.gnu.org,
 Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Sat, 02 Dec 2017 16:32:16 +0900

Allen Li <vianchielfaura <at> gmail.com> writes:

Thank you for the new patches Allen!
I don't have strong opinions on this thread; probably because
I am already in Christmass mode or something...  Anyway, I think
you guys are discussing pretty well the thing!

I have just two comments in the second patch:
> +;;;###autoload
> +(defcustom dired-confirm-shell-command t
> +  "Whether to prompt for confirmation for Dired shell commands.
> +If t, prompt"
> +  :type '(choice (const :tag "No restrictions" nil)
> +		 (const :tag "When point is on a file name initially, search file names" dwim)
> +		 (const :tag "Always search in file names" t))
> +  :group 'dired
> +  :version "26.0")
               ^^^^

* Version must be 26.1
* The :type looks unrelated with the option.
Maybe better something like this:

:type '(choice (const :tag "Ask confirmation" t)
		 (const :tag "Never ask confirmation" nil))     
> +
> -               (string-match regexp res))))
> +             (when
> +                 dired-confirm-shell-command
> +               (let ((res cmd)
You might put the option in the same line as `when', i.e.:
(when dired-confirm-shell-command

Message #47 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29465 <at> debbugs.gnu.org,
 Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#29465: 25.3;
 Confusing message for dired-do-shell-command substitution
Date: Sat, 2 Dec 2017 00:22:17 -0800

[Message part 1 (text/plain, inline)]

On Fri, Dec 1, 2017 at 11:32 PM, Tino Calancha <tino.calancha <at> gmail.com> wrote:
> Allen Li <vianchielfaura <at> gmail.com> writes:
>
> Thank you for the new patches Allen!
> I don't have strong opinions on this thread; probably because
> I am already in Christmass mode or something...  Anyway, I think
> you guys are discussing pretty well the thing!
>
> I have just two comments in the second patch:
>> +;;;###autoload
>> +(defcustom dired-confirm-shell-command t
>> +  "Whether to prompt for confirmation for Dired shell commands.
>> +If t, prompt"
>> +  :type '(choice (const :tag "No restrictions" nil)
>> +              (const :tag "When point is on a file name initially, search file names" dwim)
>> +              (const :tag "Always search in file names" t))
>> +  :group 'dired
>> +  :version "26.0")
>                ^^^^
>
> * Version must be 26.1
> * The :type looks unrelated with the option.
> Maybe better something like this:
>
> :type '(choice (const :tag "Ask confirmation" t)
>                  (const :tag "Never ask confirmation" nil))
>> +
>> -               (string-match regexp res))))
>> +             (when
>> +                 dired-confirm-shell-command
>> +               (let ((res cmd)
> You might put the option in the same line as `when', i.e.:
> (when dired-confirm-shell-command

Thanks, attached new second patch.

[0002-Add-option-for-controlling-dired-do-shell-command-pr.patch (text/x-patch, attachment)]

Message #50 received at 29465 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Allen Li <vianchielfaura <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 29465 <at> debbugs.gnu.org,
 Drew Adams <drew.adams <at> oracle.com>, Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#29465: 25.3; Confusing message for dired-do-shell-command
 substitution
Date: Tue, 22 Mar 2022 17:48:32 +0100

Allen Li <vianchielfaura <at> gmail.com> writes:

> Thanks, attached new second patch.

Thanks; pushed to Emacs 29 now (with some changes).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.