From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?K=C3=A9vin?= Le Gouguec Newsgroups: gmane.emacs.bugs Subject: bug#35564: [PATCH v3] Tweak dired warning about "wildcard" characters Date: Fri, 28 Jun 2019 08:15:44 +0200 Message-ID: <8736jujkvj.fsf@gmail.com> References: <87zho2cd4f.fsf@gmail.com> <87wohvf22u.fsf@gmail.com> <87h88cvpkj.fsf_-_@gmail.com> <87a7e27gh5.fsf@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="18907"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: 35564@debbugs.gnu.org, Stefan Monnier To: Noam Postavsky Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Jun 28 08:17:26 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hgkCL-0004lC-Sw for geb-bug-gnu-emacs@m.gmane.org; Fri, 28 Jun 2019 08:17:26 +0200 Original-Received: from localhost ([::1]:57174 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hgkCK-00044f-FR for geb-bug-gnu-emacs@m.gmane.org; Fri, 28 Jun 2019 02:17:24 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36427) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hgkBC-0003zs-9C for bug-gnu-emacs@gnu.org; Fri, 28 Jun 2019 02:16:24 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hgkB4-0008SL-Mc for bug-gnu-emacs@gnu.org; Fri, 28 Jun 2019 02:16:11 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:54941) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hgkB0-0008QD-H3 for bug-gnu-emacs@gnu.org; Fri, 28 Jun 2019 02:16:04 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hgkB0-0002tw-BO for bug-gnu-emacs@gnu.org; Fri, 28 Jun 2019 02:16:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?K=C3=A9vin?= Le Gouguec Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 28 Jun 2019 06:16:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 35564 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 35564-submit@debbugs.gnu.org id=B35564.156170255611140 (code B ref 35564); Fri, 28 Jun 2019 06:16:02 +0000 Original-Received: (at 35564) by debbugs.gnu.org; 28 Jun 2019 06:15:56 +0000 Original-Received: from localhost ([127.0.0.1]:40252 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hgkAu-0002tc-0A for submit@debbugs.gnu.org; Fri, 28 Jun 2019 02:15:56 -0400 Original-Received: from mail-wr1-f42.google.com ([209.85.221.42]:44661) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hgkAq-0002tN-2b for 35564@debbugs.gnu.org; Fri, 28 Jun 2019 02:15:52 -0400 Original-Received: by mail-wr1-f42.google.com with SMTP id r16so3068621wrl.11 for <35564@debbugs.gnu.org>; Thu, 27 Jun 2019 23:15:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=xe2m+v0xUg9Eo1v92utAIaHQGmR/rlIBcKJ75jq4k0w=; b=VrMuuE6mTLpS0/JRTVtRp4xOZWUIBttgoJ9IS2cfvwp7VkzDL1aTtGUf0pzwshuV8X 0cDZWDurwalh7c8FGiJnNSuhksHY2jMJhFXioIIHAuTosX6mDbpP2oc8yiJnCz6OiS1z kOlMR8J116mxH0dSVulQHoQ5oLUHQpU0QPx4iozzfH2R3slbt2gwvh20wELI2twDWhJ+ UqLAvY7CKiToherXJtM6k+NIVVsQ30Fkjg+bzTY2/TRLHTIBRsY4mcdFNBxeEreGkIWY ceRv89DWBBQXFG8C1PqCwrSWC5Wj0xj2mZ54qw92yZB6VxWiU8DwoT4/chfqPNjNnGSF UyCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=xe2m+v0xUg9Eo1v92utAIaHQGmR/rlIBcKJ75jq4k0w=; b=hklLw2PQPpl4gc6P2chf16wVPyvySaSrT2ljZbW8w7+gPyC18dhaBKLQk80Bn3QP2A 8XUcs+37EUsXlkexMlCOjD+u4+idUiEqDwSVYJ5r0AmpiO9IQC45FLqFfnLJAvQ6dotx Gg9Le71Wdxn+Klc/eXpkK6yzrlHfev6Pkwhxk+xMZFJRDRINQxfP/XXK43gcHT+KzmKD +rb3pTyjCQIbei/392qNxBHwLwewZB7IdG+p8E+Hmnul0KYmUaBFD3aKsuJoTnK2sZm8 r9ZtANk+N9kAvBJ82V+dYtfel8XleFE/qkWzOFc+lP7pulb4EI/xC/fjtYRgbSO9PHV5 vYNQ== X-Gm-Message-State: APjAAAX3l28al2O5Ille2sXD3tPHAWRNdybGpDoJJdV9z18PjGOxnZAU /sFkAKJRyCDkTmtiflA10Fs= X-Google-Smtp-Source: APXvYqxpCRICAfwFiMlyUqrUploMTLkI1uTBOCDL0keqOayQu5teSC7Xf+I/y58zKWyL9I7KItTwnw== X-Received: by 2002:adf:f98b:: with SMTP id f11mr545257wrr.231.1561702546295; Thu, 27 Jun 2019 23:15:46 -0700 (PDT) Original-Received: from my-little-tumbleweed (71.142.13.109.rev.sfr.net. [109.13.142.71]) by smtp.gmail.com with ESMTPSA id o4sm1046553wmh.35.2019.06.27.23.15.44 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Thu, 27 Jun 2019 23:15:45 -0700 (PDT) In-Reply-To: <87a7e27gh5.fsf@gmail.com> (Noam Postavsky's message of "Thu, 27 Jun 2019 19:31:34 -0400") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:161684 Archived-At: --=-=-= Content-Type: text/plain Noam Postavsky writes: > Yes, this was kind of bugging me, so here's a patch to reuse that logic > (it applies on top of yours). While doing this I noticed that we > earlier missed that `*` is not considered isolated, unlike `?`. Thank you very much! > Assuming no problems, I'll just push your two patches followed by mine Fine by me, although it just occurs to me that I forgot to mention the bug number in my commit messages. Here are updated patches: --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-Preserve-text-properties-in-y-or-n-p-prompts.patch >From f34a6271f86e2f6045fc8e37c157676e046f85a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= Date: Fri, 7 Jun 2019 17:03:59 +0200 Subject: [PATCH 1/2] Preserve text properties in y-or-n-p prompts * lisp/subr.el (read--propertize-prompt): New function to append the prompt face to a string. (y-or-n-p): Use it instead of discarding potential text properties. (Bug#35564) --- lisp/subr.el | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lisp/subr.el b/lisp/subr.el index baff1e909a..67c4f1da3a 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -2334,6 +2334,9 @@ memory-limit ;;;; Input and display facilities. +(defun read--propertize-prompt (prompt) + (add-face-text-property 0 (length prompt) 'minibuffer-prompt t prompt)) + (defconst read-key-empty-map (make-sparse-keymap)) (defvar read-key-delay 0.01) ;Fast enough for 100Hz repeat rate, hopefully. @@ -2671,14 +2674,14 @@ y-or-n-p (let* ((scroll-actions '(recenter scroll-up scroll-down scroll-other-window scroll-other-window-down)) (key - (let ((cursor-in-echo-area t)) + (let ((cursor-in-echo-area t) + (prompt (if (memq answer scroll-actions) + prompt + (concat "Please answer y or n. " prompt)))) (when minibuffer-auto-raise (raise-frame (window-frame (minibuffer-window)))) - (read-key (propertize (if (memq answer scroll-actions) - prompt - (concat "Please answer y or n. " - prompt)) - 'face 'minibuffer-prompt))))) + (read--propertize-prompt prompt) + (read-key prompt)))) (setq answer (lookup-key query-replace-map (vector key) t)) (cond ((memq answer '(skip act)) nil) -- 2.21.0 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0002-Tweak-dired-warning-about-wildcard-characters.patch >From a70bbeae31315a3c1b8baf979e27be3dbc4f7eac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= Date: Fri, 7 Jun 2019 17:19:44 +0200 Subject: [PATCH 2/2] Tweak dired warning about "wildcard" characters Non-isolated '?' and '*' characters may be quoted, or backslash-escaped; we do not know for a fact that the shell will interpret them as wildcards. Rephrase the prompt and highlight the characters so that the user sees exactly what we are talking about. * lisp/dired-aux.el (dired--isolated-char-p) (dired--highlight-nosubst-char, dired--no-subst-prompt): New functions. (dired-do-shell-command): Use them. * test/lisp/dired-aux-tests.el (dired-test-isolated-char-p) (dired-test-highlight-metachar): Test the new functions. (Bug#35564) --- lisp/dired-aux.el | 42 ++++++++++++++++++++++++++++++++---- test/lisp/dired-aux-tests.el | 28 ++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el index 5e4ec4d1ec..079e4f102f 100644 --- a/lisp/dired-aux.el +++ b/lisp/dired-aux.el @@ -79,6 +79,42 @@ dired--star-or-qmark-p (funcall (if keep #'string-match-p #'string-match) x string)) regexps))) +(defun dired--isolated-char-p (command pos) + "Assert whether the character at POS is isolated within COMMAND. +A character is isolated if: +- it is surrounded by whitespace, the start of the command, or + the end of the command, +- it is surrounded by `\\=`' characters." + (let ((start (max 0 (1- pos))) + (char (string (aref command pos)))) + (and (string-match + (rx (or (seq (or bos blank) + (group-n 1 (literal char)) + (or eos blank)) + (seq ?` (group-n 1 (literal char)) ?`))) + command start) + (= pos (match-beginning 1))))) + +(defun dired--highlight-nosubst-char (command char) + "Highlight occurences of CHAR that are not isolated in COMMAND. +These occurences will not be substituted; they will be sent as-is +to the shell, which may interpret them as wildcards." + (save-match-data + (let ((highlighted (substring-no-properties command)) + (pos 0)) + (while (string-match (regexp-quote char) command pos) + (let ((start (match-beginning 0)) + (end (match-end 0))) + (unless (dired--isolated-char-p command start) + (add-face-text-property start end 'warning nil highlighted)) + (setq pos end))) + highlighted))) + +(defun dired--no-subst-prompt (command char) + (let ((highlighted-command (dired--highlight-nosubst-char command char)) + (prompt "Confirm--the highlighted characters will not be substituted:")) + (format-message "%s\n%s\nProceed?" prompt highlighted-command))) + ;;;###autoload (defun dired-diff (file &optional switches) "Compare file at point with FILE using `diff'. @@ -759,11 +795,9 @@ dired-do-shell-command (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? "))) + (y-or-n-p (dired--no-subst-prompt command "*"))) ((need-confirm-p command "?") - (y-or-n-p (format-message - "Confirm--do you mean to use `?' as a wildcard? "))) + (y-or-n-p (dired--no-subst-prompt command "?"))) (t)))) (cond ((not ok) (message "Command canceled")) (t diff --git a/test/lisp/dired-aux-tests.el b/test/lisp/dired-aux-tests.el index ccd3192792..80b6393931 100644 --- a/test/lisp/dired-aux-tests.el +++ b/test/lisp/dired-aux-tests.el @@ -114,6 +114,34 @@ dired-test-bug30624 (mapc #'delete-file `(,file1 ,file2)) (kill-buffer buf))))) +(ert-deftest dired-test-isolated-char-p () + (should (dired--isolated-char-p "?" 0)) + (should (dired--isolated-char-p "? " 0)) + (should (dired--isolated-char-p " ?" 1)) + (should (dired--isolated-char-p " ? " 1)) + (should (dired--isolated-char-p "foo bar ? baz" 8)) + (should (dired--isolated-char-p "foo -i`?`" 7)) + (should-not (dired--isolated-char-p "foo `bar`?" 9)) + (should-not (dired--isolated-char-p "foo 'bar?'" 8)) + (should-not (dired--isolated-char-p "foo bar?baz" 7)) + (should-not (dired--isolated-char-p "foo bar?" 7))) + +(ert-deftest dired-test-highlight-metachar () + "Check that non-isolated meta-characters are highlighted" + (let* ((command "sed -r -e 's/oo?/a/' -e 's/oo?/a/' ? `?`") + (result (dired--highlight-nosubst-char command "?"))) + (should-not (text-property-not-all 1 14 'face nil result)) + (should (equal 'warning (get-text-property 15 'face result))) + (should-not (text-property-not-all 16 28 'face nil result)) + (should (equal 'warning (get-text-property 29 'face result))) + (should-not (text-property-not-all 30 39 'face nil result))) + (let* ((command "sed -e 's/o*/a/' -e 's/o*/a/'") + (result (dired--highlight-nosubst-char command "*"))) + (should-not (text-property-not-all 1 10 'face nil result)) + (should (equal 'warning (get-text-property 11 'face result))) + (should-not (text-property-not-all 12 23 'face nil result)) + (should (equal 'warning (get-text-property 24 'face result))) + (should-not (text-property-not-all 25 29 'face nil result)))) (provide 'dired-aux-tests) ;; dired-aux-tests.el ends here -- 2.21.0 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > +(defun dired--no-subst-prompt (command char-positions) > [=E2=80=A6] > + ((setq confirmations (dired--need-confirm-positions comman= d "*")) > + (y-or-n-p (dired--no-subst-prompt confirmations command))) > + ((setq confirmations (dired--need-confirm-positions comman= d "?")) > + (y-or-n-p (dired--no-subst-prompt command confirmations))) Mmmm, am I missing something, or have the arguments to dired--no-subst-prompt been reversed in the "*" case? I.e. shoudn't > + (y-or-n-p (dired--no-subst-prompt confirmations command))) rather be > + (y-or-n-p (dired--no-subst-prompt command confirmations))) ? As things stand, "! grep 'foo.*'" in Dired fails, saying: let: Wrong type argument: stringp, (9) --=-=-=--