Subject: Re: bug#35564: [PATCH v4] Tweak dired warning about "wildcard" characters

From: Kévin Le Gouguec <[email-masked]>
Date: Mon, 29 Jul 2019 21:01:58 +0200

Michael Heerdegen <[email-masked]> writes:

> I dunno if the double emphasizing in the y-n-prompt (coloring +
> additional underlining with "^") is a bit too much.

The patch series started out with just the coloring, which we figured
might have accessibility issues on its own (we can't assume that the
user can distinguish colors); we added the '^' markers to alleviate
this; then…

> A bit related:
> Maybe the second line could be combined with the first line so that one
> line is saved.

(Assuming the first line you mention is "Confirm" and the second line is
the command, which would make the '^' markers the third line; apologies
if I misunderstood)

… I realized that when the user fails to answer 'y' or 'n', y-or-n-p
prepends "Please answer y or n." to the prompt, i.e. this…

    sed 's/?/!/'
           ^

… becomes this:

    Please answer y or n.  sed 's/?/!/'
           ^

AFAICT, this means that we need a newline *before* the command (unless
we add an optional RETRY-PROMPT argument to y-or-n-p or something).

(I added comments to try to explain this in dired--no-subst-prompt; tell
me if they need more work.)

> I mean, the prompt is four lines high with this change,
> quite a lot.  Dunno what others think about it.

It is fairly more heavyweight than before.

And the irony is, I am still not 100% satisfied with it; I worry that
the user will take "Send 1 occurrence of `*' as-is to shell?" to mean
"Escape 1 occurrence of `*' so that the shell leaves it as-is?".

Tell me if the shed is about to crumble under the weight of paint, but
if we are fine with so many lines, could we perhaps rephrase…

> Confirm
> sed 's/?/!/'
>        ^
> Send 1 occurrence of `?' as-is to shell?

… to:

> Warning: the shell may interpret 1 occurrence of `?' as wildcard:
> sed 's/?/!/'
>        ^
> Proceed anyway?

>From 97d5ec40ccbb3a2d366aaa05b45e29690ee5288a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <[email-masked]>
Date: Mon, 29 Jul 2019 20:48:53 +0200
Subject: [PATCH] Tweak dired-do-shell-command warning some more

* lisp/dired-aux.el (dired--no-subst-prompt): Clarify what we are
concerned about.

(bug#28969, bug#35564)
---
 lisp/dired-aux.el | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 3887d75356..35052785ba 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -120,17 +120,18 @@ dired--no-subst-prompt
   ;; text will shove the command to the right, and the '^' markers
   ;; will become misaligned.
   (apply #'concat
-         `("Confirm:\n"
+         `(,(format-message
+             (ngettext
+              "Warning: the shell may interpret %d occurrence of `%s' as wildcard:\n"
+              "Warning: the shell may interpret %d occurrences of `%s' as wildcards:\n"
+              (length char-positions))
+             (length char-positions)
+             (propertize (string (aref command (car char-positions)))
+                         'face 'warning))
            ,command "\n"
            ,@(when add-markers
                (list (dired--mark-positions char-positions) "\n"))
-           ,(format-message
-             (ngettext "Send %d occurrence of `%s' as-is to shell?"
-                       "Send %d occurrences of `%s' as-is to shell?"
-                       (length char-positions))
-             (length char-positions)
-             (propertize (string (aref command (car char-positions)))
-                         'face 'warning)))))
+           "Proceed anyway?")))
 
 ;;;###autoload
 (defun dired-diff (file &optional switches)
-- 
2.22.0

(Can be applied on top of the patch series or the squashed patch; if the
latter, the commit message can be discarded.  Tests unchanged, since
they only look at the command and the markers.)

If everybody likes the prompt well enough without this umpteenth tweak,
it's fine by me.

Thank you all for your patience.