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