bug#29465: 25.3; Confusing message for dired-do-shell-command substitution

[Allen Li <vianchielfaura@gmail.com>]

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

On Fri, Dec 1, 2017 at 11:32 PM, Tino Calancha <tino.calancha@gmail.com> wrote:
> Allen Li <vianchielfaura@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.

[-- Attachment #2: 0002-Add-option-for-controlling-dired-do-shell-command-pr.patch --]
[-- Type: text/x-patch, Size: 3417 bytes --]

From 3a6861552d2a49e052b76b10d5b77b9ce2eed016 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Fri, 1 Dec 2017 22:22:53 -0800
Subject: [PATCH 2/2] Add option for controlling dired-do-shell-command prompt

* doc/emacs/dired.texi (Shell Commands in Dired): Document option
* lisp/dired-aux.el (dired-confirm-shell-command): Add option
(dired-do-shell-command): Check option before prompting
---
 doc/emacs/dired.texi |  4 +++-
 lisp/dired-aux.el    | 24 +++++++++++++++++-------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/doc/emacs/dired.texi b/doc/emacs/dired.texi
index 9348ef5042..4c0826e1a3 100644
--- a/doc/emacs/dired.texi
+++ b/doc/emacs/dired.texi
@@ -866,6 +866,7 @@ Shell Commands in Dired
 @findex dired-do-shell-command
 @kindex ! @r{(Dired)}
 @kindex X @r{(Dired)}
+@vindex dired-confirm-shell-command
 The Dired command @kbd{!} (@code{dired-do-shell-command}) reads a
 shell command string in the minibuffer, and runs that shell command on
 one or more files.  The files that the shell command operates on are
@@ -902,7 +903,8 @@ Shell Commands in Dired
 If you want to use @samp{*} as a shell wildcard with whitespace around
 it, write @samp{*""}.  In the shell, this is equivalent to @samp{*};
 but since the @samp{*} is not surrounded by whitespace, Dired does not
-treat it specially.
+treat it specially.  Emacs will prompt for confirmation if you do
+this, unless @code{dired-confirm-shell-command} is @code{nil}.
 
 @item
 Otherwise, if the command string contains @samp{?} surrounded by
diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 57eb216231..c9f240dd46 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -649,6 +649,15 @@ dired-read-shell-command
       (dired-mark-pop-up nil 'shell files
 			 'read-shell-command prompt nil nil))))
 
+;;;###autoload
+(defcustom dired-confirm-shell-command t
+  "Whether to prompt for confirmation for ‘dired-do-shell-command’.
+If t, prompt for confirmation if the command contains potentially
+dangerous characters.  If nil, never prompt for confirmation."
+  :type 'boolean
+  :group 'dired
+  :version "26.1")
+
 ;;;###autoload
 (defun dired-do-async-shell-command (command &optional arg file-list)
   "Run a shell command COMMAND on the marked files asynchronously.
@@ -737,13 +746,14 @@ dired-do-shell-command
       files)))
   (cl-flet ((need-confirm-p
              (cmd str)
-             (let ((res cmd)
-                   (regexp (regexp-quote str)))
-               ;; Drop all ? and * surrounded by spaces and `?`.
-               (while (and (string-match regexp res)
-                           (dired--star-or-qmark-p res str))
-                 (setq res (replace-match "" t t res 2)))
-               (string-match regexp res))))
+             (when dired-confirm-shell-command
+               (let ((res cmd)
+                     (regexp (regexp-quote str)))
+                 ;; Drop all ? and * surrounded by spaces and `?`.
+                 (while (and (string-match regexp res)
+                             (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
-- 
2.15.1