zsh-workers
 help / color / mirror / code / Atom feed
* Bug in completion with curly braces?
@ 2020-11-20  1:50 Felipe Contreras
  2020-11-20  2:52 ` Mikael Magnusson
  2020-11-21 15:28 ` Daniel Shahaf
  0 siblings, 2 replies; 19+ messages in thread
From: Felipe Contreras @ 2020-11-20  1:50 UTC (permalink / raw)
  To: Zsh hackers list

Hello,

While adding unquoted completions such as "stash@{0}" the completion
system gets confused while inside the curly braces.

For example, given:

  compadd -Q -- 'stash@{0}' 'stash@{1}'

The first completion correctly generates "stash@{", but the second one
generates curly braces, the third one does the same, and so on ad
infinitum.

I didn't specify file (-f) or any special completion, so why would zle
attempt curly brace expansion, especially if the words contain curly
braces, and the current character is a curly brace?

Here's a simple test:

----------------------------------------
#compdef foo

_foo () {
compadd -Q -- 'stash@{0}' 'stash@{1}'
}

_foo
----------------------------------------

Cheers.

-- 
Felipe Contreras


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

* Re: Bug in completion with curly braces?
  2020-11-20  1:50 Bug in completion with curly braces? Felipe Contreras
@ 2020-11-20  2:52 ` Mikael Magnusson
  2020-11-21 15:28 ` Daniel Shahaf
  1 sibling, 0 replies; 19+ messages in thread
From: Mikael Magnusson @ 2020-11-20  2:52 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Zsh hackers list

On 11/20/20, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> Hello,
>
> While adding unquoted completions such as "stash@{0}" the completion
> system gets confused while inside the curly braces.
>
> For example, given:
>
>   compadd -Q -- 'stash@{0}' 'stash@{1}'
>
> The first completion correctly generates "stash@{", but the second one
> generates curly braces, the third one does the same, and so on ad
> infinitum.
>
> I didn't specify file (-f) or any special completion, so why would zle
> attempt curly brace expansion, especially if the words contain curly
> braces, and the current character is a curly brace?
>
> Here's a simple test:
>
> ----------------------------------------
> #compdef foo
>
> _foo () {
> compadd -Q -- 'stash@{0}' 'stash@{1}'
> }
>
> _foo

This test case crashes for me, the following patch prevents the crash
and seems to restore normal operation, but there's probably some other
thing that has already gone wrong at this point,

diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c
index 787d376a19..67546d0470 100644
--- a/Src/Zle/compresult.c
+++ b/Src/Zle/compresult.c
@@ -608,7 +608,7 @@ instmatch(Cmatch m, int *scs)
     r += l;
     ocs = zlemetacs;
     /* Re-insert the brace beginnings, if any. */
