zsh-workers
 help / color / Atom feed
* Non-escaped special chars from var expansion
@ 2018-11-02 20:39 Wayne Davison
  2018-11-02 22:06 ` Peter Stephenson
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Wayne Davison @ 2018-11-02 20:39 UTC (permalink / raw)
  To: Zsh list

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

Hi, folks! Long time no see...

I have a particular compinit setup where special-chars from a var (other
than spaces) are not backslash-escaped.  To duplicate:

zsh -f
autoload -Uz compinit
compinit
zstyle ':completion:*' completer _oldlist _expand _complete _match _ignored
_files _prefix
zstyle ':completion:*:expand:*' tag-order all-expansions
bindkey -e
bindkey '\t' complete-word
mkdir 'foo; bar (baz)'
cd !:1
cd $PWD<tab>

The result is "cd foo;\ bar\ (baz)" instead of "cd foo\;\ bar\ \(baz\)". If
the <tab> instead executes expand-or-complete rather than complete-word
then it is expanded correctly, but I like to avoid var expansion in a path
unless I'm expanding the var itself (e.g. I don't want "vim
$OLDPWD/fname<TAB>" to expand $OLDPWD since I'm not tabbing right next to
the var).

The bug has been around for ages, and is still in the current git source.
I'm guessing that the _expand script needs a tweak, but I didn't see what
might be wrong after a quick glance.

..wayne..

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Non-escaped special chars from var expansion
  2018-11-02 20:39 Non-escaped special chars from var expansion Wayne Davison
@ 2018-11-02 22:06 ` Peter Stephenson
  2018-11-03 19:49   ` Peter Stephenson
  2018-11-02 23:21 ` Wayne Davison
  2019-11-17  6:00 ` Wayne Davison
  2 siblings, 1 reply; 5+ messages in thread
From: Peter Stephenson @ 2018-11-02 22:06 UTC (permalink / raw)
  To: Zsh list

On Fri, 2 Nov 2018 13:39:26 -0700
Wayne Davison <wayne@opencoder.net> wrote:
> I have a particular compinit setup where special-chars from a var (other
> than spaces) are not backslash-escaped.  To duplicate:
> 
> zsh -f
> autoload -Uz compinit
> compinit
> zstyle ':completion:*' completer _oldlist _expand _complete _match _ignored
> _files _prefix
> zstyle ':completion:*:expand:*' tag-order all-expansions
> bindkey -e
> bindkey '\t' complete-word
> mkdir 'foo; bar (baz)'
> cd !:1
> cd $PWD<tab>
> 
> The result is "cd foo;\ bar\ (baz)" instead of "cd foo\;\ bar\ \(baz\)".

Don't breathe too loudly but I *think* I might have worked out what's
going on.  (Which the author of the code obviously wasn't banking on.)

Your file name is making the attempted glob fail; it's been
deliberately silenced, so you don't notice.  But that also has the
effect of abandoning the attempt to quote the expression.

So I think we need to apply the quoting without the "~" that turns on
active glob characters any time we didn't get globbing to work.

(Whether the globbing should actually have worked is a question I
haven't gone into.  But I suspect it's not a problem --- the _expand
function is trying lots of things not all of which will work.)

Maybe?

diff --git a/Completion/Base/Completer/_expand b/Completion/Base/Completer/_expand
index ee3681bad..f4909826a 100644
--- a/Completion/Base/Completer/_expand
+++ b/Completion/Base/Completer/_expand
@@ -103,9 +103,19 @@ subd=("$exp[@]")
 
 # Now try globbing.
 
-[[ "$force" = *g* ]] || zstyle -T ":completion:${curcontext}:" glob &&
-    eval 'exp=( ${~exp//(#b)\\([ 	\"'"\'"'
+# We need to come out of this with consistent quoting, by hook or by crook.
+integer done_quote
+local orig_exp=$exp
+if [[ "$force" = *g* ]] || zstyle -T ":completion:${curcontext}:" glob; then
+  eval 'exp=( ${~exp//(#b)\\([ 	\"'"\'"'
+])/$match[1]} ); exp=( ${(q)exp} )' 2>/dev/null && done_quote=1
+fi
+# If the globbing failed, or we didn't try globbing, we'll do
+# it again without the "~" so globbing is simply omitted.
+if (( ! done_quote )); then
+  eval 'exp=( ${orig_exp//(#b)\\([ 	\"'"\'"'
 ])/$match[1]} ); exp=( ${(q)exp} )' 2>/dev/null
+fi
 
 ### Don't remember why we once used this instead of the (q) above.
 #    eval 'exp=( ${~exp} ); exp=( ${exp//(#b)([][()|*?^#~<>\\=])/\\${match[1]}} )' 2>/dev/null

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Non-escaped special chars from var expansion
  2018-11-02 20:39 Non-escaped special chars from var expansion Wayne Davison
  2018-11-02 22:06 ` Peter Stephenson
