bug: _files depends on extendedglob

bug: _files depends on extendedglob

[D. Ben Knoble]

Here's a transcript from a session after I finally pieced together
what was breaking in
https://github.com/benknoble/Dotfiles/commit/9c7dd6d1ec8b3caac670f1a2a030769a4c90c06c,
which contains in its message more details about how I debugged this
particular failure.

zsh --version
zsh 5.9 (x86_64-apple-darwin20.6.0)
# macOS 12.7.2
zsh -f
temp=$(mktemp -d)
fpath+=($temp)
wget https://raw.githubusercontent.com/git/git/master/contrib/completion/git-completion.zsh
-o $temp/_git
autoload -Uz compinit && compinit
_git_doc () { emulate -L zsh
  _files -W $(git --html-path)/
}
git doc <TAB>
# (eval):1: no matches found: *:globbed-files
# completion results still produced
_git_doc () { emulate -L zsh
  setopt extendedglob
  _files -W $(git --html-path)/
}
git doc <TAB>
# works like a charm

I'm not sure what the appropriate fix is, though I think some
combination of localopts and extendedglob might work? I see that some
parts of _files test for extendedglob, but the part that sets pats
that leads to the failure (line 78 for me) seems to be affected
without any test.

-- 
D. Ben Knoble

Re: bug: _files depends on extendedglob

[Mikael Magnusson]

On Fri, Mar 29, 2024 at 4:51 PM D. Ben Knoble <ben.knoble@gmail.com> wrote:
>
> Here's a transcript from a session after I finally pieced together
> what was breaking in
> https://github.com/benknoble/Dotfiles/commit/9c7dd6d1ec8b3caac670f1a2a030769a4c90c06c,
> which contains in its message more details about how I debugged this
> particular failure.
>
> zsh --version
> zsh 5.9 (x86_64-apple-darwin20.6.0)
> # macOS 12.7.2
> zsh -f
> temp=$(mktemp -d)
> fpath+=($temp)
> wget https://raw.githubusercontent.com/git/git/master/contrib/completion/git-completion.zsh
> -o $temp/_git
> autoload -Uz compinit && compinit
> _git_doc () { emulate -L zsh
>   _files -W $(git --html-path)/
> }
> git doc <TAB>
> # (eval):1: no matches found: *:globbed-files
> # completion results still produced
> _git_doc () { emulate -L zsh
>   setopt extendedglob
>   _files -W $(git --html-path)/
> }
> git doc <TAB>
> # works like a charm
>
> I'm not sure what the appropriate fix is, though I think some
> combination of localopts and extendedglob might work? I see that some
> parts of _files test for extendedglob, but the part that sets pats
> that leads to the failure (line 78 for me) seems to be affected
> without any test.

The completion system sets up the options it uses on entry, if you
change them you get to keep the pieces. (Why are you setting the
emulation mode in a completer function anyway?) If you for whatever
reason insist on doing this, then you can setopt $_comp_options to
restore the correct option set before calling _files.

-- 
Mikael Magnusson

Re: bug: _files depends on extendedglob

[Ben Knoble]

> Le 29 mars 2024 à 13:23, Mikael Magnusson <mikachu@gmail.com> a écrit :
> 
> The completion system sets up the options it uses on entry, if you
> change them you get to keep the pieces.

Good to know; I think I assumed that.

> (Why are you setting the
> emulation mode in a completer function anyway?)

In the linked commit from the original mail I explain the emulation mode: it is because the contributed Git completion script sets the emulation mode to ksh in order to leave most of the work to the bash completion scripts. I therefore (assume I) have to revert to zsh mode for _files to behave sensibly: not doing so produced a strange IIRC.

> If you for whatever
> reason insist on doing this, then you can setopt $_comp_options to
> restore the correct option set before calling _files.

That’s probably a much better workaround than what I had, thanks. I’ve confirmed that it works. Is this documented anywhere that I can rely on?

> -- 
> Mikael Magnusson

Re: bug: _files depends on extendedglob

[Bart Schaefer]

On Fri, Mar 29, 2024 at 10:44 AM Ben Knoble <ben.knoble@gmail.com> wrote:
>
> > you can setopt $_comp_options to
> > restore the correct option set
>
> Is this documented anywhere that I can rely on?

Sort of; in Completion/compinit:

> # The standard options set in completion functions.
>
> typeset -gHa _comp_options

Probably ought to, but doesn't yet, have a mention in e.g.
Etc/completion-style-guide (which also likely needs a
proofread/update).