-    if (brbeg) {
+    if (brbeg && m->brpl) {
 	int pcs = zlemetacs;

 	l = 0;


-- 
Mikael Magnusson


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

* Re: Bug in completion with curly braces?
  2020-11-20  1:50 Bug in completion with curly braces? Felipe Contreras
  2020-11-20  2:52 ` Mikael Magnusson
@ 2020-11-21 15:28 ` Daniel Shahaf
  2020-11-21 21:08   ` Felipe Contreras
  1 sibling, 1 reply; 19+ messages in thread
From: Daniel Shahaf @ 2020-11-21 15:28 UTC (permalink / raw)
  To: Felipe Contreras, Zsh hackers list

Felipe Contreras wrote on Fri, 20 Nov 2020 01:50 +00:00:
>   compadd -Q -- 'stash@{0}' 'stash@{1}'
> 
> The first completion correctly generates "stash@{", but the second one
> generates curly braces, the third one does the same, and so on ad
> infinitum.
> 

I can reproduce this in «zsh -f» zsh-5.8-259-g00d20ed.  I don't get a segfault.

> I didn't specify file (-f) or any special completion, so why would zle
> attempt curly brace expansion, especially if the words contain curly
> braces, and the current character is a curly brace?

Inserting the braces into the command line unescaped is correct since -Q
was passed to compadd.  (Note that when -Q is not provided, the braces
_are_ inserted escaped.)

Treating them as starting a brace expansion is also correct, because -Q
doesn't disable that part of the grammar.  -Q only means that when a
candidate completion is inserted into the command line, the inserted
word doesn't get escaped (e.g., compare «compadd 'foo$bar'» to «compadd
-Q 'foo$bar'»).  If that word contains metacharacters (e.g., «compadd -Q
'|| /bin/echo hello world'»), then -Q makes it the caller's
responsibility to escape them as needed.

Consequently, brace expansion is presumably attempted for the same
reason it s attempted when the words don't contain braces.  E.g., after
«compadd -Q bar baz» followed by «foo ba{<TAB>», it offers «r» and «z»
as completions, which let you build up «{bar,baz}» and «{baz,bar}»
respectively.  Similarly, given «stash@{<TAB>», it presumably takes the
brace for the start of a brace expansion, and the analogous construct
which that behaviour lets you build up is «stash@{\{0\},\{1\}}» or
«stash@{\{1\},\{0\}}» — except that when you press <TAB> a second time,
the newly-inserted brace is once again inserted unescaped, which
presumably gets handled as the start of a nested brace expansion.

As to -f, curly brace expansion is perfectly valid for arguments that
aren't filenames, such as «compadd -Q stash@\{{0,1}\}».  (Brace
expansion is several layers removed from filenameness checks, which is
why stuff like «ls {-{l,d},/bin}» works.)

You can probably just remove the -Q.

Daniel


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

* Re: Bug in completion with curly braces?
  2020-11-21 15:28 ` Daniel Shahaf
@ 2020-11-21 21:08   ` Felipe Contreras
  2020-11-21 22:32     ` Bart Schaefer
  0 siblings, 1 reply; 19+ messages in thread
From: Felipe Contreras @ 2020-11-21 21:08 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list

On Sat, Nov 21, 2020 at 9:29 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:

> Consequently, brace expansion is presumably attempted for the same
> reason it s attempted when the words don't contain braces.  E.g., after
> «compadd -Q bar baz» followed by «foo ba{<TAB>», it offers «r» and «z»
> as completions, which let you build up «{bar,baz}» and «{baz,bar}»
> respectively.  Similarly, given «stash@{<TAB>», it presumably takes the
> brace for the start of a brace expansion, and the analogous construct
> which that behaviour lets you build up is «stash@{\{0\},\{1\}}» or
> «stash@{\{1\},\{0\}}» — except that when you press <TAB> a second time,
> the newly-inserted brace is once again inserted unescaped, which
> presumably gets handled as the start of a nested brace expansion.

But zle knows { is part of the completions, that's the reason why it's
adding it after a <tab>... It's adding precisely the thing that was
just typed.

> You can probably just remove the -Q.

I can't.

-- 
Felipe Contreras


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

* Re: Bug in completion with curly braces?
  2020-11-21 21:08   ` Felipe Contreras
@ 2020-11-21 22:32     ` Bart Schaefer
  2020-11-22  0:37       ` Felipe Contreras
  0 siblings, 1 reply; 19+ messages in thread
From: Bart Schaefer @ 2020-11-21 22:32 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Daniel Shahaf, Zsh hackers list

On Sat, Nov 21, 2020 at 1:09 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> On Sat, Nov 21, 2020 at 9:29 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> > Consequently, brace expansion is presumably attempted for the same
> > reason it s attempted when the words don't contain braces.
>
> But zle knows { is part of the completions

Actually, it does not.  Every TAB starts over from scratch with
figuring out how to divide the command line into complete-able
"words".  It doesn't know what is going to be supplied to compadd as
the set of matches until much later.

Consequently the line is split before the unquoted "{" on the
assumption that a brace expansion is going to follow it.

It might be easier to understand/debug this by observing the effect of
the following example:

_foo() { compadd -Q 'stash@{0}' 'stash@{1}'; compstate[list]=keep }
compdef _foo foo

Looking at that with _complete_debug, the prefix is being set to
"stash@" rather than to "stash@{", and you get repeated TABs cycling
through "stash@{{0}" and "stash@{{1}".  It's waiting for you to close
the presumed brace expansion yourself.

This is similar to the situation in Daniel's thread where "{" in
command position is assumed to be the start of a "{ subcommand }".
There's only so much completion can do to guess what a heavily
overloaded token means.

This appears to work around it:

_foo() {
  local -a stashes=( 'stash@{0}' 'stash@{1}')
  if [[ $words[CURRENT] = *@\{ ]];
  then compadd -Q -p ${words[CURRENT]%\{} ${stashes#$words[CURRENT]}
  else compadd -Q $stashes
  fi
}

However, that gets confused if $stashes contains e.g. 'stash@{10}' and
you're trying to complete after the digit "1", so there's more
refinement required.  Any time there's an unquoted "{" on the line
you're eventually going to run into something that wants to treat it
as a brace expansion.

I even tried forcing "ignorebraces" to be on during completion, but
the best that does (in combo with other changes to the compadd) is to
convert the word on the line to be "stash@\{" which you don't want.

I've got one other idea about this but I'll send this email and think
more about the other option.


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

* Re: Bug in completion with curly braces?
  2020-11-21 22:32     ` Bart Schaefer
@ 2020-11-22  0:37       ` Felipe Contreras
  2020-11-22  2:28         ` Bart Schaefer
  0 siblings, 1 reply; 19+ messages in thread
From: Felipe Contreras @ 2020-11-22  0:37 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Daniel Shahaf, Zsh hackers list

On Sat, Nov 21, 2020 at 4:33 PM Bart Schaefer <schaefer@brasslantern.com> wrote:

> I even tried forcing "ignorebraces" to be on during completion, but
> the best that does (in combo with other changes to the compadd) is to
> convert the word on the line to be "stash@\{" which you don't want.

I don't want to provide "stash@\{0\}" because that's not what I want
to complete.

But if it's the only way completion can work, I think the completion
system should add it itself, even if I didn't provide it.

Cheers.

-- 
Felipe Contreras


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

* Re: Bug in completion with curly braces?
  2020-11-22  0:37       ` Felipe Contreras
@ 2020-11-22  2:28         ` Bart Schaefer
  2020-11-22  2:58           ` Felipe Contreras
  0 siblings, 1 reply; 19+ messages in thread
From: Bart Schaefer @ 2020-11-22  2:28 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Daniel Shahaf, Zsh hackers list

On Sat, Nov 21, 2020 at 4:38 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> I don't want to provide "stash@\{0\}" because that's not what I want
> to complete.
>
> But if it's the only way completion can work, I think the completion
> system should add it itself, even if I didn't provide it.

That is what happens if you don't pass the -Q option, but by passing
that to compadd you have explicitly told the completion system not to
do so.


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

* Re: Bug in completion with curly braces?
  2020-11-22  2:28         ` Bart Schaefer
@ 2020-11-22  2:58           ` Felipe Contreras
  2020-11-22 20:35             ` Bart Schaefer
  0 siblings, 1 reply; 19+ messages in thread
From: Felipe Contreras @ 2020-11-22  2:58 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Daniel Shahaf, Zsh hackers list

On Sat, Nov 21, 2020 at 8:28 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Sat, Nov 21, 2020 at 4:38 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > I don't want to provide "stash@\{0\}" because that's not what I want
> > to complete.
> >
> > But if it's the only way completion can work, I think the completion
> > system should add it itself, even if I didn't provide it.
>
> That is what happens if you don't pass the -Q option, but by passing
> that to compadd you have explicitly told the completion system not to
> do so.

That's right, I told it to complete something, and it can't.

So the options are:

1. Fail the completion
2. Complete something I didn't ask for (but it's close)
3. Complete correctly what I asked for

I'm saying if 3 is not feasible, then perhaps 2 would make sense
rather than 1. It's at least something.

-- 
Felipe Contreras


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

* Re: Bug in completion with curly braces?
  2020-11-22  2:58           ` Felipe Contreras
@ 2020-11-22 20:35             ` Bart Schaefer
  2020-11-22 21:20               ` Bart Schaefer
                                 ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Bart Schaefer @ 2020-11-22 20:35 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Daniel Shahaf, Zsh hackers list

On Sat, Nov 21, 2020 at 6:58 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> So the options are:
>
> 1. Fail the completion
> 2. Complete something I didn't ask for (but it's close)
> 3. Complete correctly what I asked for
>
> I'm saying if 3 is not feasible, then perhaps 2 would make sense
> rather than 1. It's at least something.

It's up to your completion function to tell it that #2 is OK, and what
to offer in that circumstance.  That's one thing e.g. _alternative is
for.

In this specific example, though, any time an unquoted "{" appears on
the command line at the time completion is invoked, it's being treated
as the start of a brace expansion and therefore removed from the match
comparisons so that completion can try to fill in a comma-separated
list of replacements.

This does appear to be related to a couple of actual bugs, because
"setopt ignorebraces" is supposed to disable this, but does not do so
correctly.  Before I get into that, here's a hack to take advantage of
the brace-completion behavior to get something that works the way I
think you would like:

_foo() {
  local stashes=('stash@{0}' 'stash@{1}');
  if [[ $words[CURRENT] = *\{* ]]
  then compadd -Q -d stashes ${stashes//\{/}
  else compadd -Q -a stashes
  fi
}

This just says "if there's already a brace on the line, pretend this
is a brace expansion but include the braces in the menu display".

As to the bug ... there are actually a few of them, all inter-related.

1)  _comp_options includes "NO_ignorebraces" so isset(IGNOREBRACES) is
never true in zle_tricky.c:get_comp_string
2) Even if that's removed from _comp_options (and set in the caller
context), when the brace is added to $PREFIX it is escaped with a
backslash, so it no longer matches the string on the line and
completion fails
3) When IGNOREBRACES is set, the $debug_indent string in
_complete_debug is mis-handled
4) Possibly as a consequence of that, invoking _complete_debug from
outside GDB with ignorebraces causes the shell to exit (no reported
error, it just exits)

#3 is relatively easy to fix but I won't yet in case someone can track
down #4 (I've had no luck).


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

* Re: Bug in completion with curly braces?
  2020-11-22 20:35             ` Bart Schaefer
@ 2020-11-22 21:20               ` Bart Schaefer
  2020-11-23  3:03               ` Daniel Shahaf
  2020-11-23  6:46               ` Felipe Contreras
  2 siblings, 0 replies; 19+ messages in thread
From: Bart Schaefer @ 2020-11-22 21:20 UTC (permalink / raw)
  To: Zsh hackers list

On Sun, Nov 22, 2020 at 12:35 PM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> 3) When IGNOREBRACES is set, the $debug_indent string in
> _complete_debug is mis-handled
> 4) Possibly as a consequence of that, invoking _complete_debug from
> outside GDB with ignorebraces causes the shell to exit (no reported
> error, it just exits)

A bit more on this ... testing with a repaired _complete_debug, I
still get the exit, but it's unpredictable.  I did manage to get it to
happen inside GDB once, resulting in

exited with code 0177

but after setting a breakpoint on exit to try to get a stack trace, I
can't reproduce in GDB any more.


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

* Re: Bug in completion with curly braces?
  2020-11-22 20:35             ` Bart Schaefer
  2020-11-22 21:20               ` Bart Schaefer
@ 2020-11-23  3:03               ` Daniel Shahaf
  2020-11-23  7:15                 ` Bart Schaefer
  2020-11-23  6:46               ` Felipe Contreras
  2 siblings, 1 reply; 19+ messages in thread
From: Daniel Shahaf @ 2020-11-23  3:03 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Felipe Contreras, Zsh hackers list

Bart Schaefer wrote on Sun, Nov 22, 2020 at 12:35:52 -0800:
> 1)  _comp_options includes "NO_ignorebraces" so isset(IGNOREBRACES) is
> never true in zle_tricky.c:get_comp_string

I'm guessing that option is set so autoloaded completion functions have
predictable parsing, but that means completion code can't tell the value
of the option in the interactive environment.

In z-sy-h we solved this by declaring an assoc parameter and setting it
to the value of $options (or emulating that if zsh/parameter is
unavailable for some reason).

Sounds like a problem that would be useful to solve generically.

Cheers,

Daniel


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

* Re: Bug in completion with curly braces?
  2020-11-22 20:35             ` Bart Schaefer
  2020-11-22 21:20               ` Bart Schaefer
  2020-11-23  3:03               ` Daniel Shahaf
@ 2020-11-23  6:46               ` Felipe Contreras
  2020-11-23  7:17                 ` Bart Schaefer
  2020-11-23 17:31                 ` Bart Schaefer
  2 siblings, 2 replies; 19+ messages in thread
From: Felipe Contreras @ 2020-11-23  6:46 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Daniel Shahaf, Zsh hackers list

On Sun, Nov 22, 2020 at 2:36 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Sat, Nov 21, 2020 at 6:58 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > So the options are:
> >
> > 1. Fail the completion
> > 2. Complete something I didn't ask for (but it's close)
> > 3. Complete correctly what I asked for
> >
> > I'm saying if 3 is not feasible, then perhaps 2 would make sense
> > rather than 1. It's at least something.
>
> It's up to your completion function to tell it that #2 is OK, and what
> to offer in that circumstance.  That's one thing e.g. _alternative is
> for.

I'm not using _alternative, I'm using compadd. If compadd doesn't do
what I told it to do, then #1 it is.

> In this specific example, though, any time an unquoted "{" appears on
> the command line at the time completion is invoked, it's being treated
> as the start of a brace expansion and therefore removed from the match
> comparisons so that completion can try to fill in a comma-separated
> list of replacements.
>
> This does appear to be related to a couple of actual bugs, because
> "setopt ignorebraces" is supposed to disable this, but does not do so
> correctly.  Before I get into that, here's a hack to take advantage of
> the brace-completion behavior to get something that works the way I
> think you would like:
>
> _foo() {
>   local stashes=('stash@{0}' 'stash@{1}');
>   if [[ $words[CURRENT] = *\{* ]]
>   then compadd -Q -d stashes ${stashes//\{/}
>   else compadd -Q -a stashes
>   fi
> }

Yeah, this almost works, but I'm not interested in hacks.

My real function is something like this:

__compadd () {
  compadd -Q -p "${2-}" -S "${3- }" ${@[4,-1]} -- ${=1}
}

I'm not going to add dozens of lines of code just for this corner-case
depending on the value of $1. If compadd doesn't work on this case,
then it doesn't work on this case.

> As to the bug ... there are actually a few of them, all inter-related.
>
> 1)  _comp_options includes "NO_ignorebraces" so isset(IGNOREBRACES) is
> never true in zle_tricky.c:get_comp_string
> 2) Even if that's removed from _comp_options (and set in the caller
> context), when the brace is added to $PREFIX it is escaped with a
> backslash, so it no longer matches the string on the line and
> completion fails
> 3) When IGNOREBRACES is set, the $debug_indent string in
> _complete_debug is mis-handled
> 4) Possibly as a consequence of that, invoking _complete_debug from
> outside GDB with ignorebraces causes the shell to exit (no reported
> error, it just exits)
>
> #3 is relatively easy to fix but I won't yet in case someone can track
> down #4 (I've had no luck).