@ 2018-11-02 23:21 ` Wayne Davison
  2019-11-17  6:00 ` Wayne Davison
  2 siblings, 0 replies; 5+ messages in thread
From: Wayne Davison @ 2018-11-02 23:21 UTC (permalink / raw)
  To: Zsh list

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

Thanks, Peter!  That is working well for me (at least in all my simple
tests).

..wayne..

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Non-escaped special chars from var expansion
  2018-11-02 22:06 ` Peter Stephenson
@ 2018-11-03 19:49   ` Peter Stephenson
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2018-11-03 19:49 UTC (permalink / raw)
  To: Zsh list

On Fri, 2 Nov 2018 22:06:57 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > bindkey '\t' complete-word
> > mkdir 'foo; bar (baz)'
> > cd !:1
> > cd $PWD<tab>
> 
> (Whether the globbing should actually have worked is a question I
> haven't gone into.  But I suspect it's not a problem --- the _expand
> function is trying lots of things not all of which will work.)

In case anyone cares --- no, the glob shouldn't have worked here because
it looks like a bad set of glob qualifiers ("unknown file atribute b").

Next problem is whether it's trying too hard to get glob compilation on
something that's expanded from a variable, without GLOB_SUBST set.
Strictly, I think that probably *is* a problem.  In principle the
quoting should happen on the result of that expansion, and only
if was expanded (from a variable or command or whatever).  I don't think
I'm going down that route myself, however.

Fix so far committed --- I will leave it at that.

pws

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Non-escaped special chars from var expansion
  2018-11-02 20:39 Non-escaped special chars from var expansion Wayne Davison
  2018-11-02 22:06 ` Peter Stephenson
  2018-11-02 23:21 ` Wayne Davison
@ 2019-11-17  6:00 ` Wayne Davison
  2 siblings, 0 replies; 5+ messages in thread
From: Wayne Davison @ 2019-11-17  6:00 UTC (permalink / raw)
  To: Zsh list

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

I've discovered that the prior fix for the following issue is actually
incomplete:

On Fri, Nov 2, 2018 at 1:39 PM Wayne Davison <wayne@opencoder.net> wrote:

> zsh -f
> autoload -Uz compinit
> compinit
> zstyle ':completion:*' completer _oldlist _expand _complete _match
> _ignored _files _prefix
> zstyle ':completion:*:expand:*' tag-order all-expansions
> bindkey -e
> bindkey '\t' complete-word
> mkdir 'foo; bar (baz)'
> cd !:1
> cd $PWD<tab>
>

The fix works for this particular case because the attempted glob with
"(baz)" generated an error. However, I've found 2 cases that don't generate
an error where the non-glob code needs to be run, but it doesn't happen
because "done_quote=1" got set.  For instance, if you just run the test
from one level deeper in that weird directory:

    mkdir foo ; cd foo ; ls $PWD<tab>

That will expand to just "ls /tmp/foo;\ bar\ (baz)/foo" (the same bad
quoting as before). Alternatively, if we name the dir with some valid glob
suffix, it also fails to quote:

    mkdir '/tmp/foo (#qN)'
    cd !:1
    ls $PWD<tab>

In both failure cases I saw that $exp was an empty string, so I tweaked the
"&& done_quote=1" part of the code (in _expand) to be "&& [[ "$exp" ]] &&
done_quote=1" and this has fixed the quoting issues I was seeing.  I'm not
sure if it is the right fix, though.

..wayne..

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-02 20:39 Non-escaped special chars from var expansion Wayne Davison
2018-11-02 22:06 ` Peter Stephenson
2018-11-03 19:49   ` Peter Stephenson
2018-11-02 23:21 ` Wayne Davison
2019-11-17  6:00 ` Wayne Davison

zsh-workers

Archives are clonable: git clone --mirror http://inbox.vuxu.org/zsh-workers

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git