zsh-workers
 help / color / mirror / code / Atom feed
* While on the subject of expansion ...
@ 2013-12-27 18:05 Bart Schaefer
  2013-12-28  8:23 ` Bart Schaefer
  2014-01-02 18:40 ` Peter Stephenson
  0 siblings, 2 replies; 3+ messages in thread
From: Bart Schaefer @ 2013-12-27 18:05 UTC (permalink / raw)
  To: zsh-workers

I noticed that the built-in expand-word widget also mishandles the case
with a backslashed quote.  There's a bit of a disagreement between
doexpansion() and get_comp_string() over who is responsible for what.
get_comp_string() strips out the backslashes, but it also "leaves ...
quotes unchanged when they are inside parameter expansions" according
to the comment in doexpansion().

2187     /* get_comp_string() leaves these quotes unchanged when they are
2188      * inside parameter expansions. */
2189     for (ts = ss; *ts; ts++)
2190         if (*ts == '"')
2191             *ts = Dnull;
2192         else if (*ts == '\'')
2193             *ts = Snull;

Therefore doexpansion() attempts to fix up the quotes, but gets it
wrong in some of the cases because it doesn't have full context at
each character position.  The "inwhat" and "instring" globals give
some hints but only for the position of the cursor, and there may be
multiple differently-quoted substrings of the current word.

Unlike the _expand completer which tries globbing separately from other
expansions, doexpansion() attempts to set up for everything to expand
at once [via a call to prefork()].  This makes the completion-oriented
[de-]quoting applied by get_comp_string() problematic.  I wonder which
kinds of parameter expansions would break if we skipped that part.

Again I suppose the theory is that quotes are much less likely to
appear in globs than they are in parameter references, so handling the
more common case is the right thing.

Incidental aside:  Both doexpansion() and expandorcomplete() contain
code to fall back on docompletion() if expansion does not change the
word, but I can't figure out how to get that branch in doexpansion()
to ever be taken.  It has something to do with words that start with
a tilde.


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

* Re: While on the subject of expansion ...
  2013-12-27 18:05 While on the subject of expansion Bart Schaefer
@ 2013-12-28  8:23 ` Bart Schaefer
  2014-01-02 18:40 ` Peter Stephenson
  1 sibling, 0 replies; 3+ messages in thread
From: Bart Schaefer @ 2013-12-28  8:23 UTC (permalink / raw)
  To: zsh-workers

On Dec 27, 10:05am, Bart Schaefer wrote:
}
} I noticed that the built-in expand-word widget also mishandles the case
} with a backslashed quote.

Tangentially related, and mentioning it just so it's on the record in
case I forget about it later:  The "compfiles" builtin is confused by
files whose names contain backslashes.  This is another pathological
case that may not be worth expending much effort.


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

* Re: While on the subject of expansion ...
  2013-12-27 18:05 While on the subject of expansion Bart Schaefer
  2013-12-28  8:23 ` Bart Schaefer
@ 2014-01-02 18:40 ` Peter Stephenson
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Stephenson @ 2014-01-02 18:40 UTC (permalink / raw)
  To: zsh-workers

On Fri, 27 Dec 2013 10:05:56 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Unlike the _expand completer which tries globbing separately from other
> expansions, doexpansion() attempts to set up for everything to expand
> at once [via a call to prefork()].  This makes the completion-oriented
> [de-]quoting applied by get_comp_string() problematic.  I wonder which
> kinds of parameter expansions would break if we skipped that part.

I suppose it might be worth a bit of trial simplification after 5.0.5 to see
what happens, though present experience suggests we'll only find out
what the damage is if the result appears in 5.0.6.

(If you think that's hairy, don't look at set_comp_sep() in compcore.c,
which attempts, rather badly, to take account of recursive quoting
i.e. quoted expressions that contain shell code for contexts like "su -c
'...".  I spent some time attacking this a few years ago but retreated
in disarray after my forces were battered to a pulp.  I think it's still
not that hard to get the shell to crash with a bit of playing around.)

> Tangentially related, and mentioning it just so it's on the record in
> case I forget about it later:  The "compfiles" builtin is confused by
> files whose names contain backslashes.  This is another pathological
> case that may not be worth expending much effort.

This one *might* be simple:  a missing removal of quotations or one too
many at a single point in the code.  I haven't looked.

pws


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

end of thread, other threads:[~2014-01-02 18:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-27 18:05 While on the subject of expansion Bart Schaefer
2013-12-28  8:23 ` Bart Schaefer
2014-01-02 18:40 ` Peter Stephenson

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).