So, in theory if the user has ignorebraces set, there would be no need
to use quotes for the curly braces?

Cheers.

-- 
Felipe Contreras


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

* Re: Bug in completion with curly braces?
  2020-11-23  3:03               ` Daniel Shahaf
@ 2020-11-23  7:15                 ` Bart Schaefer
  0 siblings, 0 replies; 19+ messages in thread
From: Bart Schaefer @ 2020-11-23  7:15 UTC (permalink / raw)
  To: Zsh hackers list

On Sun, Nov 22, 2020 at 7:04 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Bart Schaefer wrote on Sun, Nov 22, 2020 at 12:35:52 -0800:
> > 1)  _comp_options includes "NO_ignorebraces" so isset(IGNOREBRACES) is
> > never true in zle_tricky.c:get_comp_string

This turns out to be wrong.  get_comp_string is called before
_comp_options is applied.

It's possible the whole problem comes down to the backslash improperly
added to $PREFIX.

> In z-sy-h we solved this by declaring an assoc parameter and setting it
> to the value of $options (or emulating that if zsh/parameter is
> unavailable for some reason).

This role is filled by $_comp_caller_options, which should be getting
set on entry to the completion system (via eval $_comp_setup).


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

* Re: Bug in completion with curly braces?
  2020-11-23  6:46               ` Felipe Contreras
@ 2020-11-23  7:17                 ` Bart Schaefer
  2021-04-18 21:43                   ` Bart Schaefer
  2020-11-23 17:31                 ` Bart Schaefer
  1 sibling, 1 reply; 19+ messages in thread
