GNU bug report logs -
#35564
27.0.50; [PATCH] Tweak dired-do-shell-command warning about "wildcard" characters
Previous Next
Reported by: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Date: Sat, 4 May 2019 18:03:02 UTC
Severity: normal
Tags: fixed, moreinfo, patch
Merged with 28969
Found in version 27.0.50
Fixed in version 28.1
Done: Lars Ingebrigtsen <larsi <at> gnus.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 35564 in the body.
You can then email your comments to 35564 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 04 May 2019 18:03:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Kévin Le Gouguec <kevin.legouguec <at> gmail.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sat, 04 May 2019 18:03:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello,
The function dired-do-shell-command checks the user's command for any
star or question mark not surrounded by whitespace or backquotes,
asking whether they are deliberate, since the character will then be
sent as-is to the shell, instead of being replaced with the marked
file(s).
A silly example:
- Open a Dired buffer
- M-! echo "Foobar." > foo RET
- g
- with point on foo:
- ! sed 's/\./?/' RET
The way the question is phrased bothers me:
> Confirm--do you mean to use `?' as a wildcard?
The first time I met this prompt was when I included a quoted '?' in
my command as in the example above, so I definitely did *not* mean to
use '?' as a shell wildcard.
Even now, knowing what the question really means, it still trips my
brain that I must answer "yes" (as in, "yes, I know Dired will not
substitute the marked files") when I mean "no" (as in, "no, I don't
mean to use '?' as a wildcard, what is this even ab- oh wait no right
I meant yes! Yes! 🤦").
I can think of a few ways to solve this:
1. Rephrase the question to be more general, specifically without
calling the characters "wildcards"; for example:
> Confirm--do you mean to send `?' to the shell without substitution?
2. Parse the command to find out whether the shell will actually use
these characters as wildcards.
- not sure how portable this would be across different shells
- AFAICT the aim of this prompt is simply to warn the user that
Dired will not expand these characters; whether the shell will
process them as wildcards is irrelevant
3. Add an option to skip this question (more of a workaround than a
solution).
Favoring option #1, I tried to find alternative questions, but none of
the ones I came up with sounded satisfying (most of them included some
form of double-negation, which is not the kind of puzzle I want to
solve when I'm about to run a hastily-put-together Bash oneliner).
I played around with the idea of actually *showing* the
"unsubstituted" characters to the user in order to be able to say
something like…
> Confirm--the highlighted characters will not be substituted.
> Proceed?
… and ended up with the attached patch, which I am not entirely
satisfied with (for one, it replaces `y-or-n-p' with `yes-or-no-p'
merely because the former seems to strip my prompt's text attributes
somehow[1]).
[0001-Make-dired-do-shell-command-highlight-unsubstituted-.patch (text/x-diff, attachment)]
[Message part 3 (text/plain, inline)]
WDYT? Assuming that Dired calling unsubstituted characters
"wildcards" is indeed a problem,
- can someone come up with a better phrasing?
- is the highlighting, as implemented in this patch, helpful?
- does anybody know why `y-or-n-p' prompts lose their face property?
Thank you for your time.
Kévin
[1] Compare:
(let ((prompt "foobar "))
(add-face-text-property 3 6 'warning nil prompt)
(yes-or-no-p prompt))
With:
(let ((prompt "foobar "))
(add-face-text-property 3 6 'warning nil prompt)
(y-or-n-p prompt))
In GNU Emacs 27.0.50 (build 2, i686-pc-linux-gnu, GTK+ Version 3.22.11)
of 2019-05-02 built on nc10-laptop
Repository revision: 17a722982cca4e8e643c7a9102903e820e784cc6
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11902000
System Description: BunsenLabs GNU/Linux 9.8 (Helium)
Recent messages:
Mark saved where search started
Mark set
Mark saved where search started
Mark set
Making completion list...
Quit [3 times]
Mark set
Quit [2 times]
Type "q" in help window to restore its previous buffer, C-M-v to scroll help.
Quit
Quit
Configured using:
'configure --with-xwidgets'
Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF
XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS XWIDGETS JSON
PDUMPER LCMS2 GMP
Important settings:
value of $LANG: en_US.UTF-8
locale-coding-system: utf-8-unix
Major mode: Emacs-Lisp
Minor modes in effect:
global-magit-file-mode: t
magit-file-mode: t
magit-auto-revert-mode: t
auto-revert-mode: t
global-git-commit-mode: t
async-bytecomp-package-mode: t
shell-dirtrack-mode: t
show-paren-mode: t
minibuffer-depth-indicate-mode: t
icomplete-mode: t
global-page-break-lines-mode: t
page-break-lines-mode: t
electric-pair-mode: t
diff-hl-flydiff-mode: t
global-diff-hl-mode: t
diff-hl-mode: t
delete-selection-mode: t
tooltip-mode: t
global-eldoc-mode: t
eldoc-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
column-number-mode: t
line-number-mode: t
transient-mark-mode: t
Load-path shadows:
None found.
Features:
(shadow sort emacsbug sendmail nndoc gnus-dup mm-archive url-cache
debbugs-gnu debbugs soap-client url-http url-auth url-gw url url-proxy
url-privacy url-expand url-methods url-history url-cookie url-domsuf
url-util warnings rng-xsd rng-dt rng-util xsd-regexp xml tabify man
mail-extr ffap pulse diff-hl-dired magit-patch flyspell ispell dired-aux
dired-x magit-extras hi-lock cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs cus-edit whitespace
find-dired xref magit-submodule magit-obsolete magit-blame magit-stash
magit-reflog magit-bisect magit-push magit-pull magit-fetch magit-clone
magit-remote magit-commit magit-sequence magit-notes magit-worktree
magit-tag magit-merge magit-branch magit-reset magit-files magit-refs
magit-status magit magit-repos magit-apply magit-wip magit-log
which-func imenu magit-diff smerge-mode magit-core magit-autorevert
autorevert filenotify magit-margin magit-transient magit-process
magit-mode transient git-commit magit-git magit-section log-edit
pcvs-util add-log with-editor async-bytecomp async server face-remap
eieio-opt speedbar sb-image ezimage dframe magit-utils crm dash shell
pcomplete ert pp gnus-async qp gnus-ml nndraft nnmh nnfolder utf-7
epa-file gnutls network-stream nsm gnus-agent gnus-srvr gnus-score
score-mode nnvirtual gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime
smime dig mailcap nntp gnus-cache gnus-sum gnus-group gnus-undo
gnus-start gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo
parse-time gnus-spec gnus-int gnus-range message rmc puny dired
dired-loaddefs format-spec rfc822 mml mml-sec epa derived epg mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
gnus-win gnus nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045
ietf-drums text-property-search time-date mail-utils mm-util mail-prsvr
wid-edit markdown-mode rx color noutline outline vc-mtn vc-hg jka-compr
cl-print debug backtrace find-func thingatpt help-fns radix-tree
executable misearch multi-isearch vc-git vc-bzr vc-src vc-sccs vc-svn
vc-cvs vc-rcs project delight advice eighters-theme quail cl-extra
help-mode rg rg-ibuffer rg-result wgrep-rg wgrep s rg-history rg-header
rg-compat ibuf-ext ibuffer ibuffer-loaddefs grep compile comint
ansi-color ring edmacro kmacro disp-table paren mb-depth icomplete
page-break-lines elec-pair diff-hl-flydiff diff diff-hl vc-dir ewoc vc
vc-dispatcher diff-mode easy-mmode delsel cus-start cus-load mule-util
tex-site info package easymenu epg-config url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting xwidget-internal move-toolbar
gtk x-toolkit x multi-tty make-network-process emacs)
Memory information:
((conses 8 708714 111581)
(symbols 24 31862 1)
(strings 16 136515 44484)
(string-bytes 1 4039230)
(vectors 8 60958)
(vector-slots 4 1347636 61628)
(floats 8 3359 1168)
(intervals 28 69005 689)
(buffers 564 56))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sun, 05 May 2019 08:45:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> … and ended up with the attached patch, which I am not entirely
> satisfied with (for one, it replaces `y-or-n-p' with `yes-or-no-p'
> merely because the former seems to strip my prompt's text attributes
> somehow[1]).
[...]
> [1] Compare:
>
> (let ((prompt "foobar "))
> (add-face-text-property 3 6 'warning nil prompt)
> (yes-or-no-p prompt))
>
> With:
>
> (let ((prompt "foobar "))
> (add-face-text-property 3 6 'warning nil prompt)
> (y-or-n-p prompt))
'y-or-n-p' propertizes the prompt rigidly as
(read-key (propertize (if (memq answer scroll-actions)
prompt
(concat "Please answer y or n. "
prompt))
'face 'minibuffer-prompt)))))
while 'yes-or-no-p' carefully applies 'minibuffer-prompt-properties'
to any text properties provided with PROMPT.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Mon, 06 May 2019 19:41:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
martin rudalics <rudalics <at> gmx.at> writes:
>> [1] Compare:
>>
>> (let ((prompt "foobar "))
>> (add-face-text-property 3 6 'warning nil prompt)
>> (yes-or-no-p prompt))
>>
>> With:
>>
>> (let ((prompt "foobar "))
>> (add-face-text-property 3 6 'warning nil prompt)
>> (y-or-n-p prompt))
>
> 'y-or-n-p' propertizes the prompt rigidly as
>
> (read-key (propertize (if (memq answer scroll-actions)
> prompt
> (concat "Please answer y or n. "
> prompt))
> 'face 'minibuffer-prompt)))))
>
> while 'yes-or-no-p' carefully applies 'minibuffer-prompt-properties'
> to any text properties provided with PROMPT.
Well, that's interesting.
I dug into yes-or-no-p until I came across `read_minibuf()'; is this the
code you are referring to?
if (PT > BEG)
{
Fput_text_property (make_fixnum (BEG), make_fixnum (PT),
Qfront_sticky, Qt, Qnil);
Fput_text_property (make_fixnum (BEG), make_fixnum (PT),
Qrear_nonsticky, Qt, Qnil);
Fput_text_property (make_fixnum (BEG), make_fixnum (PT),
Qfield, Qt, Qnil);
if (CONSP (Vminibuffer_prompt_properties))
{
/* We want to apply all properties from
`minibuffer-prompt-properties' to the region normally,
but if the `face' property is present, add that
property to the end of the face properties to avoid
overwriting faces. */
Lisp_Object list = Vminibuffer_prompt_properties;
while (CONSP (list))
{
Lisp_Object key = XCAR (list);
list = XCDR (list);
if (CONSP (list))
{
Lisp_Object val = XCAR (list);
list = XCDR (list);
if (EQ (key, Qface))
Fadd_face_text_property (make_fixnum (BEG),
make_fixnum (PT), val, Qt, Qnil);
else
Fput_text_property (make_fixnum (BEG), make_fixnum (PT),
key, val, Qnil);
}
}
}
}
If one were to fix the issue of y-or-n-p hardcoding the face property,
what would be the best way to go?
1. Make a C DEFUN out of this snippet, and have it called by
`read_minibuf()' and `y-or-n-p'.
2. Re-implement this snippet as an Elisp defun, and have it called by
`read_minibuf()' and `y-or-n-p'.
3. (Re-implement this snippet within `y-or-n-p'.)
(FWIW, the attached patch seems to work as a workaround, but I assume
solutions 1 or 2 would be better, by virtue of reusing code)
[0001-Make-y-or-no-p-keep-the-supplied-prompt-s-face.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
Thanks for your help!
Kévin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Tue, 07 May 2019 08:16:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> I dug into yes-or-no-p until I came across `read_minibuf()'; is this the
> code you are referring to?
It is.
> If one were to fix the issue of y-or-n-p hardcoding the face property,
> what would be the best way to go?
>
> 1. Make a C DEFUN out of this snippet, and have it called by
> `read_minibuf()' and `y-or-n-p'.
>
> 2. Re-implement this snippet as an Elisp defun, and have it called by
> `read_minibuf()' and `y-or-n-p'.
>
> 3. (Re-implement this snippet within `y-or-n-p'.)
>
> (FWIW, the attached patch seems to work as a workaround, but I assume
> solutions 1 or 2 would be better, by virtue of reusing code)
By design, 'yes-or-no-p' and 'y-or-n-p' are kept apart to avoid that
people use the latter for more "crucial decisions". Part of this
distinction is that 'y-or-n-p' is asked in the echo area, so applying
'minibuffer-prompt-properties' would be conceptually inappropriate.
Obviously, applying 'minibuffer-prompt' is just as inappropriate (that
face is part of 'minibuffer-prompt-properties') but that's a decision
that has been made long ago.
So although I'd vote for a solution like the one you propose in your
patch, any decision in this area is subtle and should be approved by
others first. Also because we'd then have to decide what to do with
other clients of the 'minibuffer-prompt' face like 'read-char-choice'
or the ones in isearch.el.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Tue, 07 May 2019 13:20:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> By design, 'yes-or-no-p' and 'y-or-n-p' are kept apart to avoid that
> people use the latter for more "crucial decisions". Part of this
> distinction is that 'y-or-n-p' is asked in the echo area, so applying
> 'minibuffer-prompt-properties' would be conceptually inappropriate.
> Obviously, applying 'minibuffer-prompt' is just as inappropriate (that
> face is part of 'minibuffer-prompt-properties') but that's a decision
> that has been made long ago.
>
> So although I'd vote for a solution like the one you propose in your
> patch, any decision in this area is subtle and should be approved by
> others first. Also because we'd then have to decide what to do with
> other clients of the 'minibuffer-prompt' face like 'read-char-choice'
> or the ones in isearch.el.
I don't see any good reason why face `minibuffer-prompt'
should be used, especially by default (users can do
whatever they like) for situations where there is no
active minibuffer, i.e., for prompting situations
generally. It should instead serve as a useful clue
that the minibuffer is being used. (Just one opinion.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 08 May 2019 20:43:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Drew Adams <drew.adams <at> oracle.com> writes:
> I don't see any good reason why face `minibuffer-prompt'
> should be used, especially by default (users can do
> whatever they like) for situations where there is no
> active minibuffer, i.e., for prompting situations
> generally. It should instead serve as a useful clue
> that the minibuffer is being used. (Just one opinion.)
I'd hazard a guess[1] that this was done to make y-or-n-p and
yes-or-no-p similar from a UI point of view: they both prompt the user
for a binary answer, therefore the prompt might as well look the same in
both situations.
From what I can tell the use of 'minibuffer-prompt' in minibuffer-less
situations has enough precedents (the "other clients" Martin mentions)
that the "minibuffer-" prefix might be considered a historical accident
by now…
(Perhaps those clients could be migrated to a new face,
e.g. 'message-prompt', which would inherit 'minibuffer-prompt' by
default?)
martin rudalics <rudalics <at> gmx.at> writes:
> So although I'd vote for a solution like the one you propose in your
> patch, any decision in this area is subtle and should be approved by
> others first. Also because we'd then have to decide what to do with
> other clients of the 'minibuffer-prompt' face like 'read-char-choice'
> or the ones in isearch.el.
Fair enough. Should I raise the issue on emacs-devel, or create a new
bug report? Just to make sure I am not omitting something, is this how
you would sum up the issue?
- In the context of bug#35564, I would like to add text properties to
the y-or-n-p prompt (although I'm open to other, simpler solutions
e.g. simply changing Dired's message).
- While this can be patched within y-or-n-p and we can call it a day,
the minibuffer-prompt-face-adding code could be factored out of
'read_minibuf'.
- This raises two questions:
1. Do we actually want to use the 'minibuffer-prompt' face in this
context, since the minibuffer is not involved?
2. What do we do with other clients of 'minibuffer-prompt', which
use the same (propertize prompt 'face 'minibuffer-prompt) idiom?
Thank you both for your thoughts on this.
[1] AFAICT, the commit that added this face (927be33, back when y-or-n-p
was still a C function) does not say why this was thought to be a
good idea.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 08 May 2019 22:41:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> Drew Adams <drew.adams <at> oracle.com> writes:
>
> > I don't see any good reason why face `minibuffer-prompt'
> > should be used, especially by default (users can do
> > whatever they like) for situations where there is no
> > active minibuffer, i.e., for prompting situations
> > generally. It should instead serve as a useful clue
> > that the minibuffer is being used. (Just one opinion.)
>
> I'd hazard a guess[1] that this was done to make y-or-n-p and
> yes-or-no-p similar from a UI point of view: they both prompt the user
> for a binary answer, therefore the prompt might as well look the same in
> both situations.
>
> From what I can tell the use of 'minibuffer-prompt' in minibuffer-less
> situations has enough precedents (the "other clients" Martin mentions)
> that the "minibuffer-" prefix might be considered a historical accident
> by now…
Not an accident. The "precedents" are themselves bugs,
IMO. The minibuffer is the minibuffer. It's important
that users be aware when it is active. That may not
matter much for `yes-or-no-p', as it insists on "yes"
or "no". But it is important for the minibuffer in
general. The minibuffer is an editing buffer; you can
do lots of things in it and with it.
> (Perhaps those clients could be migrated to a new face,
> e.g. 'message-prompt', which would inherit 'minibuffer-prompt' by
> default?)
That would be fine by me. Or no face. Is it really
important that the prompt be distinguished by a face?
(To be clear, I have no objection to our using faces
for prompts.)
> - In the context of bug#35564, I would like to add text properties to
> the y-or-n-p prompt (although I'm open to other, simpler solutions
> e.g. simply changing Dired's message).
I wouldn't mention bug #35564. And that bug should not,
itself, also deal with prompt faces. It should be only
about `dired-do-shell-command' warning about wildcard
characters.
Any question (for this command or in general) about
prompt faces is independent of this bug report about
the `dired-do-shell-command' treatment of wildcards.
(Just one opinion.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 09 May 2019 08:14:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> (Perhaps those clients could be migrated to a new face,
> e.g. 'message-prompt', which would inherit 'minibuffer-prompt' by
> default?)
Simply 'prompt' would be more intuitive since most messages do not show
prompts. But I'm afraid we'll have to stick to what we have now.
> Fair enough. Should I raise the issue on emacs-devel, or create a new
> bug report? Just to make sure I am not omitting something, is this how
> you would sum up the issue?
>
> - In the context of bug#35564, I would like to add text properties to
> the y-or-n-p prompt (although I'm open to other, simpler solutions
> e.g. simply changing Dired's message).
>
> - While this can be patched within y-or-n-p and we can call it a day,
> the minibuffer-prompt-face-adding code could be factored out of
> 'read_minibuf'.
>
> - This raises two questions:
>
> 1. Do we actually want to use the 'minibuffer-prompt' face in this
> context, since the minibuffer is not involved?
>
> 2. What do we do with other clients of 'minibuffer-prompt', which
> use the same (propertize prompt 'face 'minibuffer-prompt) idiom?
That would sum it up (but since I don't use Dired I can't comment on
that).
Note that this isssue also touches one Drew raised elsewhere - whether
'tooltip-show' should retain face properties of the original text or
show text uniformly with the 'tooltip' face. Maybe we should
introduce an option like 'prompts-retain-text-properites' so users
have the choice.
> [1] AFAICT, the commit that added this face (927be33, back when y-or-n-p
> was still a C function) does not say why this was thought to be a
> good idea.
It's probably part of a concept to have prompts appear uniformly and
integrate them into the framework of themes. Note that Emacs usually
cannot control the appearance of dialog boxes or system tooltips.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 09 May 2019 14:21:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> > (Perhaps those clients could be migrated to a new face,
> > e.g. 'message-prompt', which would inherit 'minibuffer-prompt' by
> > default?)
>
> Simply 'prompt' would be more intuitive since most messages do not show
> prompts.
Sounds OK to me.
> But I'm afraid we'll have to stick to what we have now.
Why? Not sure what you mean.
> Note that this isssue also touches one Drew raised elsewhere - whether
> 'tooltip-show' should retain face properties of the original text or
> show text uniformly with the 'tooltip' face. Maybe we should
> introduce an option like 'prompts-retain-text-properites' so users
> have the choice.
I would prefer that the two be separated. Tooltip text
is quite different from prompts in use cases and behavior.
Wrt tooltips, I also don't see why we need an option, or
even a defvar. Tooltips should just accept and respect
propertized strings.
When you use `x-show-tip' there is no such problem - you
can apply properties as usual. It is only `help-echo'
tooltips that do not respect properties (AFAIK).
Another, simpler possibility, for dealing with face
`tooltip':
It is not possible to simply _bind_ a face for the
duration (or lex scope) of a function. But if we
use a face variable for this, e.g. `tooltip-face',
then it should be simple to do so.
IOW, work around the limitation that you cannot bind
a face by binding a face variable and using that for
`help-echo'. That should make it simple for any code
to control the appearance of the tooltip text.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 09 May 2019 17:52:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 35564 <at> debbugs.gnu.org (full text, mbox):
>> But I'm afraid we'll have to stick to what we have now.
>
> Why? Not sure what you mean.
I mean that it won't be easy to convince others that we need a new
face for propertizing prompts.
> I would prefer that the two be separated. Tooltip text
> is quite different from prompts in use cases and behavior.
They are similar in the following aspect: Both prompts and tooltips
are often displayed using toolkit functions. GTK tooltips are by
default not propertized because the system doesn't accept any face
properties for them. If Emacs used balloon tooltips on Windows,
propertizing them would not be possible either. And both 'y-or-n-p'
and 'yes-or-no-p', when implemented via dialog popups, don't adopt our
text properties either.
The question is now whether an application should accept the uniform
appearance of such objects as prescribed by the toolkit used and as
such obey the toolkit's look-and-feel or insist to use its own
implementations. Emacs leaves that choice to its users. Which means
that users are told things like "if you want this mode to behave as
intended, you have to customize variables like 'use-dialog-box' or
'x-gtk-use-system-tooltips'". Nothing bad with that, but some users
might be uncertain whether they should agree. In particular when such
an option affects all sorts of tooltips or prompts.
martin
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 09 May 2019 20:05:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> >> But I'm afraid we'll have to stick to what we have now.
> >
> > Why? Not sure what you mean.
>
> I mean that it won't be easy to convince others that we need a new
> face for propertizing prompts.
Why assume that? Who needs to be convinced?
If we can't have a new face for this then I'd like
to see non-minibuffer prompting have no face at all,
by default.
> > I would prefer that the two be separated. Tooltip text
> > is quite different from prompts in use cases and behavior.
>
> They are similar in the following aspect: Both prompts and tooltips
> are often displayed using toolkit functions.
That sounds like an implementation thing, not a user-level thing.
And as I said, `x-show-tooltip' has no problem with
showing propertized text.
> GTK tooltips are by
> default not propertized because the system doesn't accept any face
> properties for them. If Emacs used balloon tooltips on Windows,
> propertizing them would not be possible either. And both 'y-or-n-p'
> and 'yes-or-no-p', when implemented via dialog popups, don't adopt our
> text properties either.
Oh, so `x-show-tooltip' only supports properties on
some platforms (e.g. MS Windows)? If so, that's
unfortunate.
Yes, I don't expect window-dialogs to respect
propertized text. Again, unfortunate - but livable.
> The question is now whether an application should accept the uniform
> appearance of such objects as prescribed by the toolkit used and as
> such obey the toolkit's look-and-feel or insist to use its own
> implementations. Emacs leaves that choice to its users. Which means
> that users are told things like "if you want this mode to behave as
> intended, you have to customize variables like 'use-dialog-box' or
> 'x-gtk-use-system-tooltips'". Nothing bad with that, but some users
> might be uncertain whether they should agree. In particular when such
> an option affects all sorts of tooltips or prompts.
Got it; thx.
I would like to see Emacs allow, when possible, Emacsy
things as much as possible. But I can understand that
there can be some tradeoffs.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sun, 09 Jun 2019 11:09:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello,
Here is my second attempt at solving this issue.
To recap: dired-do-shell-command warns the user about non-isolated '*'
and '?' characters since the function will not substitute them. It
refers to these characters as "wildcards", which can be incorrect: they
may be quoted or backslash-escaped, in which case the shell will not
interpret them as wildcards.
My main motivation to change this warning is that it trips my brain to
have to answer "yes" ("yes, I want to use wildcards") when no wildcards
are involved.
I could not come up with a simple, self-sufficient rephrasing for the
warning, so I decided to display the command itself as part of the
warning prompt, highlighting the non-isolated characters.
The first patch adjusts y-or-n-p so that it preserves the prompt's text
properties. The second patch changes dired-do-shell-command's prompt.
[0001-Preserve-text-properties-in-y-or-n-p-prompts.patch (text/x-diff, attachment)]
[0002-Tweak-dired-warning-about-wildcard-characters.patch (text/x-diff, attachment)]
[Message part 4 (text/plain, inline)]
Sample screenshot:
[dired-warning-highlight.png (image/png, attachment)]
[Message part 6 (text/plain, inline)]
I am not sure these patches should be applied as-is. Some things I
wonder about:
1. About read--propertize-prompt…
1. Should the function return a copy of its argument instead of
propertizing it directly?
2. Is it properly named? Does it fit in subr.el? I placed it there
because I figured other users of read-char in subr.el could use
it, e.g. read-char-choice.
2. dired-aux.el already contains some logic to detect isolated
characters; I could not think of a way to re-use it, so I added my
own functions to find *non*-isolated characters. I added unit tests
for these new functions; still, there may be some redundancy there.
WDYT?
PS1: I am still absolutely open to simply rephrasing the prompt… I just
cannot come up with good alternatives.
PS2: CC'ing Stefan to follow up on the discussion on emacs-devel.
<https://lists.gnu.org/archive/html/emacs-devel/2019-05/msg00339.html>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 12 Jun 2019 12:24:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> +(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 ((n (length command))
> + (whitespace '(?\s ?\t)))
> + (or (= n 1)
> + (and (= pos 0)
> + (memq (elt command 1) whitespace))
> + (and (= pos (1- n))
> + (memq (elt command (1- pos)) whitespace))
> + (and
> + (> pos 0)
> + (< pos (1- n))
> + (let ((prev (elt command (1- pos)))
> + (next (elt command (1+ pos))))
> + (or (and (memq prev whitespace)
> + (memq next whitespace))
> + (and (= prev ?`)
> + (= next ?`))))))))
I think this might be better expressed in regexp:
(and (string-match
(rx-to-string '(seq (or bos (in blank ?`))
(group (eval (string (aref command pos))))
(or eos (in blank ?`))))
command (max 0 (1- pos)))
(= pos (match-beginning 1)))
Although this gives different results for things like:
(dired--isolated-char-p "?`foo`" 0)
Do we care about that? (if yes, then that's a missing test case)
> +(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)))
And perhaps the regexp above (if it's correct) can be integrated into
this search? (maybe not though, since negation isn't straightforward in
regexps)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 12 Jun 2019 14:30:01 GMT)
Full text and
rfc822 format available.
Message #44 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> (rx-to-string '(seq (or bos (in blank ?`))
> (group (eval (string (aref command pos))))
> (or eos (in blank ?`))))
Can't we use `(... ,.. ..) instead of `eval`?
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 13 Jun 2019 06:20:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> gmail.com> writes:
> I think this might be better expressed in regexp:
I guess it is; I just tend to get superstitious about messing with match
data.
> Although this gives different results for things like:
>
> (dired--isolated-char-p "?`foo`" 0)
>
> Do we care about that? (if yes, then that's a missing test case)
I think we do care; if I look at what the existing code says,
(dired--star-or-qmark-p "?`foo`" "?")
;; => nil
Off the top of my head, this is the best I can come up with to satisfy
this edge case:
(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 (or (string-match
(rx-to-string '(seq (or bos blank)
(group char)
(or eos blank)))
command start)
(string-match
(rx-to-string '(seq ?`
(group char)
?`))
command start))
(= pos (match-beginning 1)))))
> And perhaps the regexp above (if it's correct) can be integrated into
> this search? (maybe not though, since negation isn't straightforward in
> regexps)
I will look into that.
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
> Can't we use `(... ,.. ..) instead of `eval`?
>
>
> Stefan
That works too from what I tested, although it's not necessary anymore
with the new version above.
Thank you both for the review! I will come back with an updated patch
(with the new test case) Some Time Later™.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 13 Jun 2019 08:00:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> I guess it is; I just tend to get superstitious about messing with match
> data.
Instead, you should get superstitious about using the match data too
far from the corresponding match.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 13 Jun 2019 16:54:02 GMT)
Full text and
rfc822 format available.
Message #53 received at 35564 <at> debbugs.gnu.org (full text, mbox):
>
> I think we do care; if I look at what the existing code says,
>
> (dired--star-or-qmark-p "?`foo`" "?")
> ;; => nil
> (let ((start (max 0 (1- pos)))
> (char (string (aref command pos))))
> (rx-to-string '(seq (or bos blank)
> (group char)
`char' in this context translates to the "." regexp (i.e., any
character). Yeah it's a bit weird. I have a patch in mind to remove
the need for eval or rx-to-string. I'll send in a few days (to a new
bug, it's getting off-topic here).
Meanwhile, I suggest:
(let ((start (max 0 (1- pos)))
(char (aref command pos)))
(and (string-match
(rx-to-string `(or (seq (or bos blank)
(group-n 1 ,char)
(or eos blank))
(seq ?` (group-n 1 ,char) ?`)))
command start)
(= pos (match-beginning 1))))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Tue, 18 Jun 2019 08:54:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 35564 <at> debbugs.gnu.org (full text, mbox):
npostavs <at> gmail.com writes:
> (rx-to-string `(or (seq (or bos blank)
> (group-n 1 ,char)
> (or eos blank))
> (seq ?` (group-n 1 ,char) ?`)))
Ah! Thanks for teaching me about \(?NUM: ... \), I did not know this
Elisp extension. This makes the whole thing much more readable.
For a minute I thought that maybe this patch should also add (require
'rx) somewhere in dired-aux.el, but AFAICT this is not necessary since
rx-to-string is autoloaded… Do I understand correctly?
Thanks again for the review, and for your efforts with bug#36237.
(And thank you Stefan for your advice on being superstitious about the
right things :) )
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 19 Jun 2019 00:13:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> For a minute I thought that maybe this patch should also add (require
> 'rx) somewhere in dired-aux.el, but AFAICT this is not necessary since
> rx-to-string is autoloaded… Do I understand correctly?
Yes, that's correct.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 26 Jun 2019 06:17:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
And here is the third set of patches.
[0001-Preserve-text-properties-in-y-or-n-p-prompts.patch (text/x-patch, attachment)]
[0002-Tweak-dired-warning-about-wildcard-characters.patch (text/x-patch, attachment)]
[Message part 4 (text/plain, inline)]
The first patch is unchanged (it adjusts y-or-n-p so that the prompt's
text properties are preserved), the second patch uses the new
(literal …) feature from rx and adds a test case for
dired--isolated-char-p[1].
Quoting my previous email:
> Some things I wonder about:
>
> 1. About read--propertize-prompt…
>
> 1. Should the function return a copy of its argument instead of
> propertizing it directly?
>
> 2. Is it properly named? Does it fit in subr.el? I placed it there
> because I figured other users of read-char in subr.el could use
> it, e.g. read-char-choice.
>
> 2. dired-aux.el already contains some logic to detect isolated
> characters; I could not think of a way to re-use it, so I added my
> own functions to find *non*-isolated characters. I added unit tests
> for these new functions; still, there may be some redundancy there.
Thank you for your time.
[1] (should-not (dired--isolated-char-p "foo `bar`?" 9))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 26 Jun 2019 13:29:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Please do not hard-code any particular face. Ever.
(add-face-text-property 0 (length prompt)
'minibuffer-prompt t prompt))
^^^^^^^^^^^^^^^^^^
Yes, I know that the code this patch would replace
already does that. But it shouldn't.
Nothing is gained by imposing such things on users
and making it hard for them to control such behavior
as they wish.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 26 Jun 2019 14:34:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 35564 <at> debbugs.gnu.org (full text, mbox):
I wouldn't bother with the read--propertize-prompt auxiliary
function, but ... LGTM (including the use of a hardcoded face ;-)
Stefan
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> And here is the third set of patches.
>
>
>
>
> The first patch is unchanged (it adjusts y-or-n-p so that the prompt's
> text properties are preserved), the second patch uses the new
> (literal …) feature from rx and adds a test case for
> dired--isolated-char-p[1].
>
> Quoting my previous email:
>
>> Some things I wonder about:
>>
>> 1. About read--propertize-prompt…
>>
>> 1. Should the function return a copy of its argument instead of
>> propertizing it directly?
>>
>> 2. Is it properly named? Does it fit in subr.el? I placed it there
>> because I figured other users of read-char in subr.el could use
>> it, e.g. read-char-choice.
>>
>> 2. dired-aux.el already contains some logic to detect isolated
>> characters; I could not think of a way to re-use it, so I added my
>> own functions to find *non*-isolated characters. I added unit tests
>> for these new functions; still, there may be some redundancy there.
>
> Thank you for your time.
>
>
> [1] (should-not (dired--isolated-char-p "foo `bar`?" 9))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 27 Jun 2019 05:59:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Drew Adams <drew.adams <at> oracle.com> writes:
> Please do not hard-code any particular face. Ever.
>
> (add-face-text-property 0 (length prompt)
> 'minibuffer-prompt t prompt))
> ^^^^^^^^^^^^^^^^^^
>
> Yes, I know that the code this patch would replace
> already does that. But it shouldn't.
>
> Nothing is gained by imposing such things on users
> and making it hard for them to control such behavior
> as they wish.
While I am not entirely satisfied with the resolution of the previous
discussion on emacs-devel[1] (mostly because this thread arguably lacks
a resolution[2]), I'd like to cross this bug (i.e. Dired's
brain-tripping warning) off my list before tackling the more general
issue of read-* functions abusing the minibuffer-prompt face.
[1] https://lists.gnu.org/archive/html/emacs-devel/2019-05/msg00340.html
[2] https://lists.gnu.org/archive/html/emacs-devel/2019-05/msg00344.html
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 27 Jun 2019 06:16:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
> I wouldn't bother with the read--propertize-prompt auxiliary
> function, but ... LGTM
Fair enough. I figured having a function would reduce the inertia of
changing other prompts which are propertized rigidly, and make it easier
to (maybe, someday) change the face that these prompts use in one go.
Though it's probably better to wait for a few prompts to be adapted,
deduce some pattern in the way they are propertized, and then introduce
a new function to do the job. I'll submit a revised patch without the
auxiliary function.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 27 Jun 2019 23:32:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>> 2. dired-aux.el already contains some logic to detect isolated
>> characters
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 `?`.
Assuming no problems, I'll just push your two patches followed by mine
(I considered squashing, but probably that makes things harder to
follow in this case).
[0003-Dedup-dired-aux-isolated-char-searching.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 28 Jun 2019 06:16:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Noam Postavsky <npostavs <at> gmail.com> 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:
[0001-Preserve-text-properties-in-y-or-n-p-prompts.patch (text/x-patch, attachment)]
[0002-Tweak-dired-warning-about-wildcard-characters.patch (text/x-patch, attachment)]
[Message part 4 (text/plain, inline)]
> +(defun dired--no-subst-prompt (command char-positions)
> […]
> + ((setq confirmations (dired--need-confirm-positions command "*"))
> + (y-or-n-p (dired--no-subst-prompt confirmations command)))
> + ((setq confirmations (dired--need-confirm-positions command "?"))
> + (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)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 28 Jun 2019 15:36:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 35564 <at> debbugs.gnu.org (full text, mbox):
(Apologies if I missed something - I took only a
quick look at the patches and the bug report.)
The old question
"Confirm--do you mean to use `*' as a wildcard? "
Seemed clear enough, I thought. But I see now from
the bug report that it could be confusing.
But the new question (and actually it's not even a
question - it should be, no?) seems even less clear
to me:
"Confirm--the highlighted characters will not be substituted:"
(And too long - "highlighted chars won't be substituted"
says the same thing as "the highlighted...".)
Maybe something like this?
"Should highlighted chars be substituted? "
or
"Substitute highlighted occurrences of `*'? "
But see next -
1. It's not clear to me what someone will understand
by "substituted" here. What would (otherwise) be
substituted for what, where, and for what purpose?
What substitution are we talking about, and how
would a user be expected to know what we mean, here?
2. Are there multiple different "characterS" involved,
or is the confirmation about only _one_ character,
in (possibly) multiple locations - occurrences of
one char?
3. Is it the case that the new prompt does not, itself,
show the character? Do you have to look elsewhere
to see which char or chars(?) are meant by the
prompt? Shouldn't the prompt itself show the char?
I think more thought might need to be put into this,
by those who understand what the code actually does,
ways in which the resulting behavior could be
confusing, and just what it is we're asking the user
to confirm.
(I'm not one who really understands all of this. I'm
just saying that my guess is that things are still
not so clear after the patching.)
A final comment, which I'm not sure is relevant:
We should not, in any case, _rely_ on any
highlighting to get across meaning (semantics).
Highlighting should always be an extra - a
nice-to-have. Some users will not see the
highlighting - it cannot be the only thing that
gets the intended meaning across.
(Again, I'm not saying that we _are_ relying on
highlighting this way. I just want to be sure
we're not. We don't want to unnecessarily
introduce an accessibility problem.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 28 Jun 2019 17:59:01 GMT)
Full text and
rfc822 format available.
Message #86 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Drew Adams <drew.adams <at> oracle.com> writes:
> But the new question (and actually it's not even a
> question - it should be, no?) seems even less clear
> to me:
A question would probably make the interaction more fluid, I agree.
As fun as messing with text properties has been, I would still very much
favour a simpler solution in the form of a better question (where
"better" mostly means "does not talk about wildcards").
As I mentioned, the only reason I came up with these highlighting
shenanigans is because I could not come up with a better phrasing.
Point taken about the new phrasing possibly being less clear; I was
aiming for some sort of "is the following statement correct? yes/no"
interaction.
> (And too long - "highlighted chars won't be substituted"
> says the same thing as "the highlighted...".)
(Silly question: is it kosher to use contractions such as "won't" in
user-facing text? Or were you pointing out the superfluous "the" and/or
suggesting "chars" rather than "characters"?)
> "Should highlighted chars be substituted? "
>
> or
>
> "Substitute highlighted occurrences of `*'? "
Mmm; currently this prompt is raised when the code detects that the
characters will *not* be substituted; answering "yes" makes Dired go on
with the command, answering "no" aborts.
If we phrased the question like you suggest, we should probably change
the code to actually perform the substitutions should the user answer
"yes".
> 1. It's not clear to me what someone will understand
> by "substituted" here. What would (otherwise) be
> substituted for what, where, and for what purpose?
> What substitution are we talking about, and how
> would a user be expected to know what we mean, here?
Right. Not sure how to make things clearer without quoting
dired-do-shell-command's documentation, which would make the prompt
quite verbose.
> 2. Are there multiple different "characterS" involved,
> or is the confirmation about only _one_ character,
> in (possibly) multiple locations - occurrences of
> one char?
One character in (possibly) multiple locations.
> 3. Is it the case that the new prompt does not, itself,
> show the character? Do you have to look elsewhere
> to see which char or chars(?) are meant by the
> prompt? Shouldn't the prompt itself show the char?
It does; the prompt shows the full command, and applies the warning face
to the character(s).
> A final comment, which I'm not sure is relevant:
>
> We should not, in any case, _rely_ on any
> highlighting to get across meaning (semantics).
> Highlighting should always be an extra - a
> nice-to-have. Some users will not see the
> highlighting - it cannot be the only thing that
> gets the intended meaning across.
>
> (Again, I'm not saying that we _are_ relying on
> highlighting this way. I just want to be sure
> we're not. We don't want to unnecessarily
> introduce an accessibility problem.)
With the current patches, we absolutely totally completely _would_ rely
on highlighting to get across semantics. Thank you for spelling it out
as an accessibility problem; that kind of confirms my nagging feeling
that the highlighting method has an unfavorable benefit/cost ratio (IOW,
it's cute, but it might make things worse for some users).
So to conclude, these are the paths forward that I see:
(0. Drop the issue and grit my teeth when the warning shows up.)
1. find a simple rephrasing,
2. keep trying to make a more elaborate prompt, only using some other
tricks to point out the characters.
Example of path 1:
> Confirm--do you mean to send `*' verbatim to the shell?
(I don't like this one because it sounds like "do you want us to quote
`*' to make sure the shell does not expand it?")
Example of path 2:
> Confirm--do you mean to send these characters as-is to the shell?
> sed -e 's/foo?/foo!/' -e 's/bar?/bar!'
> ^ ^
(I.e. using '^' to denote the non-isolated characters; not sure how
clear it is that "these" refers to "the caracters underlined by a '^'")
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 28 Jun 2019 18:45:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> > (And too long - "highlighted chars won't be substituted"
> > says the same thing as "the highlighted...".)
>
> (Silly question: is it kosher to use contractions such as "won't" in
> user-facing text? Or were you pointing out the superfluous "the" and/or
> suggesting "chars" rather than "characters"?)
Yes, and yes, and yes.
Well, for the first "yes": I can't vouch for what's
kosher. But yes.
> > "Should highlighted chars be substituted? "
> >
> > or
> >
> > "Substitute highlighted occurrences of `*'? "
>
> Mmm; currently this prompt is raised when the code detects that the
> characters will *not* be substituted; answering "yes" makes Dired go on
> with the command, answering "no" aborts.
Yes, I assumed, in saying that, that the sense of
the code would need to be reversed if the question
was put that way. The question should be in a form
that's easiest for users to understand and not
misunderstand, if that's feasible in terms of code.
> If we phrased the question like you suggest, we should probably change
> the code to actually perform the substitutions should the user answer
> "yes".
Yes, that's what I meant. But that wording was
just a casual suggestion, to get the point across.
Someone (e.g. you) might well come up with something
better.
> > 1. It's not clear to me what someone will understand
> > by "substituted" here. What would (otherwise) be
> > substituted for what, where, and for what purpose?
> > What substitution are we talking about, and how
> > would a user be expected to know what we mean, here?
>
> Right. Not sure how to make things clearer without quoting
> dired-do-shell-command's documentation, which would make the prompt
> quite verbose.
Maybe someone else has a suggestion. It's worth
working on, I think.
> > 2. Are there multiple different "characterS" involved,
> > or is the confirmation about only _one_ character,
> > in (possibly) multiple locations - occurrences of
> > one char?
>
> One character in (possibly) multiple locations.
That's what I thought. Then don't say chars (plural).
> > 3. Is it the case that the new prompt does not, itself,
> > show the character? Do you have to look elsewhere
> > to see which char or chars(?) are meant by the
> > prompt? Shouldn't the prompt itself show the char?
>
> It does; the prompt shows the full command, and applies the warning face
> to the character(s).
Good.
> > A final comment, which I'm not sure is relevant:
> >
> > We should not, in any case, _rely_ on any
> > highlighting to get across meaning (semantics).
> > Highlighting should always be an extra - a
> > nice-to-have. Some users will not see the
> > highlighting - it cannot be the only thing that
> > gets the intended meaning across.
> >
> > (Again, I'm not saying that we _are_ relying on
> > highlighting this way. I just want to be sure
> > we're not. We don't want to unnecessarily
> > introduce an accessibility problem.)
>
> With the current patches, we absolutely totally completely _would_ rely
> on highlighting to get across semantics. Thank you for spelling it out
> as an accessibility problem; that kind of confirms my nagging feeling
> that the highlighting method has an unfavorable benefit/cost ratio (IOW,
> it's cute, but it might make things worse for some users).
There is likely another way to make those occurrences
stand out (in addition to, not instead of, highlighting).
But I'm no expert on that. Maybe Eli has a suggestion.
Emacs doesn't jump through zillions of hoops to try
maximize accessibility. But it's good to keep it in
mind and, at least when other things are equal, to DTRT
in this regard.
> So to conclude, these are the paths forward that I see:
>
> (0. Drop the issue and grit my teeth when the warning shows up.)
>
> 1. find a simple rephrasing,
>
> 2. keep trying to make a more elaborate prompt, only using some other
> tricks to point out the characters.
>
> Example of path 1:
>
> > Confirm--do you mean to send `*' verbatim to the shell?
You can drop "Confirm--". You could even drop
"do you mean to". If a reply is required to the
question (e.g. it's `y-or-n-p') then users cannot
avoid it or miss it.
> (I don't like this one because it sounds like "do you want us to quote
> `*' to make sure the shell does not expand it?")
>
> Example of path 2:
>
> > Confirm--do you mean to send these characters as-is to the shell?
> > sed -e 's/foo?/foo!/' -e 's/bar?/bar!'
> > ^ ^
>
> (I.e. using '^' to denote the non-isolated characters; not sure how
> clear it is that "these" refers to "the caracters underlined by a '^'")
Much better, IMO. Again: drop "Confirm--do you
mean to", and use "these occurrences of `?'", not
"these characters". There is only one char, in
perhaps multiple locations.
And I do think the char (`?' or whatever) should
be mentioned explicitly in the question, not just
have its occurrences indicated in the command to
be sent.
(Thanks for working on this.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 29 Jun 2019 13:49:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Drew Adams <drew.adams <at> oracle.com> writes:
>> > A final comment, which I'm not sure is relevant:
>> >
>> > We should not, in any case, _rely_ on any
>> > highlighting to get across meaning (semantics).
>> > Highlighting should always be an extra - a
>> > nice-to-have. Some users will not see the
>> > highlighting - it cannot be the only thing that
>> > gets the intended meaning across.
>> With the current patches, we absolutely totally completely _would_ rely
>> on highlighting to get across semantics. Thank you for spelling it out
>> as an accessibility problem; that kind of confirms my nagging feeling
>> that the highlighting method has an unfavorable benefit/cost ratio (IOW,
>> it's cute, but it might make things worse for some users).
> There is likely another way to make those occurrences
> stand out (in addition to, not instead of, highlighting).
> But I'm no expert on that. Maybe Eli has a suggestion.
>
> Emacs doesn't jump through zillions of hoops to try
> maximize accessibility. But it's good to keep it in
> mind and, at least when other things are equal, to DTRT
> in this regard.
Yes, we should definitely be careful not to make accessibility worse;
thank you for bringing this up Drew.
>> 1. find a simple rephrasing,
>>
>> > Confirm--do you mean to send `*' verbatim to the shell?
>> (I don't like this one because it sounds like "do you want us to quote
>> `*' to make sure the shell does not expand it?")
>> 2. keep trying to make a more elaborate prompt, only using some other
>> tricks to point out the characters.
>>
>> > Confirm--do you mean to send these characters as-is to the shell?
>> > sed -e 's/foo?/foo!/' -e 's/bar?/bar!'
>> > ^ ^
>>
>> (I.e. using '^' to denote the non-isolated characters; not sure how
>> clear it is that "these" refers to "the caracters underlined by a '^'")
I don't know about the '^' trick, if the minibuffer window is narrow
enough to cause line wrapping the result won't be very readable. And I
doubt a screen reader would handle this kind of thing any better than
highlighting (someone please correct me if I'm wrong about that).
I like the use of "as-is to shell": short and clear.
> Again: drop "Confirm--do you mean to", and use
> "these occurrences of `?'", not "these
> characters". There is only one char, in perhaps
> multiple locations.
>
> And I do think the char (`?' or whatever) should
> be mentioned explicitly in the question, not just
> have its occurrences indicated in the command to
> be sent.
Agreed on both these points. Updated patch is below, it produces
prompts like these (still using highlighting):
echo foo*
Send 1 occurence of ‘*’ as-is to shell? (y or n)
echo foo* bar* *
Send 2 occurences of ‘*’ as-is to shell? (y or n)
The last case (where there are both as-is and substituted "*") isn't so
great without highlighting (you have to count the "*"s and work out if
something unexpected is happening), but I think it's at least not worse
than the current situation.
[0003-Dedup-dired-aux-isolated-char-searching-Bug-35564.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 29 Jun 2019 14:14:03 GMT)
Full text and
rfc822 format available.
Message #95 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> Date: Fri, 28 Jun 2019 11:43:55 -0700 (PDT)
> From: Drew Adams <drew.adams <at> oracle.com>
> Cc: 35564 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>,
> Stefan Monnier <monnier <at> iro.umontreal.ca>
>
> > > We should not, in any case, _rely_ on any
> > > highlighting to get across meaning (semantics).
> > > Highlighting should always be an extra - a
> > > nice-to-have. Some users will not see the
> > > highlighting - it cannot be the only thing that
> > > gets the intended meaning across.
> > >
> > > (Again, I'm not saying that we _are_ relying on
> > > highlighting this way. I just want to be sure
> > > we're not. We don't want to unnecessarily
> > > introduce an accessibility problem.)
> >
> > With the current patches, we absolutely totally completely _would_ rely
> > on highlighting to get across semantics. Thank you for spelling it out
> > as an accessibility problem; that kind of confirms my nagging feeling
> > that the highlighting method has an unfavorable benefit/cost ratio (IOW,
> > it's cute, but it might make things worse for some users).
>
> There is likely another way to make those occurrences
> stand out (in addition to, not instead of, highlighting).
> But I'm no expert on that. Maybe Eli has a suggestion.
We could both highlight and underline the relevant text. We already
do that in other cases.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 29 Jun 2019 14:31:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> >> > Confirm--do you mean to send these characters as-is to the shell?
> >> > sed -e 's/foo?/foo!/' -e 's/bar?/bar!'
> >> > ^ ^
>
> I don't know about the '^' trick, if the minibuffer window is narrow
> enough to cause line wrapping the result won't be very readable. And I
> doubt a screen reader would handle this kind of thing any better than
> highlighting (someone please correct me if I'm wrong about that).
Another possibility I almost mentioned is to
use, by default, a face that uses `:box' or
`:overline', or some such properties to make
the char occurrences stand out without relying
on color. That might at least help with some
who have difficulty distinguishing color, but
it's not an ideal solution either.
I don't think we should try to jump through
too many hoops about this. The main thing,
I think, is to put the char itself in the
sentence preceding the quoted command text.
The use of `^' is not too bad, I think, even
given the problems you mention. If the char
occurrences that are problematic are not
obvious then a user can cancel the command
and check `*Messages*' for the full feedback.
> Agreed on both these points. Updated patch is below, it produces
> prompts like these (still using highlighting):
>
> echo foo*
> Send 1 occurence of ‘*’ as-is to shell? (y or n)
>
> echo foo* bar* *
> Send 2 occurences of ‘*’ as-is to shell? (y or n)
Good. But "occurrences", not "occurences".
> The last case (where there are both as-is and substituted "*") isn't so
> great without highlighting (you have to count the "*"s and work out if
> something unexpected is happening), but I think it's at least not worse
> than the current situation.
I vote for also adding the ^ indications underneath.
If you think that is too often too problematic then
maybe do something like one of these:
1. Give users a way to opt out or to remove that on
demand.
2. Automatically remove it, based on window width,
whether there are multiple lines, or whatever.
But this should be controllable by a user (e.g.
an option).
Agreed about use of screenreaders. Users should
be able to turn off the ^ indicators.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 03 Jul 2019 19:48:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
This series of patches consists in
- the same 2 patches as v3
- Noam's refactoring
- some amendments ("occurrence", order of arguments in tests)
- a proof-of-concept for marking the occurrences with '^'
Here is a list of improvements that I plan on tackling Soonish™
1. refrain from adding markers if the minibuffer is not wide enough,
2. use (:inherit '(warning underline)) instead of warning, so that
- if the warning face has some underlining, it is used,
- otherwise the underline face makes sure that we don't rely only on
colors.
Thank you all for your your reviews and your patience. Sorry I can't
manage to take more time to work on this.
[0001-Preserve-text-properties-in-y-or-n-p-prompts.patch (text/x-patch, attachment)]
[0002-Tweak-dired-warning-about-wildcard-characters.patch (text/x-patch, attachment)]
[0003-Dedup-dired-aux-isolated-char-searching-Bug-35564.patch (text/x-patch, attachment)]
[0004-fixup-Dedup-dired-aux-isolated-char-searching-Bug-35.patch (text/x-patch, attachment)]
[0005-Add-markers-below-non-isolated-chars-in-dired-prompt.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 12 Jul 2019 15:11:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello,
I have now added '^' markers below the highlighted command, on condition
that the echo area is wide enough not to wrap lines.
Do we want to add some customizability (highlight face, whether or not
to display '^' markers), or is this good enough for now?
The patch series now includes:
- two patches to make y-or-n-p preserve text properties and implement an
initial version of highlighting,
- Noam's refactoring patch, plus fixups,
- one patch to add '^' markers,
- one last patch to make tests less tedious to maintain.
[0001-Preserve-text-properties-in-y-or-n-p-prompts.patch (text/x-diff, attachment)]
[0002-Tweak-dired-warning-about-wildcard-characters.patch (text/x-diff, attachment)]
[0003-Dedup-dired-aux-isolated-char-searching-Bug-35564.patch (text/x-diff, attachment)]
[0004-fixup-Dedup-dired-aux-isolated-char-searching-Bug-35.patch (text/x-diff, attachment)]
[0005-Add-markers-below-non-isolated-chars-in-dired-prompt.patch (text/x-diff, attachment)]
[0006-Simplify-highlighting-assertions.patch (text/x-diff, attachment)]
[Message part 8 (text/plain, inline)]
Again, thank you for your patience and your reviews.
PS: the prompt now looks like this ('?' characters are highlighted with
the warning face):
With markers:
> Confirm:
> sed 's/\?/!/'
> ^
> Send 1 occurrence of ‘?’ as-is to shell?
Without markers:
> Confirm:
> sed 's/\?/!/'
> Send 1 occurrence of ‘?’ as-is to shell?
I added the "Confirm:" line because
- y-or-n-p adds "Please answer y or n. " before the prompt when the
user fails to answer correctly, so the markers would not line up if
the command remained on the first line,
- y-or-n-p adds " (y or n)" after the prompt; I find it more legible to
have the question next to this suffix, so I did not want to move the
question to the first line.
Merged 28969 35564.
Request was from
Noam Postavsky <npostavs <at> gmail.com>
to
control <at> debbugs.gnu.org.
(Sun, 14 Jul 2019 22:39:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 27 Jul 2019 11:21:02 GMT)
Full text and
rfc822 format available.
Message #109 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier
> <monnier <at> iro.umontreal.ca>, Drew Adams <drew.adams <at> oracle.com>, Noam
> Postavsky <npostavs <at> gmail.com>
> Date: Fri, 12 Jul 2019 17:10:26 +0200
>
> I have now added '^' markers below the highlighted command, on condition
> that the echo area is wide enough not to wrap lines.
>
> Do we want to add some customizability (highlight face, whether or not
> to display '^' markers), or is this good enough for now?
>
>
> The patch series now includes:
>
> - two patches to make y-or-n-p preserve text properties and implement an
> initial version of highlighting,
> - Noam's refactoring patch, plus fixups,
> - one patch to add '^' markers,
> - one last patch to make tests less tedious to maintain.
Any more comments, anyone?
Is there anything in these changes that would warrant a NEWS entry?
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 27 Jul 2019 17:27:02 GMT)
Full text and
rfc822 format available.
Message #112 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier
>> <monnier <at> iro.umontreal.ca>, Drew Adams <drew.adams <at> oracle.com>, Noam
>> Postavsky <npostavs <at> gmail.com>
>> Date: Fri, 12 Jul 2019 17:10:26 +0200
>>
>> I have now added '^' markers below the highlighted command, on condition
>> that the echo area is wide enough not to wrap lines.
>>
>> Do we want to add some customizability (highlight face, whether or not
>> to display '^' markers), or is this good enough for now?
>>
>>
>> The patch series now includes:
>>
>> - two patches to make y-or-n-p preserve text properties and implement an
>> initial version of highlighting,
>> - Noam's refactoring patch, plus fixups,
>> - one patch to add '^' markers,
>> - one last patch to make tests less tedious to maintain.
>
> Any more comments, anyone?
Michael had some comments over at bug#28969, but no objections AFAICT.
Michael, did you get the chance to try the patch out?
> Is there anything in these changes that would warrant a NEWS entry?
The changes are only cosmetic: the user interaction has not changed
(tell Dired to run a command, press 'y' to confirm). The prompt is
simply more verbose now.
There are no new variables for the user to customize either, if I am not
mistaken.
NB: I squashed all those patches in [2], to make it easier to try the
new prompt out. The squashed patch's commit message summarizes every
change, and mentions both bugs; I don't know if it makes more sense to
commit the series or the squashed patch to the repository.
[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28969#22
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28969#19
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 27 Jul 2019 22:04:02 GMT)
Full text and
rfc822 format available.
Message #115 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
> Any more comments, anyone?
Just a couple of very minor questions from me:
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> -(defun dired--no-subst-prompt (char-positions command)
> +(defun dired--mark-positions (positions)
> + (let ((markers (make-string
> + (1+ (apply #'max positions))
Is POSITIONS guaranteed to be non-nil? (The max function takes at least
one argument.)
> Subject: [PATCH 6/6] Simplify highlighting assertions
>
> * test/lisp/dired-aux-tests.el (dired-test--check-highlighting):
> New function.
> (dired-test-highlight-metachar): Use it.
Will this simplification hinder debugging of test failures? I don't
have an opinion on the proposed change, it's just something to consider.
> Again, thank you for your patience and your reviews.
Thank you for working on this,
--
Basil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 27 Jul 2019 22:24:02 GMT)
Full text and
rfc822 format available.
Message #118 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> Michael, did you get the chance to try the patch out?
I skimmed through the changes and have installed it, but I didn't yet
try it. Will do ASAP, hopefully tonight.
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 27 Jul 2019 23:33:01 GMT)
Full text and
rfc822 format available.
Message #121 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>> -(defun dired--no-subst-prompt (char-positions command)
>> +(defun dired--mark-positions (positions)
>> + (let ((markers (make-string
>> + (1+ (apply #'max positions))
>
> Is POSITIONS guaranteed to be non-nil? (The max function takes at least
> one argument.)
AFAICT dired--mark-positions is only called by dired--no-subst-prompt,
which is only used when there is at least one ambiguous character to
highlight.
So as things stand now, POSITIONS will always be non-nil. Nothing
prevents someone from attempting to re-use the function with a
potentially-nil argument though.
I don't know what makes more sense here: adding an assertion? Handling
the nil case explicitly for robustness?
>> Subject: [PATCH 6/6] Simplify highlighting assertions
>>
>> * test/lisp/dired-aux-tests.el (dired-test--check-highlighting):
>> New function.
>> (dired-test-highlight-metachar): Use it.
>
> Will this simplification hinder debugging of test failures? I don't
> have an opinion on the proposed change, it's just something to consider.
Mmm. Since the assertion that fails is now nested in a more generic
function, the report shown in the ERT-Results buffer might be somewhat
less informative; one has to bring up the backtrace to understand the
context.
I could try my hand at an ERT explainer for these assertions. Or we
could just drop the 6th patch… I do find the tests easier to read and
write with it though.
PS: Looking at this made me realize that patch #5 was borked (missed a
parenthesis in dired-test-highlight-metachar, so the tests just plain
wouldn't run). Here is the patch series with patches #5 and #6 fixed.
The squashed patch[1] remains the same.
[0001-Preserve-text-properties-in-y-or-n-p-prompts.patch (text/x-diff, attachment)]
[0002-Tweak-dired-warning-about-wildcard-characters.patch (text/x-diff, attachment)]
[0003-Dedup-dired-aux-isolated-char-searching-Bug-35564.patch (text/x-diff, attachment)]
[0004-fixup-Dedup-dired-aux-isolated-char-searching-Bug-35.patch (text/x-diff, attachment)]
[0005-Add-markers-below-non-isolated-chars-in-dired-prompt.patch (text/x-diff, attachment)]
[0006-Simplify-highlighting-assertions.patch (text/x-diff, attachment)]
[Message part 8 (text/plain, inline)]
Thank you for your review.
[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28969#19
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 27 Jul 2019 23:42:01 GMT)
Full text and
rfc822 format available.
Message #124 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>>> -(defun dired--no-subst-prompt (char-positions command)
>>> +(defun dired--mark-positions (positions)
>>> + (let ((markers (make-string
>>> + (1+ (apply #'max positions))
>>
>> Is POSITIONS guaranteed to be non-nil? (The max function takes at least
>> one argument.)
>
> AFAICT dired--mark-positions is only called by dired--no-subst-prompt,
> which is only used when there is at least one ambiguous character to
> highlight.
>
> So as things stand now, POSITIONS will always be non-nil. Nothing
> prevents someone from attempting to re-use the function with a
> potentially-nil argument though.
>
> I don't know what makes more sense here: adding an assertion? Handling
> the nil case explicitly for robustness?
I think it's fine the way it is, though a docstring/comment never hurts.
I was just checking.
>>> Subject: [PATCH 6/6] Simplify highlighting assertions
>>>
>>> * test/lisp/dired-aux-tests.el (dired-test--check-highlighting):
>>> New function.
>>> (dired-test-highlight-metachar): Use it.
>>
>> Will this simplification hinder debugging of test failures? I don't
>> have an opinion on the proposed change, it's just something to consider.
>
> Mmm. Since the assertion that fails is now nested in a more generic
> function, the report shown in the ERT-Results buffer might be somewhat
> less informative; one has to bring up the backtrace to understand the
> context.
>
> I could try my hand at an ERT explainer for these assertions. Or we
> could just drop the 6th patch… I do find the tests easier to read and
> write with it though.
That's good enough for me,
--
Basil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Mon, 29 Jul 2019 03:30:03 GMT)
Full text and
rfc822 format available.
Message #127 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> I skimmed through the changes and have installed it, but I didn't yet
> try it. Will do ASAP, hopefully tonight.
Ok, I did. It worked well without problems, and nicely indicates the
questionable character(s).
I dunno if the double emphasizing in the y-n-prompt (coloring +
additional underlining with "^") is a bit too much. A bit related:
Maybe the second line could be combined with the first line so that one
line is saved. I mean, the prompt is four lines high with this change,
quite a lot. Dunno what others think about it.
Anyway, I like it.
Thanks,
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Mon, 29 Jul 2019 18:17:01 GMT)
Full text and
rfc822 format available.
Message #130 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> I dunno if the double emphasizing in the y-n-prompt (coloring +
> additional underlining with "^") is a bit too much. A bit related:
Maybe additional underlining should be used only when the current
display doesn't support colors.
> Maybe the second line could be combined with the first line so that one
> line is saved. I mean, the prompt is four lines high with this change,
> quite a lot. Dunno what others think about it.
Then with using colors the prompt could fit into one line
(when the display supports colors).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Mon, 29 Jul 2019 19:03:02 GMT)
Full text and
rfc822 format available.
Message #133 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> 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?
[0001-Tweak-dired-do-shell-command-warning-some-more.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
(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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 02 Aug 2019 05:33:01 GMT)
Full text and
rfc822 format available.
Message #136 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> 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…
Yeah, as mentioned, when coloring is possible, I would also just leave
out the ^ underlining.
> (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)
Yes, I suggested to combine Confirm... with the command in the first line.
> … 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.)
Looks fine.
> 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,
No, a valid concern, and it's good that you noticed.
> 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?
I'm not happy with that either. Look at it: there are quotes around the
critical part, so the user might become crazy trying to find out why
Emacs thinks the shell might interpret that as a wildcard. The
highlighting even more makes it look like there is something wrong with
the command.
Even "proceed anyway" as you chose makes it sound like this is an
exception/ something wrong although it is totally legitimate.
Maybe just telling what dired did not do would be better? Like
"N occurrences of X will not be replaced with the file/file list -
proceed?
Because, there are two things we mix up: (1) dired does not substitute
with files, though the user might have wanted that, and (2) the char is
send to the shell and is a wildcard there, so the result might also not
be what the user intended. Do we want to warn about (1) or (2)? Both
seems to be too much for one line of text.
If we don't find a good wording maybe use something like
`read-multiple-choice' or some other mechanism where the user is allowed
to hit a help key (?, and that key should also be visible in the
prompt). We can leave an explanation that doesn't have to fit into one
line in the help text. I only mention `read-multiple-choice' because it
provides all of that out of the box, of course there are alternative
ways.
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 08 Aug 2019 10:41:02 GMT)
Full text and
rfc822 format available.
Message #139 received at 35564 <at> debbugs.gnu.org (full text, mbox):
First, apologies for taking so long to respond - I was AFK last week. I
might not be very reactive these coming weeks either.
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Yeah, as mentioned, when coloring is possible, I would also just leave
> out the ^ underlining.
OK. What exactly do we mean by "coloring is possible"?
1. "Does the warning face have a distinct foreground or background from
the prompt face?"
2. "Are the colors distinguishable enough?" (e.g. what
shr-color-visible tries to guess IIUC)
3. Something else?
What bothers me is that even if we can assert #2, nothing guarantees
that these colors will be distinguishable *to the user* (who may
e.g. have some form of color blindness). It would therefore be nice if
this user could force Emacs to use ^ markers; I guess that would involve
a new variable.
I stayed away from this path because I wasn't convinced that we needed a
full-blown customization option for a supposedly rare branch in
dired-do-shell-command, and that we could live with the redundant
coloring plus underlining.
I'd be happy to make the prompt more succinct, as soon as I'm sure I
understand what we mean by "coloring is possible"!
>> > Warning: the shell may interpret 1 occurrence of `?' as wildcard:
>> > sed 's/?/!/'
>> > ^
>> > Proceed anyway?
>
> I'm not happy with that either. Look at it: there are quotes around the
> critical part, so the user might become crazy trying to find out why
> Emacs thinks the shell might interpret that as a wildcard.
Right. Becoming crazy because Emacs sees phantom wildcards is the
reason why I started this bug report; I hoped that by saying "*may*
interpret", the user would understand that Emacs is basically saying
"this looks like a common footgun; I don't speak shell though so you
tell me".
> Maybe just telling what dired did not do would be better? Like
> "N occurrences of X will not be replaced with the file/file list -
> proceed?
That would be the most correct description of the situation. I didn't
go that way because the user might not be aware of this feature; I
failed to come up with a short prompt that would 1. explain the
substitution feature and 2. explain why Dired will not activate it here.
> Because, there are two things we mix up: (1) dired does not substitute
> with files, though the user might have wanted that, and (2) the char is
> send to the shell and is a wildcard there, so the result might also not
> be what the user intended. Do we want to warn about (1) or (2)? Both
> seems to be too much for one line of text.
Very much agreed.
> If we don't find a good wording maybe use something like
> `read-multiple-choice' or some other mechanism where the user is allowed
> to hit a help key (?, and that key should also be visible in the
> prompt). We can leave an explanation that doesn't have to fit into one
> line in the help text. I only mention `read-multiple-choice' because it
> provides all of that out of the box, of course there are alternative
> ways.
That sounds like a good compromise. I'll see what I can come up with.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 08 Aug 2019 21:23:02 GMT)
Full text and
rfc822 format available.
Message #142 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> First, apologies for taking so long to respond - I was AFK last week. I
> might not be very reactive these coming weeks either.
I use the substitution feature in dired-do-shell-command quite rarely,
but today I needed to use it, and it strikes as partly unusable
and confusing. There are several problems:
1. Answering "no" cancels the command. Instead of this,
it should proceed without substitution. There is a special key
`C-g' to cancel the command.
2. The current question is too ambiguous:
Confirm--do you mean to use ‘?’ as a wildcard? (y or n)
A wildcard can mean both dired substitution and shell substitution.
A better question should use the same terms as documented in the
docstring of `dired-do-shell-command', i.e. "marked files", "file list".
So a better question would be:
Confirm--do you mean to substitute ‘?’ with marked files? (y or n)
Or something similar that makes clear that substitution applies
to dired files, not files matched by shell.
3. I still can't be sure if after asking these question, dired still
does the right thing. I'd prefer to have an option to show the
final command before running it, exactly like `C-u M-x rgrep' does
with its `confirm' argument. Yes, its command line is quite long,
but this is not a problem: wrapped minibuffer content is less
problematic than multi-line prompts.
> What bothers me is that even if we can assert #2, nothing guarantees
> that these colors will be distinguishable *to the user* (who may
> e.g. have some form of color blindness). It would therefore be nice if
> this user could force Emacs to use ^ markers; I guess that would involve
> a new variable.
As was already discussed in this thread, using (:inherit '(warning underline))
will solve this problem and improve accessibility. So there will be
no need in multi-line prompt when using underline face attribute.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 09 Aug 2019 12:44:02 GMT)
Full text and
rfc822 format available.
Message #145 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> I use the substitution feature in dired-do-shell-command quite rarely,
> but today I needed to use it, and it strikes as partly unusable
> and confusing.
Welcome to the club :)
> 2. The current question is too ambiguous:
>
> Confirm--do you mean to use ‘?’ as a wildcard? (y or n)
>
> A wildcard can mean both dired substitution and shell substitution.
> A better question should use the same terms as documented in the
> docstring of `dired-do-shell-command', i.e. "marked files", "file list".
> So a better question would be:
>
> Confirm--do you mean to substitute ‘?’ with marked files? (y or n)
>
> Or something similar that makes clear that substitution applies
> to dired files, not files matched by shell.
Mmm, I think that the current prompt *does* use the same terms as
documented in the docstring: it simply mistakenly assumes that if '?'
and '*' are not "isolated", the shell will unconditionally process them
as wildcards. It is a heuristic that fails to consider that '?' and '*'
may be quoted or escaped.
So with this prompt, "yes" means "yes, I want the shell to (possibly)
substitute these characters", while "no" means "By Jove, what a silly
mistake I was about to make! Thank you ever so much for catching it
Dired old chap! Let me add some backquotes around this '?' so that you
can be sure I mean for you to substitute it for the marked files."
IIUC, your suggested prompt does not match what dired-do-shell-command
actually does: the function only ever substitutes '?' if it is
"isolated", i.e. surrounded with whitespace or backquotes. Cf. the
docstring:
> ‘*’ 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.
(Drew suggested that we may want to change *the code* to behave as your
prompt suggests[1], I sketched a possible way to let the user select
which occurrences to substitute[2], but did not act on it as AFAICT this
use-case is already handled by adding backquotes around '?')
>> What bothers me is that even if we can assert #2, nothing guarantees
>> that these colors will be distinguishable *to the user* (who may
>> e.g. have some form of color blindness). It would therefore be nice if
>> this user could force Emacs to use ^ markers; I guess that would involve
>> a new variable.
>
> As was already discussed in this thread, using (:inherit '(warning underline))
> will solve this problem and improve accessibility. So there will be
> no need in multi-line prompt when using underline face attribute.
Mmm. I went to a TTY to check how (:inherit '(underline)) looks. Since
(display-supports-face-attributes-p '(:underline t)) is nil there, the
"underline" face is defined as (:weight bold), which merely makes the
foreground color brighter. So (:inherit '(warning underline)) amounts
to just (:inherit '(warning)).
Perhaps (display-supports-face-attributes-p '(:underline t)) can be used
to decide whether we need to add ^ markers.
[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=35564#89
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28969#19
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 09 Aug 2019 18:11:01 GMT)
Full text and
rfc822 format available.
Message #148 received at 35564 <at> debbugs.gnu.org (full text, mbox):
>> 2. The current question is too ambiguous:
>>
>> Confirm--do you mean to use ‘?’ as a wildcard? (y or n)
>>
>> A wildcard can mean both dired substitution and shell substitution.
>> A better question should use the same terms as documented in the
>> docstring of `dired-do-shell-command', i.e. "marked files", "file list".
>> So a better question would be:
>>
>> Confirm--do you mean to substitute ‘?’ with marked files? (y or n)
>>
>> Or something similar that makes clear that substitution applies
>> to dired files, not files matched by shell.
>
> Mmm, I think that the current prompt *does* use the same terms as
> documented in the docstring: it simply mistakenly assumes that if '?'
> and '*' are not "isolated", the shell will unconditionally process them
> as wildcards. It is a heuristic that fails to consider that '?' and '*'
> may be quoted or escaped.
Do you mean this case: despite that the docstring of
`dired-do-shell-command' mentions "a shell wildcard",
typing on a file:
! cat '*'
and confirming with `y':
Confirm--do you mean to use ‘*’ as a wildcard? (y or n) y
still doesn't use * as "a shell wildcard". Then I agree.
>> As was already discussed in this thread, using (:inherit '(warning underline))
>> will solve this problem and improve accessibility. So there will be
>> no need in multi-line prompt when using underline face attribute.
>
> Mmm. I went to a TTY to check how (:inherit '(underline)) looks. Since
> (display-supports-face-attributes-p '(:underline t)) is nil there, the
> "underline" face is defined as (:weight bold), which merely makes the
> foreground color brighter. So (:inherit '(warning underline)) amounts
> to just (:inherit '(warning)).
>
> Perhaps (display-supports-face-attributes-p '(:underline t)) can be used
> to decide whether we need to add ^ markers.
You can check all possible face attributes to find the one
available on tty: underline, weight: bold, inverse-video, ...
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 15 Aug 2019 21:00:03 GMT)
Full text and
rfc822 format available.
Message #151 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> IIUC, your suggested prompt does not match what dired-do-shell-command
> actually does: the function only ever substitutes '?' if it is
> "isolated", i.e. surrounded with whitespace or backquotes. Cf. the
> docstring:
>
>> ‘*’ 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.
I collected a short summary that shows one case is still missing
(‘!’ means Dired prompt called on the marked file, and
‘$’ is the corresponding shell command):
1. ?
! cat ?
$ cat marked
! cat ./?
Confirm--do you mean to use ‘?’ as a wildcard? (y or n) y
$ cat ./? marked
! cat ?""
Confirm--do you mean to use ‘?’ as a wildcard? (y or n) y
$ cat ? marked
! cat '?'
Confirm--do you mean to use ‘?’ as a wildcard? (y or n) y
$ cat '?' marked
cat: '?': No such file or directory
! cat ./`?`
$ cat ./marked
2. *
! cat *
$ cat marked
! cat ./*
Confirm--do you mean to use ‘*’ as a wildcard? (y or n) y
$ cat ./* marked
! cat *""
Confirm--do you mean to use ‘*’ as a wildcard? (y or n) y
$ cat * marked
! cat '*'
Confirm--do you mean to use ‘*’ as a wildcard? (y or n) y
$ cat '*' marked
cat: '*': No such file or directory
Now the missing case - how to do the same that ‘cat ./`?`’ does,
i.e. how to substitute ‘*’ by marked files in such Dired prompt:
! cat ./`*`
Confirm--do you mean to use ‘*’ as a wildcard? (y or n) y
$ cat ./`*` marked
/bin/bash: marked: command not found
cat: ./: Is a directory
Why can't it run this shell command:
$ cat ./marked
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Mon, 19 Aug 2019 04:56:01 GMT)
Full text and
rfc822 format available.
Message #154 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> Now the missing case - how to do the same that ‘cat ./`?`’ does,
> i.e. how to substitute ‘*’ by marked files in such Dired prompt:
>
> ! cat ./`*`
> Confirm--do you mean to use ‘*’ as a wildcard? (y or n) y
> $ cat ./`*` marked
> /bin/bash: marked: command not found
> cat: ./: Is a directory
>
> Why can't it run this shell command:
>
> $ cat ./marked
AFAICT, because dired--star-or-qmark-p does not handle `*`:
> Isolated means that MATCH is surrounded by spaces or at the beginning/end
> of STRING followed/prefixed with an space. A match to ‘`?`’,
> isolated or not, is also valid.
I've skimmed the docstrings and comments for dired--star-or-qmark-p,
dired-shell-stuff-it and dired-do-shell-command, but I could not find a
rationale for not handling that case. git log -G'`\*`' hasn't finished
yet but so far it hasn't told me anything either.
If this is something we want[1], we can add it independently of this bug
report. If no-one has committed it (or created a new bug report for it)
by the time I get back to coding, I might throw in a patch for that in
the series; chances are it might simplify the code somewhat, since ? and
* will then be handled similarly.
Thanks for the survey Juri!
[1] I see no reason not to support it, since otherwise the shell
translates
`*`
into the command
first-file-according-to-locale other-files…
which doesn't strike me as very useful behaviour. Substituting `*`
for the file list, like we substitute `?` for each file, could make
sense, e.g. for
! some-command "`*`"
where some-command wants a space-separated file list as a single
argument (though I can't come up with an actual command off the top
of my head).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 10 Oct 2019 18:46:02 GMT)
Full text and
rfc822 format available.
Message #157 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Finally got around to try out rmc.el.
A brief recap of the issue: dired-do-shell-command looks out for any
non-isolated metacharacters[1], and prompts the user when it finds some.
The problem is that the prompt is downright misleading under some
circumstances. E.g. after marking some files in a Dired buffer:
! sed 's/?/!/g' RET
=> Confirm--do you mean to use `?' as a wildcard?
The answer a user must input to proceed is "yes", despite '?' not being
a wildcard in this situation; the answer some users may give intuitively
is "no" (or, in my case, "whaaa?").
This patch series initially tried to shove the command in the prompt,
highlight the non-isolated characters, and re-phrase the prompt to be
more accurate (i.e. not talk about wildcards).
It went through a several iterations for a few reasons[2]; most recently
Michael suggested using read-multiple-choice [bug#35564#136]; I looked
at how nsm.el uses it, saw that is was good, and got distracted for two
months.
Here is the new series:
[0001-Tweak-dired-warning-about-wildcard-characters.patch (text/x-patch, attachment)]
[0002-Dedup-dired-aux-isolated-char-searching-Bug-35564.patch (text/x-patch, attachment)]
[0003-Add-markers-below-non-isolated-chars-in-dired-prompt.patch (text/x-patch, attachment)]
[0004-Simplify-highlighting-assertions.patch (text/x-patch, attachment)]
[0005-Hide-detailed-explanations-in-a-togglable-help-buffe.patch (text/x-patch, attachment)]
[Message part 7 (text/plain, inline)]
Highlights:
- removed the patch for y-or-n-p, since we don't need it anymore,
- (squashed Noam's patch with my fixups,)
- the last patch contains the new stuff:
- the default prompt is now as concise as the old one,
- pressing 'd' toggles a help buffer which highlights occurrences
using the warning face,
- when the help buffer is enabled, pressing 'm' toggles the '^'
markers.
Squashed patch for convenience:
[0001-Tweak-dired-warning-about-wildcard-characters.patch (text/x-patch, attachment)]
[Message part 9 (text/plain, inline)]
To try the changes out, it's enough to reload dired-aux.el, mark a few
files in Dired, type e.g.
! sed 's/?/!/g' RET
… and play with the new prompt.
Let me know if this UI looks OK, and how the implementation may be
improved. Thank you for your patience.
Not addressed in this patch series:
- letting the user iterate over non-isolated occurrences and
selectively substitute them,
- allowing '*' to be substituted when surrounded by backquotes, just
like '?'.
I do find these features valuable (or at least worthy of discussion),
however the current bug reports were motivated merely by an inaccurate
warning; I'd like to close this first before considering further
changes.
[1] '?' when not surrounded by whitespace or backquotes,
'*' when not surrounded by whitespace.
[2] Trying to find the right balance between concision and accurate
explanation, considering that some users may not know about the
file-substitution feature; also trying to make the highlighting
"accessible", i.e. not just relying on colored faces.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Tue, 22 Oct 2019 15:12:02 GMT)
Full text and
rfc822 format available.
Message #160 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Casual and nonchalant bump.
I realize that everyone has their plate full right now (Emacs has tabs!
Face extension beyond EOL is customizable! What a time to be alive!),
so I am not expecting this to get any immediate attention. In case it
helps though, here is a comparison when running e.g. sed 's/?/!/':
Old prompt:
> Confirm--do you mean to use ‘?’ as a wildcard? (y or n)
New prompt based on read-multiple-choice:
> Warning: 1 occurrence of ‘?’ will not be substituted. Proceed?
> ([Y]es, [N]o, toggle [D]etails, [?]):
Hitting 'd' pops a buffer showing more information[1]. The commands
then become:
> ([Y]es, [N]o, toggle [D]etails, toggle [M]arkers, [?])
Hitting 'm' shows/hides '^' markers below the occurrences; 'd' quits the
details window.
Screenshots in GUI session:
[gui-basic.png (image/png, attachment)]
[gui-details.png (image/png, attachment)]
[gui-markers.png (image/png, attachment)]
[Message part 5 (text/plain, inline)]
Screenshots in TTY:
[tty-basic.png (image/png, attachment)]
[tty-details.png (image/png, attachment)]
[tty-markers.png (image/png, attachment)]
[Message part 9 (text/plain, inline)]
The patch(es) can be found in my previous message[2] (along with some
context and rationale). Let me know if there is anything I can do to
help with the review.
Thank you for your time.
[1] Contents of the details buffer:
> If your command contains occurrences of ‘*’ surrounded by
> whitespace, ‘dired-do-shell-command’ substitutes them for the
> entire file list to process. Otherwise, if your command contains
> occurrences of ‘?’ surrounded by whitespace or ‘`’, Dired will
> run the command once for each file, substituting ‘?’ for each
> file name.
>
> Your command contains occurrences of ‘?’ that will not be
> substituted, and will be passed through normally to the shell.
>
> sed 's/?/!/'
'?' is highlighted with the warning face.
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=35564#157
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Tue, 22 Oct 2019 17:13:01 GMT)
Full text and
rfc822 format available.
Message #163 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> Casual and nonchalant bump.
>
> I realize that everyone has their plate full right now (Emacs has tabs!
> Face extension beyond EOL is customizable! What a time to be alive!)
Yeah, indeed. Looking at your patch is at my list - will have a look
soon. Thanks for your summary, that is very kind and useful.
Without having had a look yet - your last version addresses everything
brought up so far and can be considered final, right?
Thanks,
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Tue, 22 Oct 2019 20:46:03 GMT)
Full text and
rfc822 format available.
Message #166 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> I realize that everyone has their plate full right now (Emacs has tabs!
> Face extension beyond EOL is customizable! What a time to be alive!),
> so I am not expecting this to get any immediate attention.
Sorry for leaving your new mail unanswered for so long
(although I marked your mail with the "read-later" tag :)
> New prompt based on read-multiple-choice:
>
>> Warning: 1 occurrence of ‘?’ will not be substituted. Proceed?
>> ([Y]es, [N]o, toggle [D]etails, [?]):
>
> Hitting 'd' pops a buffer showing more information[1]. The commands
> then become:
>
>> ([Y]es, [N]o, toggle [D]etails, toggle [M]arkers, [?])
>
> Hitting 'm' shows/hides '^' markers below the occurrences; 'd' quits the
> details window.
Finally we have the best solution where the prompt is concise and
fits into one line, still allowing to show more information on demand,
thanks for that.
Some minor details that I still don't understand:
1. Why there is the verbose option “toggle [D]etails”
while just “[?]” should be enough. For example, like
in query-replace typing ‘?’ displays the Help window,
just typing ‘?’ here could display the Dired Help.
2. Would it be possible to customize the prompt to accept
short answers “y, n” instead of long answers “Yes, No”?
When ‘read-answer-short’ is ‘auto’ by default,
the behaviour of ‘read-answer’ depends on whether
`yes-or-no-p' is set to `y-or-n-p'. In this case
‘read-answer’ accepts short single-key answers.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Tue, 22 Oct 2019 21:12:01 GMT)
Full text and
rfc822 format available.
Message #169 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> 1. Why there is the verbose option “toggle [D]etails”
> while just “[?]” should be enough. For example, like
> in query-replace typing ‘?’ displays the Help window,
> just typing ‘?’ here could display the Dired Help.
'?' is an automatic option added by read-multiple-choice. It pops-up an
autogenerated buffer repeating the prompt and showing longer
descriptions for each option.
I would have liked '?' to show what I ended up putting in the "details"
buffer; ideally read-multiple-choice would have an optional HELP-MESSAGE
argument that could be squeezed between the prompt and the key
description when showing the help buffer.
Since that's not how things work, I went for the additional option
('d').
> 2. Would it be possible to customize the prompt to accept
> short answers “y, n” instead of long answers “Yes, No”?
> When ‘read-answer-short’ is ‘auto’ by default,
> the behaviour of ‘read-answer’ depends on whether
> `yes-or-no-p' is set to `y-or-n-p'. In this case
> ‘read-answer’ accepts short single-key answers.
I didn't know about read-answer, that was an interesting discovery.
read-multiple-choice only accepts single-key answers, you cannot answer
with long options if I'm not mistaken. You cannot choose what the
prompt looks like either; you simply provide a list of (KEY NAME
[DESCRIPTION]) tuples; NAMEs will be shown in the prompt, DESCRIPTIONs
will be shown in the auto-generated help buffer.
(Let me know if that does not answer your question; I might have
misunderstood.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Tue, 22 Oct 2019 21:33:02 GMT)
Full text and
rfc822 format available.
Message #172 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Without having had a look yet - your last version addresses everything
> brought up so far and can be considered final, right?
I think so. Going over for bug#28969 and bug#35564, here are the
discussion points I could identify:
- y-or-n-p propertizing its prompt rigidly: out of scope, since we use
read-multiple-choice now.
- The prompt getting too long: it's now much shorter than the
four(!)-line version I came up with in v4; it concisely spells out the
issue (some characters will not be substituted) and invites the user
to ask for more details if needed.
- Asking the user whether they'd like to actually substitute these
characters: out of scope; not sure it's necessary, since the new
"details" buffer explains how to work around this for '?' (using
backquotes).
(Though no such workaround exists for '*'. Allowing '*' to be
isolated with backquotes just like '?' would be a natural thing to do
IMO, but that's unrelated to fixing this confusing prompt.)
- Ensuring accessibility: users who cannot distinguish the 'warning'
face are now invited to add optional '^' markers.
- Preventing '`' being linked to the backquote macro in the docstring
for dired-do-shell-command: still no idea how to fix that, but that
can be investigated independently.
Here are some remaining issues I can think of:
- The code that toggles the '^' markers does not check that the command
is not wrapped/truncated (i.e. that the window is wide enough).
- The details window might not be tall enough, in which case maybe I
should add [f]orward-/[b]ackward-page actions like nsm.el does.
- In dired--no-subst-confirm, I did my best to make the window-popping
dance as graceful as possible (unwind-protect so that the details
buffer is killed even after C-g, save-window-excursion to restore the
window configuration…), but maybe it falls apart in cases I haven't
considered.
- UI bikeshedding: maybe drop the leading "Warning:", add the underline
face to the mix…
- Code quality: some of the small functions I wrote exist for no other
reason than I found the resulting code to be easier to follow; they
could probably be inlined if others do not share my preferences.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sun, 27 Oct 2019 22:04:02 GMT)
Full text and
rfc822 format available.
Message #175 received at 35564 <at> debbugs.gnu.org (full text, mbox):
>> 1. Why there is the verbose option “toggle [D]etails”
>> while just “[?]” should be enough. For example, like
>> in query-replace typing ‘?’ displays the Help window,
>> just typing ‘?’ here could display the Dired Help.
>
> '?' is an automatic option added by read-multiple-choice. It pops-up an
> autogenerated buffer repeating the prompt and showing longer
> descriptions for each option.
>
> I would have liked '?' to show what I ended up putting in the "details"
> buffer; ideally read-multiple-choice would have an optional HELP-MESSAGE
> argument that could be squeezed between the prompt and the key
> description when showing the help buffer.
'?' would be more preferable since this is the standard way to ask
for additional information in Dired, for example, on error it shows:
Dired error--type ? for details
where '?' shows the details. dired-do-shell-command should do the same
in its prompt. read-multiple-choice could be changed to not add
its own help option when a '?' is provided in its 'choices' arg.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 30 Oct 2019 22:56:02 GMT)
Full text and
rfc822 format available.
Message #178 received at 35564 <at> debbugs.gnu.org (full text, mbox):
>>> 1. Why there is the verbose option “toggle [D]etails”
>>> while just “[?]” should be enough. For example, like
>>> in query-replace typing ‘?’ displays the Help window,
>>> just typing ‘?’ here could display the Dired Help.
>>
>> '?' is an automatic option added by read-multiple-choice. It pops-up an
>> autogenerated buffer repeating the prompt and showing longer
>> descriptions for each option.
>>
>> I would have liked '?' to show what I ended up putting in the "details"
>> buffer; ideally read-multiple-choice would have an optional HELP-MESSAGE
>> argument that could be squeezed between the prompt and the key
>> description when showing the help buffer.
>
> '?' would be more preferable since this is the standard way to ask
> for additional information in Dired, for example, on error it shows:
>
> Dired error--type ? for details
>
> where '?' shows the details. dired-do-shell-command should do the same
> in its prompt. read-multiple-choice could be changed to not add
> its own help option when a '?' is provided in its 'choices' arg.
I meant using the same logic as in 'read-answer':
(if (assoc "help" answers)
answers
(append answers '(("help" ?? "show this help message"))))
i.e. if '?' is provided in the function argument then use it,
otherwise use the default value.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Mon, 04 Nov 2019 06:38:01 GMT)
Full text and
rfc822 format available.
Message #181 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
>> '?' would be more preferable since this is the standard way to ask
>> for additional information in Dired, for example, on error it shows:
>>
>> Dired error--type ? for details
>>
>> where '?' shows the details. dired-do-shell-command should do the same
>> in its prompt. read-multiple-choice could be changed to not add
>> its own help option when a '?' is provided in its 'choices' arg.
>
> I meant using the same logic as in 'read-answer':
>
> (if (assoc "help" answers)
> answers
> (append answers '(("help" ?? "show this help message"))))
>
> i.e. if '?' is provided in the function argument then use it,
> otherwise use the default value.
I'd like to keep read-multiple-choice's built-in help action available
though, I find it quite ergonomic. Plus, the fact that it is bound to ?
and C-h makes it consistent with help-for-help, another command whose
purpose is to describe prompt bindings in more details.
Here are the ways forward I can see:
1. keep 'd' for "details" (a la nsm.el),
2. use 'h' for "help": more intuitive for "what the ftok is going on"
reactions, though perhaps confusing when shown alongside '?',
3. teach read-multiple-choice to let go of '?', and…
3.1. that's it; give up on the bindings-explaining help buffer,
3.2. move the code generating this buffer to an external function,
which callers could re-use to fill in their own help buffer,
4. add a third, optional argument to read-multiple-choice
(e.g. help-text) that would be added to the help buffer
(e.g. squeezed between the prompt and the bindings).
Ranked according to my preference:
1. (keeping "help" mnemonics (?, C-h) for "what can I do" actions, and
"details/description/debug" mnemonics for "give more context"
actions)
4. (reduces the number of actions for this specific prompt)
3.2. (same, only more involved)
3.1. (sad to give up on the bindings description)
2. (see rationale for 1; plus 'h' and '?' doing different things might
be confusing)
Tell me if there are other solutions I missed, or if you find any of
these satisfying!
Thank you for your time, and for the review.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Tue, 05 Nov 2019 22:59:02 GMT)
Full text and
rfc822 format available.
Message #184 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> I'd like to keep read-multiple-choice's built-in help action available
> though, I find it quite ergonomic. Plus, the fact that it is bound to ?
> and C-h makes it consistent with help-for-help, another command whose
> purpose is to describe prompt bindings in more details.
>
> Here are the ways forward I can see:
>
> 1. keep 'd' for "details" (a la nsm.el),
>
> 2. use 'h' for "help": more intuitive for "what the ftok is going on"
> reactions, though perhaps confusing when shown alongside '?',
Indeed, providing several help keys is confusing.
It should be less confusing to provide only '?'
and to teach read-multiple-choice to use our help text
where we could describe each supported key (y/n/etc.)
and their effect explicitly.
> 3. teach read-multiple-choice to let go of '?', and…
>
> 3.1. that's it; give up on the bindings-explaining help buffer,
>
> 3.2. move the code generating this buffer to an external function,
> which callers could re-use to fill in their own help buffer,
It seems this read-multiple-choice needs to be rewritten anyway
to use the minibuffer instead of read-event. So the ability
to override '?' could be added at the same time as well.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 07 Nov 2019 22:59:05 GMT)
Full text and
rfc822 format available.
Message #187 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> It seems this read-multiple-choice needs to be rewritten anyway
> to use the minibuffer instead of read-event. So the ability
> to override '?' could be added at the same time as well.
A patch in bug#38076 replaces read-char-choice with
read-char-from-minibuffer, and in one place in the patch
it uses read-char-from-minibuffer in files--ask-user-about-large-file
simply as
(read-char-from-minibuffer
(concat prompt " (y)es or (n)o or (l)iterally ")
'(?y ?Y ?n ?N ?l ?L))
Maybe it would be much simpler to use something like
(read-char-from-minibuffer
(concat "1 occurrence of ‘?’ will not be substituted. Proceed? (y)es or (n)o or (h)elp ")
'(?y ?Y ?n ?N ?h ?H))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sun, 10 Nov 2019 20:56:01 GMT)
Full text and
rfc822 format available.
Message #190 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> A patch in bug#38076 replaces read-char-choice with
> read-char-from-minibuffer, and in one place in the patch
> it uses read-char-from-minibuffer in files--ask-user-about-large-file
> simply as
>
> (read-char-from-minibuffer
> (concat prompt " (y)es or (n)o or (l)iterally ")
> '(?y ?Y ?n ?N ?l ?L))
>
> Maybe it would be much simpler to use something like
>
> (read-char-from-minibuffer
> (concat "1 occurrence of ‘?’ will not be substituted. Proceed? (y)es or (n)o or (h)elp ")
> '(?y ?Y ?n ?N ?h ?H))
A better example is ask-user-about-supersession-threat in userlock.el
that using read-char-from-minibuffer displays such prompt:
file changed on disk; really edit the buffer? (y, n, r or C-h)
Please use a similarly short prompt:
1 occurrence of ‘?’ will not be substituted. Proceed? (y, n or ?)
where '?' will show the Dired help window.
This will simplify the user interface to make it less confusing
(no more choice for two separate help texts).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sun, 10 Nov 2019 20:57:02 GMT)
Full text and
rfc822 format available.
Message #193 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> Going over for bug#28969 and bug#35564, here are the
> discussion points I could identify:
It seems all these points are resolved now:
> - y-or-n-p propertizing its prompt rigidly: out of scope, since we use
> read-multiple-choice now.
y-or-n-p doesn't propertize its prompt rigidly now, but indeed this is
out of scope when using read-char-from-minibuffer.
> - The prompt getting too long: it's now much shorter than the
> four(!)-line version I came up with in v4; it concisely spells out the
> issue (some characters will not be substituted) and invites the user
> to ask for more details if needed.
Please use a shorter prompt like
1 occurrence of ‘?’ will not be substituted. Proceed? (y, n or ?)
> - Asking the user whether they'd like to actually substitute these
> characters: out of scope; not sure it's necessary, since the new
> "details" buffer explains how to work around this for '?' (using
> backquotes).
Displaying the new "Dired help" buffer on demand is a good idea.
> - Ensuring accessibility: users who cannot distinguish the 'warning'
> face are now invited to add optional '^' markers.
In the new "Dired help" buffer where there is enough space to add
the command line with '^' markers.
> - Preventing '`' being linked to the backquote macro in the docstring
> for dired-do-shell-command: still no idea how to fix that, but that
> can be investigated independently.
Maybe use double quotes "`" as an exception.
> Here are some remaining issues I can think of:
>
> - The code that toggles the '^' markers does not check that the command
> is not wrapped/truncated (i.e. that the window is wide enough).
Everything should be explained in the new "Dired help" buffer
including the command line with '^' markers.
> - The details window might not be tall enough, in which case maybe I
> should add [f]orward-/[b]ackward-page actions like nsm.el does.
Yesterday I added new keybindings C-v/M-v for scrolling the original
window from the minibuffer.
> - In dired--no-subst-confirm, I did my best to make the window-popping
> dance as graceful as possible (unwind-protect so that the details
> buffer is killed even after C-g, save-window-excursion to restore the
> window configuration…), but maybe it falls apart in cases I haven't
> considered.
Maybe some of the standard display-window functions can handle this,
but this is a minor question.
> - UI bikeshedding: maybe drop the leading "Warning:"
Right, this makes the prompt shorter.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 14 Nov 2019 07:04:02 GMT)
Full text and
rfc822 format available.
Message #196 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Thanks for your input Juri. I see that your work on
read-char-from-minibuffer has been pushed to master; I'll start working
on a v6 using that function instead of read-multiple-choice ASAP.
Juri Linkov <juri <at> linkov.net> writes:
> Please use a shorter prompt like
>
> 1 occurrence of ‘?’ will not be substituted. Proceed? (y, n or ?)
Will do.
> In the new "Dired help" buffer where there is enough space to add
> the command line with '^' markers.
Note that the current implementation is quite naive: it falls apart if
the command is wider than the help window. E.g.:
some-command "first?argument" "sec⤸
⤹ond?argument" "final-argument"
^ ⤸
⤹ ^
(⤸ and ⤹ represent fringe indicators for wrapped lines.)
I don't know how important it is to handle this situation, since
read-char-from-minibuffer allows C-x o'ing to the help buffer and
toggling truncated lines.
>> - Preventing '`' being linked to the backquote macro in the docstring
>> for dired-do-shell-command: still no idea how to fix that, but that
>> can be investigated independently.
>
> Maybe use double quotes "`" as an exception.
Maybe. The docstring single-quotes every other character it mentions
though (?, *, &, ;), so that would look sort of inconsistent.
Note that this problem also impacts other docstrings[1].
>> - The details window might not be tall enough, in which case maybe I
>> should add [f]orward-/[b]ackward-page actions like nsm.el does.
>
> Yesterday I added new keybindings C-v/M-v for scrolling the original
> window from the minibuffer.
Nice!
(Out of curiosity, would it make sense to also bind C-x < and C-x >?)
>> - UI bikeshedding: maybe drop the leading "Warning:"
>
> Right, this makes the prompt shorter.
Will do then.
Again, thank you for your time.
[1]
subr.el:372:like `%', `\\=`' and `\\='', use (error \"%s\" MESSAGE).
subr.el:388:like `%', `\\=`' and `\\='', use (error \"%s\" MESSAGE).
emulation/viper-util.el:1173:symbols like `\\=`', `\\='', `:', `\"', `)', and `{' are excluded.
leim/quail/cyrillic.el:1362:`]', `\\', `\\=`' and `[' keys respectively, Caps Lock does not affect them."
leim/quail/hebrew.el:116: `\\=`' is used to switch levels instead of Alt-Gr.
leim/quail/hebrew.el:606: `\\=`' is used to switch levels instead of Alt-Gr.
leim/quail/thai.el:50: `ฃ' and `ฅ' are assigned to `\\=`' and `~' respectively,
Note also that there are places where this works as intended:
emacs-lisp/backquote.el:253: "See `\\=`' (also `pcase') for the usage of `,'.")
emacs-lisp/backquote.el:257: "See `\\=`' for the usage of `,@'.")
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 16 Nov 2019 21:08:02 GMT)
Full text and
rfc822 format available.
Message #199 received at 35564 <at> debbugs.gnu.org (full text, mbox):
>> In the new "Dired help" buffer where there is enough space to add
>> the command line with '^' markers.
>
> Note that the current implementation is quite naive: it falls apart if
> the command is wider than the help window. E.g.:
>
> some-command "first?argument" "sec⤸
> ⤹ond?argument" "final-argument"
> ^ ⤸
> ⤹ ^
>
> (⤸ and ⤹ represent fringe indicators for wrapped lines.)
>
> I don't know how important it is to handle this situation, since
> read-char-from-minibuffer allows C-x o'ing to the help buffer and
> toggling truncated lines.
Or truncated lines could be enabled by default in the "Dired help"
buffer by using (setq truncate-lines nil)
>>> - The details window might not be tall enough, in which case maybe I
>>> should add [f]orward-/[b]ackward-page actions like nsm.el does.
>>
>> Yesterday I added new keybindings C-v/M-v for scrolling the original
>> window from the minibuffer.
>
> Nice!
> (Out of curiosity, would it make sense to also bind C-x < and C-x >?)
Wouldn't ‘C-x <’ and ‘C-x >’ be more needed to scroll the minibuffer
horizontally? I tried to toggle truncated lines in the minibuffer,
and then ‘C-x <’ and ‘C-x >’ scroll the minibuffer horizontally.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Wed, 18 Dec 2019 07:12:01 GMT)
Full text and
rfc822 format available.
Message #202 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello,
Here is a new revision of this patch series, which aims to rephrase
dired-do-shell-command's warning about occurrences of '?' and '*' that
will not be substituted for filenames, with the following goals in mind:
1. cease to call these characters "wildcards" since they may be quoted
or escaped,
2. cater to users who do not know about the substitution feature,
3. keep the default prompt as concise as the current one.
The first revisions[1][2][3][4] focused on goals 1 and 2, to the
detriment of 3. The last revision[5] hid the verbosity behind an
optional explanatory buffer, using read-multiple-choice. Since this
function already generates a help buffer bound to '?', I bound the
explanatory buffer to 'd' for "details", a la nsm-query-user.
Juri suggested[6] that read-char-from-minibuffer might fit the bill
better. Since I feel kind of torn between these options, I'm putting
them both forward.
First, the scaffolding patches (same as v5):
[0001-Tweak-dired-warning-about-wildcard-characters.patch (text/x-patch, attachment)]
[0002-Dedup-dired-aux-isolated-char-searching-Bug-35564.patch (text/x-patch, attachment)]
[0003-Add-markers-below-non-isolated-chars-in-dired-prompt.patch (text/x-patch, attachment)]
[0004-Simplify-highlighting-assertions.patch (text/x-patch, attachment)]
[Message part 6 (text/plain, inline)]
Then, the patch adding read-multiple-choice (mostly the same as v5):
[0005-Hide-detailed-explanations-in-a-togglable-help-buffe-rmc.patch (text/x-patch, attachment)]
[Message part 8 (text/plain, inline)]
Or, the patch adding read-char-from-minibuffer:
[0005-Hide-detailed-explanations-in-a-togglable-help-buffe-rcfm.patch (text/x-patch, attachment)]
[Message part 10 (text/plain, inline)]
Squashed patches:
[0001-Tweak-dired-warning-about-wildcard-characters-rmc-squashed.patch (text/x-patch, attachment)]
[0001-Tweak-dired-warning-about-wildcard-characters-rcfm-squashed.patch (text/x-patch, attachment)]
[Message part 13 (text/plain, inline)]
For reference, here is the diff between both the read-multiple-choice
and the read-char-from-minibuffer options:
[rmc-vs-rcfm.patch (text/x-patch, attachment)]
[Message part 15 (text/plain, inline)]
Once applied, the patches can be tried out by
- opening a Dired buffer,
- hitting '!',
- inputting e.g. "sed 's/?/!/'".
WDYT?
Thank you for your time.
[1] bug#35564#5
[2] bug#35564#38
[3] bug#35564#62
[4] bug#35564#101
[5] bug#35564#157
[6] bug#35564#187
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Thu, 19 Dec 2019 22:21:01 GMT)
Full text and
rfc822 format available.
Message #205 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> Once applied, the patches can be tried out by
>
> - opening a Dired buffer,
> - hitting '!',
> - inputting e.g. "sed 's/?/!/'".
>
> WDYT?
Thanks. I tried out your patch with read-char-from-minibuffer
and it works smoothly without any problem. Please ask Eli
for a permission to push it before starting the pretest.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 20 Dec 2019 08:54:02 GMT)
Full text and
rfc822 format available.
Message #208 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> From: Juri Linkov <juri <at> linkov.net>
> Cc: 35564 <at> debbugs.gnu.org, Michael Heerdegen <michael_heerdegen <at> web.de>,
> Noam Postavsky <npostavs <at> gmail.com>, Stefan Monnier
> <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>, Drew Adams
> <drew.adams <at> oracle.com>
> Date: Fri, 20 Dec 2019 00:01:03 +0200
>
> > Once applied, the patches can be tried out by
> >
> > - opening a Dired buffer,
> > - hitting '!',
> > - inputting e.g. "sed 's/?/!/'".
> >
> > WDYT?
>
> Thanks. I tried out your patch with read-char-from-minibuffer
> and it works smoothly without any problem. Please ask Eli
> for a permission to push it before starting the pretest.
I'll let other participants of this long discussion to chime in, but
in general I'd like to postpone this till after the emacs-27 branch is
cut (hopefully, very soon), as this constitutes a significant behavior
change, AFAIU.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 20 Dec 2019 20:35:01 GMT)
Full text and
rfc822 format available.
Message #211 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
> I'll let other participants of this long discussion to chime in, but
> in general I'd like to postpone this till after the emacs-27 branch is
> cut (hopefully, very soon), as this constitutes a significant behavior
> change, AFAIU.
I won't insist too much for this to land on Emacs 27, since a) the "bug"
it fixes is fairly minor, b) I know everyone's plate is quite full, and
c) I mostly use the master branch anyway, so it's not like I'll have to
wait to benefit.
For the record though, I'll point out a few reasons why I think it
should be "safe" to include this in the upcoming release:
- the changes are fairly limited in scope: they only affect
dired-do-shell-command;
- in the simplest case, the UI change is minor: it turns this message:
> Confirm--do you mean to use ‘?’ as a wildcard? (y or n)
into this one:
> 1 occurrence of ‘?’ will not be substituted. Proceed? (y, n, ?) ?
(or, with read-multiple-choice:)
> 1 occurrence of ‘?’ will not be substituted. Proceed? (_y_es, _n_o, toggle _d_etails, _?_):
- the "riskiest" refactoring changes have been handled by Noam[1] and
are partially covered by unit tests.
Most of the lengthy discussion was about finding the right balance
between message correctness and verbosity; hopefully the eventual
behavior change is not that significant.
> Thanks.
Thank you for your time.
[1] https://debbugs.gnu.org/cgi/bugreport.cgi?att=2;filename=0002-Dedup-dired-aux-isolated-char-searching-Bug-35564.patch;bug=35564;msg=202
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Fri, 20 Dec 2019 20:45:01 GMT)
Full text and
rfc822 format available.
Message #214 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> Thanks. I tried out your patch with read-char-from-minibuffer
> and it works smoothly without any problem. Please ask Eli
> for a permission to push it before starting the pretest.
Glad to hear it!
Like Eli, I'd like to collect some feedback, e.g. from Michael who
submitted bug#28969 and suggested read-multiple-choice.
Some additional remarks:
- I do not have push access AFAIK, so feel free to install whenever you
think there is consensus on whether to wait for emacs-27 to fork off.
- I guess there is also the question of whether to push the five patches
or the squashed one (all have valid commit messages, unless I messed
up).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 21 Dec 2019 07:09:02 GMT)
Full text and
rfc822 format available.
Message #217 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: Juri Linkov <juri <at> linkov.net>, 35564 <at> debbugs.gnu.org,
> michael_heerdegen <at> web.de, npostavs <at> gmail.com, monnier <at> iro.umontreal.ca,
> drew.adams <at> oracle.com
> Date: Fri, 20 Dec 2019 21:34:07 +0100
>
> - in the simplest case, the UI change is minor: it turns this message:
>
> > Confirm--do you mean to use ‘?’ as a wildcard? (y or n)
>
> into this one:
>
> > 1 occurrence of ‘?’ will not be substituted. Proceed? (y, n, ?) ?
>
> (or, with read-multiple-choice:)
>
> > 1 occurrence of ‘?’ will not be substituted. Proceed? (_y_es, _n_o, toggle _d_etails, _?_):
Is this the best wording you've been able to arrive at? It sounds
slightly confusing to me (but then I don't use this facility too
much). The confusing part is that it talks about "substitution", and
the user might not be aware that there is any substitution going on.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sat, 21 Dec 2019 07:09:02 GMT)
Full text and
rfc822 format available.
Message #220 received at 35564 <at> debbugs.gnu.org (full text, mbox):
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: 35564 <at> debbugs.gnu.org, Michael Heerdegen <michael_heerdegen <at> web.de>,
> Noam Postavsky <npostavs <at> gmail.com>, Stefan Monnier
> <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>, Drew Adams
> <drew.adams <at> oracle.com>
> Date: Fri, 20 Dec 2019 21:43:59 +0100
>
> - I guess there is also the question of whether to push the five patches
> or the squashed one (all have valid commit messages, unless I messed
> up).
The latter, please.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sun, 22 Dec 2019 16:03:01 GMT)
Full text and
rfc822 format available.
Message #223 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Cc: Juri Linkov <juri <at> linkov.net>, 35564 <at> debbugs.gnu.org,
>> michael_heerdegen <at> web.de, npostavs <at> gmail.com, monnier <at> iro.umontreal.ca,
>> drew.adams <at> oracle.com
>> Date: Fri, 20 Dec 2019 21:34:07 +0100
>>
>> - in the simplest case, the UI change is minor: it turns this message:
>>
>> > Confirm--do you mean to use ‘?’ as a wildcard? (y or n)
>>
>> into this one:
>>
>> > 1 occurrence of ‘?’ will not be substituted. Proceed? (y, n, ?) ?
>>
>> (or, with read-multiple-choice:)
>>
>> > 1 occurrence of ‘?’ will not be substituted. Proceed? (_y_es, _n_o, toggle _d_etails, _?_):
>
> Is this the best wording you've been able to arrive at? It sounds
> slightly confusing to me (but then I don't use this facility too
> much). The confusing part is that it talks about "substitution", and
> the user might not be aware that there is any substitution going on.
Indeed, that's why the prompt now supports an additional action to pop a
help buffer explaining what the deal is[1].
Fundamentally, this prompt *is* about the substitution feature. Dired
detects non-isolated occurrences of '*' and '?', and requests
confirmation before proceeding without substituting them. With this in
mind, explicitly mentioning "substitution" doesn't sound too outlandish…
(
Or, we could assume that the current message is correct (i.e. it's the
wildcards we want to warn about) but the condition that triggers it is
wrong, i.e. Dired should be smarter and only warn when the characters
are unquoted and unescaped. That sounds complex to implement though.
As Drew noted[2], another way to handle this would be asking users
whether they want to substitute the non-isolated characters. That
still implies talking about "substitution" though. Also, users can
already mark occurrences of '?' for substitution using backquotes, as
explained in the new help buffer.
)
To wrap up, I'd say that the current message makes me go:
"Huh? No, I don't want to use these characters as wildcards."
*hits "n", command is aborted*
*confusion: extreme & growing*
I'm hoping that the new message will have users go:
"Huh? Why would Dired substitute these characters?"
*hits "?", skims, hits "y", moves on*
*confusion: mild & receding*
Thank you for your patience, and for reviewing this.
[1]
> If your command contains occurrences of ‘*’ surrounded by
> whitespace, ‘dired-do-shell-command’ substitutes them for the
> entire file list to process. Otherwise, if your command contains
> occurrences of ‘?’ surrounded by whitespace or ‘`’, Dired will
> run the command once for each file, substituting ‘?’ for each
> file name.
>
> Your command contains occurrences of ‘?’ that will not be
> substituted, and will be passed through normally to the shell.
>
> sed 's/?/!/'
>
> (Press ^ to add markers below these occurrences.)
[2] bug#35564#83
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sun, 20 Sep 2020 11:44:02 GMT)
Full text and
rfc822 format available.
Message #226 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> - I guess there is also the question of whether to push the five patches
>> or the squashed one (all have valid commit messages, unless I messed
>> up).
>
> The latter, please.
Kévin, skimming this thread, there seemed to be a general consensus that
your patch series should be applied. Could you work up a single
squashed patch to apply? There's so many patches with different
variations that it's difficult to tell which ones should be applied.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Added tag(s) moreinfo.
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org.
(Sun, 20 Sep 2020 11:44:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sun, 20 Sep 2020 12:05:01 GMT)
Full text and
rfc822 format available.
Message #231 received at 35564 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Kévin, skimming this thread, there seemed to be a general consensus that
> your patch series should be applied. Could you work up a single
> squashed patch to apply? There's so many patches with different
> variations that it's difficult to tell which ones should be applied.
Thanks for following up on this! I was planning to do so after the
release of 27.1, but somehow never found the time.
Here is the squashed patch using read-char-from-minibuffer (which unless
I messed something up should be the same as the one from December[1]).
[0001-Tweak-dired-warning-about-wildcard-characters.patch (text/x-diff, attachment)]
[Message part 3 (text/plain, inline)]
[1] <https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Tweak-dired-warning-about-wildcard-characters-rcfm-squashed.patch;att=11;msg=202;bug=35564>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35564; Package
emacs.
(Sun, 20 Sep 2020 12:19:02 GMT)
Full text and
rfc822 format available.
Message #234 received at 35564 <at> debbugs.gnu.org (full text, mbox):
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> Here is the squashed patch using read-char-from-minibuffer (which unless
> I messed something up should be the same as the one from December[1]).
Thanks for the speedy squash. :-) I've now applied it to Emacs 28
after some extremely light testing.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Added tag(s) fixed.
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org.
(Sun, 20 Sep 2020 12:19:02 GMT)
Full text and
rfc822 format available.
bug marked as fixed in version 28.1, send any further explanations to
35564 <at> debbugs.gnu.org and Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org.
(Sun, 20 Sep 2020 12:19:03 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Mon, 19 Oct 2020 11:24:07 GMT)
Full text and
rfc822 format available.
This bug report was last modified 4 years and 47 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.