Re: bug: _files depends on extendedglob

[Oliver Kiddle]

Ben Knoble wrote:
>
> In the linked commit from the original mail I explain the emulation
> mode: it is because the contributed Git completion script sets the
> emulation mode to ksh in order to leave most of the work to the bash
> completion scripts. I therefore (assume I) have to revert to zsh mode
> for _files to behave sensibly: not doing so produced a strange IIRC.

git doc is not an official git command so I'm not sure whether what
you're doing falls within the scope of what that contributed completion
supports but if it does then forcing ksh emulation on your function is
arguably a bug in that git completion.

I'd be very interested if you could elucidate on why you choose to use
that completion instead of the git completion that comes with zsh. It
is perhaps faster due to reduced functionality but in wrapping the bash
completion it has a much more limited interface that prevents it taking
advantage of many of zsh's more powerful features. Zsh's completion does
support handling additional commands in the same manner with, e.g. a
_git-doc function. There can be benefits to upstream projects carrying
completion functions but I usually avoid anything that wraps a bash
function.

> > reason insist on doing this, then you can setopt $_comp_options to
>
> That’s probably a much better workaround than what I had, thanks.
> I’ve confirmed that it works. Is this documented anywhere that I can
> rely on?

That's relying on internal implementation details of the completion
system so you can't really rely on it.

Oliver

Re: bug: _files depends on extendedglob

[D. Ben Knoble]

On Fri, Mar 29, 2024 at 3:43 PM Oliver Kiddle <opk@zsh.org> wrote:
>
> Ben Knoble wrote:
> >
> > In the linked commit from the original mail I explain the emulation
> > mode: it is because the contributed Git completion script sets the
> > emulation mode to ksh in order to leave most of the work to the bash
> > completion scripts. I therefore (assume I) have to revert to zsh mode
> > for _files to behave sensibly: not doing so produced a strange IIRC.
>
> git doc is not an official git command so I'm not sure whether what
> you're doing falls within the scope of what that contributed completion
> supports but if it does then forcing ksh emulation on your function is
> arguably a bug in that git completion.

Correct; I'm adding completion for a custom command (implemented by a
script called git-doc). I think the author would argue it's by design,
though it does make extension harder (unless you like writing bash
completion scripts). Author's post from 2013 indicated some
performance issues with the Zsh-provided script
(https://felipec.wordpress.com/2013/07/31/how-i-fixed-git-zsh-completion/);
have those been resolved? [The post itself seems a bit inflammatory,
so please don't read its tone into my words.]

> I'd be very interested if you could elucidate on why you choose to use
> that completion instead of the git completion that comes with zsh. It
> is perhaps faster due to reduced functionality but in wrapping the bash
> completion it has a much more limited interface that prevents it taking
> advantage of many of zsh's more powerful features. Zsh's completion does
> support handling additional commands in the same manner with, e.g. a
> _git-doc function. There can be benefits to upstream projects carrying
> completion functions but I usually avoid anything that wraps a bash
> function.

See https://github.com/benknoble/Dotfiles/commit/9c7dd6d1ec8b3caac670f1a2a030769a4c90c06c
and https://github.com/Homebrew/legacy-homebrew/issues/16992: the zsh
contrib script from Git is installed in a location such that it
supersedes the default script. At this point it's what I'm used to,
and I haven't spent time trying the other yet. It works well enough
for me at the moment. (Perhaps it's less a choice and more a "default"
that works, where by "default" I mean "that's what my package manager
did and I don't have the time to fight with it at the moment.)

> > > reason insist on doing this, then you can setopt $_comp_options to
> >
> > That’s probably a much better workaround than what I had, thanks.
> > I’ve confirmed that it works. Is this documented anywhere that I can
> > rely on?
>
> That's relying on internal implementation details of the completion
> system so you can't really rely on it.

Bart indicated it was undocumented but deserved to be.

-- 
D. Ben Knoble

Re: bug: _files depends on extendedglob

[Bart Schaefer]

On Sat, Mar 30, 2024 at 8:28 AM D. Ben Knoble <ben.knoble@gmail.com> wrote:
>
> > That's relying on internal implementation details of the completion
> > system so you can't really rely on it.
>
> Bart indicated it was undocumented but deserved to be.

Both of those things can be true.

What we probably should do is create a "compopts" builtin** and
document that as the way to manipulate this.

** It can't be a shell function because of the way "localoptions" works.