From: Bart Schaefer @ 2020-11-23  7:17 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Zsh hackers list

On Sun, Nov 22, 2020 at 10:46 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> So, in theory if the user has ignorebraces set, there would be no need
> to use quotes for the curly braces?

As far as I can tell, that should be how it works, yes.


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

* Re: Bug in completion with curly braces?
  2020-11-23  6:46               ` Felipe Contreras
  2020-11-23  7:17                 ` Bart Schaefer
@ 2020-11-23 17:31                 ` Bart Schaefer
  2020-11-24 23:21                   ` Oliver Kiddle
  1 sibling, 1 reply; 19+ messages in thread
From: Bart Schaefer @ 2020-11-23 17:31 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Daniel Shahaf, Zsh hackers list

On Sun, Nov 22, 2020 at 10:46 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> Yeah, this almost works, but I'm not interested in hacks.
[...]
> I'm not going to add dozens of lines of code just for this corner-case
> depending on the value of $1. If compadd doesn't work on this case,
> then it doesn't work on this case.

There could easily be more documentation around "compadd -Q", because
really it is itself a hack.

The parts of completion that analyze/dismantle the string on the
command line, before _main_complete or another entry point are even
called, are always going to process the line as if special characters
that are not quoted do in fact have their special meanings.  The real
purpose of -Q is not to disable quoting, it's to indicate that the
calling function has already applied the appropriate quoting and
therefore compadd should not also attempt it.

