[-- Attachment #1: Type: text/plain, Size: 723 bytes --] % cd $(mktemp -d) % ZDOTDIR=$PWD HOME=$PWD zsh -f % autoload -Uz compinit % compinit % zsh_directory_name() { [[ $1 == c ]] && compadd -S ']' foo } Given the above setup, when 1. $LBUFFER is `cd ~[` or `cd ~[fo`, 2. $RBUFFER is empty, and 3. I press ^I, 4. then $LBUFFER becomes `cd ~[foo]`. 1. $LBUFFER is `cd ~[`, 2. $RBUFFER is `fo` or `]`, and 3. I press ^I, 4. then completion beeps and the buffer remains unchanged. `functions -t _complete` shows that the problem is caused by $compstate[context] becoming `tilde` instead of `subscript` as soon as $SUFFIX is non-empty, which causes _complete() to not call _subscript(), which is the only point of entry to _dynamic_directory_name(). [-- Attachment #2: Type: text/html, Size: 2739 bytes --]
> On 21 January 2021 at 14:37 Marlon Richert <marlon.richert@gmail.com> wrote: > % cd $(mktemp -d) > % ZDOTDIR=$PWD HOME=$PWD zsh -f > % autoload -Uz compinit > % compinit > % zsh_directory_name() { [[ $1 == c ]] && compadd -S ']' foo } > > Given the above setup, when > > 1. $LBUFFER is `cd ~[` or `cd ~[fo`, > 2. $RBUFFER is empty, and > 3. I press ^I, > 4. then $LBUFFER becomes `cd ~[foo]`. > > 1. $LBUFFER is `cd ~[`, > 2. $RBUFFER is `fo` or `]`, and > 3. I press ^I, > 4. then completion beeps and the buffer remains unchanged. > > `functions -t _complete` shows that the problem is caused by > $compstate[context] becoming `tilde` instead of `subscript` as soon as > $SUFFIX is non-empty, which causes _complete() to not call _subscript(), > which is the only point of entry to _dynamic_directory_name(). The following is somewhere near, but this is quite complex and somewhat at odds with completion in other contexts --- it's not clear to me whether or not I should need that special diversion in _main_complete but currently I do. (Your compadd -S ']' is going to add too many ']'s in this case, I think, but that's a separate problem.) pws diff --git a/Completion/Base/Core/_main_complete b/Completion/Base/Core/_main_complete index 6b2cf2bcf..2bcbd2118 100644 --- a/Completion/Base/Core/_main_complete +++ b/Completion/Base/Core/_main_complete @@ -94,8 +94,18 @@ if [[ -z "$compstate[quote]" ]]; then if [[ -o equals ]] && compset -P 1 '='; then compstate[context]=equal elif [[ "$PREFIX" != */* && "$PREFIX[1]" = '~' ]]; then - compset -p 1 - compstate[context]=tilde + if [[ "$PREFIX" = '~['[^\]]## ]]; then + # Inside ~[...] should be treated as a subscript. + compset -p 2 + # To be consistent, we ignore all but the contents of the square + # brackets. + compset -S '\]*' + compstate[context]=subscript + [[ -n $_comps[-subscript-] ]] && $_comps[-subscript-] && return + else + compset -p 1 + compstate[context]=tilde + fi fi fi diff --git a/Completion/Zsh/Context/_subscript b/Completion/Zsh/Context/_subscript index 0c9a89ad5..0d9632864 100644 --- a/Completion/Zsh/Context/_subscript +++ b/Completion/Zsh/Context/_subscript @@ -1,6 +1,8 @@ #compdef -subscript- -local expl ind osuf=']' flags sep +local expl ind osuf flags sep + +[[ $ISUFFIX = *\]* ]] || osuf=\] if [[ "$1" = -q ]]; then compquote osuf diff --git a/Functions/Chpwd/zsh_directory_name_cdr b/Functions/Chpwd/zsh_directory_name_cdr index cb72e4600..b653e7c38 100644 --- a/Functions/Chpwd/zsh_directory_name_cdr +++ b/Functions/Chpwd/zsh_directory_name_cdr @@ -16,8 +16,10 @@ elif [[ $1 = c ]]; then typeset -a keys values values=(${${(f)"$(cdr -l)"}/ ##/:}) keys=(${values%%:*}) + local addsuffix + [[ $ISUFFIX = *\]* ]] || addsuffix='-S]' _describe -t dir-index 'recent directory index' \ - values -V unsorted -S']' + values -V unsorted $addsuffix return fi fi diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c index 8c0534708..1622d8a6b 100644 --- a/Src/Zle/zle_main.c +++ b/Src/Zle/zle_main.c @@ -1067,6 +1067,7 @@ redrawhook(void) int old_incompfunc = incompfunc; char *args[2]; Thingy lbindk_save = lbindk, bindk_save = bindk; + struct modifier zmod_save = zmod; refthingy(lbindk_save); refthingy(bindk_save); @@ -1094,6 +1095,7 @@ redrawhook(void) * restore lastcmd manually so that we don't mess up the global state */ lastcmd = lastcmd_prev; + zmod = zmod_save; } }
Could someone please review Peter's patch, below?
On Mon, Jan 25, 2021 at 5:07 PM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
> > On 21 January 2021 at 14:37 Marlon Richert <marlon.richert@gmail.com> wrote:
> > % cd $(mktemp -d)
> > % ZDOTDIR=$PWD HOME=$PWD zsh -f
> > % autoload -Uz compinit
> > % compinit
> > % zsh_directory_name() { [[ $1 == c ]] && compadd -S ']' foo }
> >
> > Given the above setup, when
> >
> > 1. $LBUFFER is `cd ~[` or `cd ~[fo`,
> > 2. $RBUFFER is empty, and
> > 3. I press ^I,
> > 4. then $LBUFFER becomes `cd ~[foo]`.
> >
> > 1. $LBUFFER is `cd ~[`,
> > 2. $RBUFFER is `fo` or `]`, and
> > 3. I press ^I,
> > 4. then completion beeps and the buffer remains unchanged.
> >
> > `functions -t _complete` shows that the problem is caused by
> > $compstate[context] becoming `tilde` instead of `subscript` as soon as
> > $SUFFIX is non-empty, which causes _complete() to not call _subscript(),
> > which is the only point of entry to _dynamic_directory_name().
>
> The following is somewhere near, but this is quite complex and somewhat
> at odds with completion in other contexts --- it's not clear to me
> whether or not I should need that special diversion in _main_complete
> but currently I do. (Your compadd -S ']' is going to add too many
> ']'s in this case, I think, but that's a separate problem.)
>
> diff --git a/Completion/Base/Core/_main_complete b/Completion/Base/Core/_main_complete
> index 6b2cf2bcf..2bcbd2118 100644
> --- a/Completion/Base/Core/_main_complete
> +++ b/Completion/Base/Core/_main_complete
> @@ -94,8 +94,18 @@ if [[ -z "$compstate[quote]" ]]; then
> if [[ -o equals ]] && compset -P 1 '='; then
> compstate[context]=equal
> elif [[ "$PREFIX" != */* && "$PREFIX[1]" = '~' ]]; then
> - compset -p 1
> - compstate[context]=tilde
> + if [[ "$PREFIX" = '~['[^\]]## ]]; then
> + # Inside ~[...] should be treated as a subscript.
> + compset -p 2
> + # To be consistent, we ignore all but the contents of the square
> + # brackets.
> + compset -S '\]*'
> + compstate[context]=subscript
> + [[ -n $_comps[-subscript-] ]] && $_comps[-subscript-] && return
> + else
> + compset -p 1
> + compstate[context]=tilde
> + fi
> fi
> fi
>
> diff --git a/Completion/Zsh/Context/_subscript b/Completion/Zsh/Context/_subscript
> index 0c9a89ad5..0d9632864 100644
> --- a/Completion/Zsh/Context/_subscript
> +++ b/Completion/Zsh/Context/_subscript
> @@ -1,6 +1,8 @@
> #compdef -subscript-
>
> -local expl ind osuf=']' flags sep
> +local expl ind osuf flags sep
> +
> +[[ $ISUFFIX = *\]* ]] || osuf=\]
>
> if [[ "$1" = -q ]]; then
> compquote osuf
> diff --git a/Functions/Chpwd/zsh_directory_name_cdr b/Functions/Chpwd/zsh_directory_name_cdr
> index cb72e4600..b653e7c38 100644
> --- a/Functions/Chpwd/zsh_directory_name_cdr
> +++ b/Functions/Chpwd/zsh_directory_name_cdr
> @@ -16,8 +16,10 @@ elif [[ $1 = c ]]; then
> typeset -a keys values
> values=(${${(f)"$(cdr -l)"}/ ##/:})
> keys=(${values%%:*})
> + local addsuffix
> + [[ $ISUFFIX = *\]* ]] || addsuffix='-S]'
> _describe -t dir-index 'recent directory index' \
> - values -V unsorted -S']'
> + values -V unsorted $addsuffix
> return
> fi
> fi
> diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
> index 8c0534708..1622d8a6b 100644
> --- a/Src/Zle/zle_main.c
> +++ b/Src/Zle/zle_main.c
> @@ -1067,6 +1067,7 @@ redrawhook(void)
> int old_incompfunc = incompfunc;
> char *args[2];
> Thingy lbindk_save = lbindk, bindk_save = bindk;
> + struct modifier zmod_save = zmod;
>
> refthingy(lbindk_save);
> refthingy(bindk_save);
> @@ -1094,6 +1095,7 @@ redrawhook(void)
> * restore lastcmd manually so that we don't mess up the global state
> */
> lastcmd = lastcmd_prev;
> + zmod = zmod_save;
> }
> }
> On 31 March 2021 at 09:10 Marlon Richert <marlon.richert@gmail.com> wrote:
> Could someone please review Peter's patch, below?
This change is already in.
pws
On Wed, Mar 31, 2021 at 11:25 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
> > On 31 March 2021 at 09:10 Marlon Richert <marlon.richert@gmail.com> wrote:
> > Could someone please review Peter's patch, below?
>
> This change is already in.
Really? I don't see it in Src/Zle/zle_main.c on the master branch.
On Wed, Mar 31, 2021 at 11:29 AM Marlon Richert
<marlon.richert@gmail.com> wrote:
>
> On Wed, Mar 31, 2021 at 11:25 AM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> > > On 31 March 2021 at 09:10 Marlon Richert <marlon.richert@gmail.com> wrote:
> > > Could someone please review Peter's patch, below?
> >
> > This change is already in.
>
> Really? I don't see it in Src/Zle/zle_main.c on the master branch.
You are right that the other changes are in. Sorry, I only checked
`git log -L 1060,1100:Src/Zle/zle_main.c` and when that failed to show
any changes, I assumed that the whole patch had not been applied yet.
However, why apply only the changes in the other files and not the one
in Src/Zle/zle_main.c ? Is that intentional or an oversight?
> On 31 March 2021 at 09:37 Marlon Richert <marlon.richert@gmail.com> wrote:
> On Wed, Mar 31, 2021 at 11:29 AM Marlon Richert
> <marlon.richert@gmail.com> wrote:
> >
> > On Wed, Mar 31, 2021 at 11:25 AM Peter Stephenson
> > <p.w.stephenson@ntlworld.com> wrote:
> > > > On 31 March 2021 at 09:10 Marlon Richert <marlon.richert@gmail.com> wrote:
> > > > Could someone please review Peter's patch, below?
> > >
> > > This change is already in.
> >
> > Really? I don't see it in Src/Zle/zle_main.c on the master branch.
>
> You are right that the other changes are in. Sorry, I only checked
> `git log -L 1060,1100:Src/Zle/zle_main.c` and when that failed to show
> any changes, I assumed that the whole patch had not been applied yet.
>
> However, why apply only the changes in the other files and not the one
> in Src/Zle/zle_main.c ? Is that intentional or an oversight?
Apologies, that was intentional because I realised the diff I sent included
some old stuff which actually wasn't needed. The zle_main.c chunk in the
patch is irrelevant to the fix.
pws
This bug is similar to the one I reported in workers/47858. This works correctly: % setopt completeinword % tst() { print "\n$compstate[context]"; zle -I } % # place cursor after '${(' and press Tab: % : ${(foo brace_parameter % foo=( ${( brace_parameter % foo=${( brace_parameter % [[ ${(foo ]] brace_parameter But if you've already typed more and you want to add parameter flags later on, $compstate[context] is no longer accurate: % # place cursor after '${(' and press Tab: % : ${()foo} command % foo=( ${( ) array_value % foo=${()bar} value % [[ ${()foo} ]] condition This matters when using the _prefix completer. I would expect to get parameter flag completions for the cases above. But because $compstate[context] does not equal 'brace_parameter', _complete will never call _brace_parameter.
[-- Attachment #1: Type: text/plain, Size: 1267 bytes --] On Fri, May 7, 2021 at 7:09 AM Marlon Richert <marlon.richert@gmail.com> wrote: > > This bug is similar to the one I reported in workers/47858. Only superficially. > This works correctly: > > % setopt completeinword > % tst() { print "\n$compstate[context]"; zle -I } I think there's something missing here for the test case. A style or a bindkey? Something to run "tst"? > % # place cursor after '${(' and press Tab: > % : ${()foo} This is actually a failure in the interpretation of the cursor position within the C code. An existing comment says: * We are still within the parameter flags. There's no * point trying to do anything clever here with * parameter names. Instead, just report that we are in * a brace parameter but let the completion function * decide what to do about it. The issue was that "still within the flags" assumed complete_in_word was not set. An earlier test had a similar mistake when the closing brace of the entire parameter expression was already present. Patch below attempts to fix those assumptions, but still leaves further effort up to the completion function ... which currently completes nothing when the parens are empty. I'm leaving that for someone else to work on. I also expanded on an existing comment. [-- Attachment #2: check_param.txt --] [-- Type: text/plain, Size: 2137 bytes --] diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c index 5162d97dc..131e86825 100644 --- a/Src/Zle/compcore.c +++ b/Src/Zle/compcore.c @@ -1125,6 +1125,18 @@ check_param(char *s, int set, int test) * * TODO: passing s as a parameter while we get some mysterious * offset "offs" into it via a global sucks badly. + * + * From ../lex.c we know: + * wb is the beginning position of the current word in the line + * we is the position of the end of the current word in the line + * From zle_tricky.c we know: + * offs is position within the word where we are completing + * + * So wb + offs is the current cursor position if COMPLETE_IN_WORD + * is set, otherwise it is the end of the word (same as we). + * + * Note below we are thus stepping backward from our completion + * position to find a '$' in the current word (if any). */ for (p = s + offs; ; p--) { if (*p == String || *p == Qstring) { @@ -1171,13 +1183,13 @@ check_param(char *s, int set, int test) char *tb = b; /* If this is a ${...}, see if we are before the '}'. */ - if (!skipparens(Inbrace, Outbrace, &tb)) + if (!skipparens(Inbrace, Outbrace, &tb) && tb - s <= offs) return NULL; /* Ignore the possible (...) flags. */ - b++, br++; - if ((qstring ? skipparens('(', ')', &b) : - skipparens(Inpar, Outpar, &b)) > 0) { + tb = ++b, br++; + if ((qstring ? skipparens('(', ')', &tb) : + skipparens(Inpar, Outpar, &tb)) > 0 || tb - s >= offs) { /* * We are still within the parameter flags. There's no * point trying to do anything clever here with @@ -1188,6 +1200,14 @@ check_param(char *s, int set, int test) ispar = 2; return NULL; } + if ((qstring ? '(' : Inpar) == *b) { + /* + * We are inside the braces but on the opening paren. + * There is nothing useful to complete here? + */ + return NULL; + } else + b = tb; /* Skip over the flags */ for (tb = p - 1; tb > s && *tb != Outbrace && *tb != Inbrace; tb--); if (tb > s && *tb == Inbrace && (tb[-1] == String || *tb == Qstring))
On Fri, May 7, 2021 at 7:09 AM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> This matters when using the _prefix completer.
Incidentally see workers/46047 regarding some other issues with _prefix.
On 8 May 2021, at 03:04, Bart Schaefer <schaefer@brasslantern.com> wrote:
>
>> This works correctly:
>>
>> % setopt completeinword
>> % tst() { print "\n$compstate[context]"; zle -I }
>
> I think there's something missing here for the test case. A style or
> a bindkey? Something to run "tst"?
Ah, yes. I apparently forget to add:
% zle -C tst complete-word tst
% bindkey '^I' tst
But it looks like you figured out the gist of it anyway. :)
It's still not working correctly in Zsh 5.9. Now compstate[context]
becomes `command` instead of `subscript` when $RBUFFER starts with a
non-space character.
On Mon, Jan 25, 2021 at 5:07 PM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> > On 21 January 2021 at 14:37 Marlon Richert <marlon.richert@gmail.com> wrote:
> > % cd $(mktemp -d)
> > % ZDOTDIR=$PWD HOME=$PWD zsh -f
> > % autoload -Uz compinit
> > % compinit
> > % zsh_directory_name() { [[ $1 == c ]] && compadd -S ']' foo }
> >
> > Given the above setup, when
> >
> > 1. $LBUFFER is `cd ~[` or `cd ~[fo`,
> > 2. $RBUFFER is empty, and
> > 3. I press ^I,
> > 4. then $LBUFFER becomes `cd ~[foo]`.
> >
> > 1. $LBUFFER is `cd ~[`,
> > 2. $RBUFFER is `fo` or `]`, and
> > 3. I press ^I,
> > 4. then completion beeps and the buffer remains unchanged.
> >
> > `functions -t _complete` shows that the problem is caused by
> > $compstate[context] becoming `tilde` instead of `subscript` as soon as
> > $SUFFIX is non-empty, which causes _complete() to not call _subscript(),
> > which is the only point of entry to _dynamic_directory_name().
>
> The following is somewhere near, but this is quite complex and somewhat
> at odds with completion in other contexts --- it's not clear to me
> whether or not I should need that special diversion in _main_complete
> but currently I do. (Your compadd -S ']' is going to add too many
> ']'s in this case, I think, but that's a separate problem.)
>
> pws
>
> diff --git a/Completion/Base/Core/_main_complete b/Completion/Base/Core/_main_complete
> index 6b2cf2bcf..2bcbd2118 100644
> --- a/Completion/Base/Core/_main_complete
> +++ b/Completion/Base/Core/_main_complete
> @@ -94,8 +94,18 @@ if [[ -z "$compstate[quote]" ]]; then
> if [[ -o equals ]] && compset -P 1 '='; then
> compstate[context]=equal
> elif [[ "$PREFIX" != */* && "$PREFIX[1]" = '~' ]]; then
> - compset -p 1
> - compstate[context]=tilde
> + if [[ "$PREFIX" = '~['[^\]]## ]]; then
> + # Inside ~[...] should be treated as a subscript.
> + compset -p 2
> + # To be consistent, we ignore all but the contents of the square
> + # brackets.
> + compset -S '\]*'
> + compstate[context]=subscript
> + [[ -n $_comps[-subscript-] ]] && $_comps[-subscript-] && return
> + else
> + compset -p 1
> + compstate[context]=tilde
> + fi
> fi
> fi
>
> diff --git a/Completion/Zsh/Context/_subscript b/Completion/Zsh/Context/_subscript
> index 0c9a89ad5..0d9632864 100644
> --- a/Completion/Zsh/Context/_subscript
> +++ b/Completion/Zsh/Context/_subscript
> @@ -1,6 +1,8 @@
> #compdef -subscript-
>
> -local expl ind osuf=']' flags sep
> +local expl ind osuf flags sep
> +
> +[[ $ISUFFIX = *\]* ]] || osuf=\]
>
> if [[ "$1" = -q ]]; then
> compquote osuf
> diff --git a/Functions/Chpwd/zsh_directory_name_cdr b/Functions/Chpwd/zsh_directory_name_cdr
> index cb72e4600..b653e7c38 100644
> --- a/Functions/Chpwd/zsh_directory_name_cdr
> +++ b/Functions/Chpwd/zsh_directory_name_cdr
> @@ -16,8 +16,10 @@ elif [[ $1 = c ]]; then
> typeset -a keys values
> values=(${${(f)"$(cdr -l)"}/ ##/:})
> keys=(${values%%:*})
> + local addsuffix
> + [[ $ISUFFIX = *\]* ]] || addsuffix='-S]'
> _describe -t dir-index 'recent directory index' \
> - values -V unsorted -S']'
> + values -V unsorted $addsuffix
> return
> fi
> fi
> diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
> index 8c0534708..1622d8a6b 100644
> --- a/Src/Zle/zle_main.c
> +++ b/Src/Zle/zle_main.c
> @@ -1067,6 +1067,7 @@ redrawhook(void)
> int old_incompfunc = incompfunc;
> char *args[2];
> Thingy lbindk_save = lbindk, bindk_save = bindk;
> + struct modifier zmod_save = zmod;
>
> refthingy(lbindk_save);
> refthingy(bindk_save);
> @@ -1094,6 +1095,7 @@ redrawhook(void)
> * restore lastcmd manually so that we don't mess up the global state
> */
> lastcmd = lastcmd_prev;
> + zmod = zmod_save;
> }
> }