Consequently if the calling function does not apply quoting, -Q is
only going to work well in cases where the matches either have no
common prefixes, or the common prefixes do not contain any of the
special characters, or menu completion is going to be forced (so the
user selects an entire match all at once rather than exit/re-enter
completion with the prefix).

(I would be happy to have someone (Oliver?) argue the untruth of that
last statement.)


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

* Re: Bug in completion with curly braces?
  2020-11-23 17:31                 ` Bart Schaefer
@ 2020-11-24 23:21                   ` Oliver Kiddle
  2020-11-25  5:06                     ` Felipe Contreras
  0 siblings, 1 reply; 19+ messages in thread
From: Oliver Kiddle @ 2020-11-24 23:21 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Felipe Contreras, Daniel Shahaf, Zsh hackers list

Bart Schaefer wrote:
> Consequently if the calling function does not apply quoting, -Q is
> only going to work well in cases where the matches either have no
> common prefixes, or the common prefixes do not contain any of the
> special characters, or menu completion is going to be forced (so the
> user selects an entire match all at once rather than exit/re-enter
> completion with the prefix).
>
> (I would be happy to have someone (Oliver?) argue the untruth of that
> last statement.)

I think it is essentially true except that your list of three special
cases can perhaps be expanded. To say it doesn't "work well" perhaps
misrepresents that it does do something well-defined, just not
especially useful when misused.

If you search for compadd.*-Q in completion functions, the cases are not
normal. It is often combined with -U which disables matching altogether.
You'll see cases where $PREFIX was used in building the actual values
passed to compadd. Cases where compquote has been used first. And where
QIPREFIX is checked. And anything that looks normal is probably wrong,
e.g. in _gdb.

I've been assuming that in Felipe's case, he is forced to use -Q because
that is what bashcompinit does. bashcompinit uses it because of a patch
he submitted in 30136 to make it do so. At the time, I wasn't motivated
to review it but can remember being somewhat skeptical. Looking in more
detail now, it is correct in essence but it doesn't surprise me that it
should also cause some unexpected issues.

Looking at the bash example in 30136, it would appear that quoting
is left up to the completion function. Is that always true for bash
completions? Matches are added with an unquoted trailing space but also
some quoted spaces. To get it to pass through compgen, it seems two
levels of quoting and removing space from IFS was necessary. Is this
idiom common?

We could do something like the following in bashcompinit to special case
only space suffixes:

-      compadd -Q "${suf[@]}" -a matches && ret=0
+      compadd "${suf[@]}" - "${(@)${(Q@)matches}:#*\ }" && ret=0
+      compadd -S ' ' - ${${(M)${(Q)matches}:#*\ }% } && ret=0

For the more general case, we'd need to identify characters
with one-level of quoting. Or do a limited number of special cases cover
real-world usage?

Ultimately there's always going to be a mismatch between what needs
quoting in bash and in zsh. Note that in bash no quoting is needed for
git stashes:

bash$ echo stash{1}
stash{1}

So a bash completion for git didn't need to add quotes - hence this
whole discussion. Perhaps we could add an option to allow that. Stuff
like @\{1\}.. is also common in git.

If completion that comes with git is really a lot faster, I think you'd
be better served by finding out why and adding styles to zsh's
completion to allow it to take a faster, more rudimentary approach in
certain cases. It may even already be possible by disabling the right
tags.

Oliver


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

* Re: Bug in completion with curly braces?
  2020-11-24 23:21                   ` Oliver Kiddle
@ 2020-11-25  5:06                     ` Felipe Contreras
  2021-12-02 19:45                       ` PATCH: bashcompinit quoting (was Re: Bug in completion with curly braces?) Oliver Kiddle
  0 siblings, 1 reply; 19+ messages in thread
From: Felipe Contreras @ 2020-11-25  5:06 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: Bart Schaefer, Daniel Shahaf, Zsh hackers list

On Tue, Nov 24, 2020 at 5:22 PM Oliver Kiddle <opk@zsh.org> wrote:

> Looking at the bash example in 30136, it would appear that quoting
> is left up to the completion function. Is that always true for bash
> completions? Matches are added with an unquoted trailing space but also
> some quoted spaces. To get it to pass through compgen, it seems two
> levels of quoting and removing space from IFS was necessary. Is this
> idiom common?

It is common to modify IFS in order to add the correct elements to the
COMPREPLY array. As of the multiple levels of quoting I don't know
what you mean. What do you want to add?

> We could do something like the following in bashcompinit to special case
> only space suffixes:
>
> -      compadd -Q "${suf[@]}" -a matches && ret=0
> +      compadd "${suf[@]}" - "${(@)${(Q@)matches}:#*\ }" && ret=0
> +      compadd -S ' ' - ${${(M)${(Q)matches}:#*\ }% } && ret=0

Yes, that should work. Although I think bash would add two spaces if
-o nospace isn't specified.

Recently I tried to do something similar, which left me wondering; why
isn't there a way to specify the suffix already present in the words?

For example Completion/Base/Completer/_expand has code to split into
dir, space, and normal arrays, and add different suffixes to each one
of them.

It would be much easier to somehow say "consider the X ending a suffix".

> For the more general case, we'd need to identify characters
> with one-level of quoting. Or do a limited number of special cases cover
> real-world usage?
>
> Ultimately there's always going to be a mismatch between what needs
> quoting in bash and in zsh. Note that in bash no quoting is needed for
> git stashes:
>
> bash$ echo stash{1}
> stash{1}
>
> So a bash completion for git didn't need to add quotes - hence this
> whole discussion. Perhaps we could add an option to allow that. Stuff
> like @\{1\}.. is also common in git.

Personally I would like to see "stash@{0}" completed, not
"stash@\{0\}", but anything is better than no completion.

> If completion that comes with git is really a lot faster, I think you'd
> be better served by finding out why and adding styles to zsh's
> completion to allow it to take a faster, more rudimentary approach in
> certain cases. It may even already be possible by disabling the right
> tags.

Yes, but it's not an easy task.

In the zsh git completion everything creates a huge tree of functions
of functions of functions. Even after I find the actual code that is
doing something, that would only be for one case, then I have to go to
the next one. And that would be only for one command.

In the git bash completion it's just one function, and all the
commands call that function, so it's much easier to understand.

I sent a patch to call git's __git_complete_revlist from inside zsh's
git completion, to showcase one way how this "fast" mode could work.

Cheers.

-- 
Felipe Contreras


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

* Re: Bug in completion with curly braces?
  2020-11-23  7:17                 ` Bart Schaefer
@ 2021-04-18 21:43                   ` Bart Schaefer
  0 siblings, 0 replies; 19+ messages in thread
From: Bart Schaefer @ 2021-04-18 21:43 UTC (permalink / raw)
  To: Zsh hackers list

On Sun, Nov 22, 2020 at 11:17 PM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> On Sun, Nov 22, 2020 at 10:46 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > So, in theory if the user has ignorebraces set, there would be no need
> > to use quotes for the curly braces?
>
> As far as I can tell, that should be how it works, yes.

Reviving this thread because it ended without resolution, but I do
have a patch for _complete_debug that makes the situation a little
less confusing:

diff --git a/Completion/Base/Widget/_complete_debug
b/Completion/Base/Widget/_complete_debug
index 85a0f372a..94fd4accd 100644
--- a/Completion/Base/Widget/_complete_debug
+++ b/Completion/Base/Widget/_complete_debug
@@ -14,7 +14,11 @@ integer debug_fd=-1
     exec {debug_fd}>&2 2>| $tmp
   fi

-  local -a debug_indent; debug_indent=( '%'{3..20}'(e. .)' )
+  local -a debug_indent
+  () {
+    setopt localoptions no_ignorebraces
+    debug_indent=( '%'{3..20}'(e. .)' )
+  }
   local PROMPT4 PS4="${(j::)debug_indent}+%N:%i> "
   setopt xtrace
   : $ZSH_NAME $ZSH_VERSION


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

* PATCH: bashcompinit quoting (was Re: Bug in completion with curly braces?)
  2020-11-25  5:06                     ` Felipe Contreras
@ 2021-12-02 19:45                       ` Oliver Kiddle
  0 siblings, 0 replies; 19+ messages in thread
From: Oliver Kiddle @ 2021-12-02 19:45 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: Felipe Contreras

On 24 Nov 2020, Felipe Contreras wrote:
> > We could do something like the following in bashcompinit to special case
> > only space suffixes:
> >
> > -      compadd -Q "${suf[@]}" -a matches && ret=0
> > +      compadd "${suf[@]}" - "${(@)${(Q@)matches}:#*\ }" && ret=0
> > +      compadd -S ' ' - ${${(M)${(Q)matches}:#*\ }% } && ret=0
>
> Yes, that should work. Although I think bash would add two spaces if
> -o nospace isn't specified.

I never did anything further with this a year ago. Revisiting it
and trying to find examples, that approach does seem to be more
compatible with bash.

bashcompinit did acquire two uses of -Q. The other case is for
filename completion. The bash complete builtin has at some point
acquired a noquote option which will "Tell readline not to quote
the completed words if they are filenames (quoting filenames is the
default)." So I think the right change in that position is merely
to remove the -Q. The patch does this.

Bash has a few newer complete options and a new compopt builtin
for changing the applicable options from within a bash completion
function. It doesn't look especially hard to implement these but
I'm not sure how wise that is. bashcompinit was possibly a mistake
to begin with. If people are using it then supporting newer features
would improve their experience. And maybe give them less motivation
to write a native zsh function.

Oliver

diff --git a/Completion/bashcompinit b/Completion/bashcompinit
index b278ac8f4..adb659ca1 100644
--- a/Completion/bashcompinit
+++ b/Completion/bashcompinit
@@ -24,9 +24,10 @@ _bash_complete() {
     if [[ ${argv[${argv[(I)filenames]:-0}-1]} = -o ]]; then
       compset -P '*/' && matches=( ${matches##*/} )
       compset -S '/*' && matches=( ${matches%%/*} )
-      compadd -Q -f "${suf[@]}" -a matches && ret=0
+      compadd -f "${suf[@]}" -a matches && ret=0
     else
-      compadd -Q "${suf[@]}" -a matches && ret=0
+      compadd "${suf[@]}" - "${(@)${(Q@)matches}:#*\ }" && ret=0
+      compadd -S ' ' - ${${(M)${(Q)matches}:#*\ }% } && ret=0
     fi
   fi
 


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

end of thread, other threads:[~2021-12-02 19:45 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20  1:50 Bug in completion with curly braces? Felipe Contreras
2020-11-20  2:52 ` Mikael Magnusson
2020-11-21 15:28 ` Daniel Shahaf
2020-11-21 21:08   ` Felipe Contreras
2020-11-21 22:32     ` Bart Schaefer
2020-11-22  0:37       ` Felipe Contreras
2020-11-22  2:28         ` Bart Schaefer
2020-11-22  2:58           ` Felipe Contreras
2020-11-22 20:35             ` Bart Schaefer
2020-11-22 21:20               ` Bart Schaefer
2020-11-23  3:03               ` Daniel Shahaf
2020-11-23  7:15                 ` Bart Schaefer
2020-11-23  6:46               ` Felipe Contreras
2020-11-23  7:17                 ` Bart Schaefer
2021-04-18 21:43                   ` Bart Schaefer
2020-11-23 17:31                 ` Bart Schaefer
2020-11-24 23:21                   ` Oliver Kiddle
2020-11-25  5:06                     ` Felipe Contreras
2021-12-02 19:45                       ` PATCH: bashcompinit quoting (was Re: Bug in completion with curly braces?) Oliver Kiddle

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).