Re: [PATCH] completion: repair config completion for Zsh
- From
D. Ben Knoble <ben.knoble+github@gmail.com>
- Date
- Jan 3, 2025, 18:07 UTC
- Message-ID
- <CALnO6CDpH4w8nOx7YDygtnY+G5NTBRbMGs7HTTDUHguYkCELiw@mail.gmail.com>
- In-Reply-To
- <4060690e-39b0-678a-75ef-af30beb439f3@gmail.com>
On Fri, Jan 3, 2025 at 12:21 PM Philippe Blain <levraiphilippeblain@gmail.com> wrote:
Show 22 quoted lines
>
> Hi Ben,
>
> Le 2024-12-29 à 19:00, D. Ben Knoble via GitGitGadget a écrit :
> > From: "D. Ben Knoble" <ben.knoble+github@gmail.com>
> >
> > Commit 1e0ee4087e (completion: add and use
> > __git_compute_first_level_config_vars_for_section, 2024-02-10) uses an
> > indirect variable syntax that is only valid for Bash, but the Zsh
> > completion code relies on the Bash completion code to function. Zsh
> > supports a different indirect variable expansion using ${(P)var}, but in
> > `emulate ksh` mode does not support Bash's ${!var}.
> >
> > This manifests as completing strange config options like
> > "__git_first_level_config_vars_for_section_remote" as a choice for the
> > command line
> >
> > git config set remote.
>
> Sorry for breaking the zsh completion with this change. Tip: it is customary
> in this project to CC commit authors when you identify a commit that
> caused a regression :)Once upon a time I knew that ;) thanks for the reminder.
Show 5 quoted lines
> > > [wall of text] > > I'm not sure this wall of text brings valuable information to the > commit message.
How about something like the range-diff (pushed to the remote) pasted to the PR?
> Thanks, I verified that this indeed works, at least with on my (old) system > with Bash 3.2.57 and Zsh 5.0.8.
Excellent!
Show 10 quoted lines
> > I'm wondering what could be done to prevent regressions like this in > the future. In [1], brian mentions a way to test the whole test suite with Zsh > in "sh" mode, which could be added to one of our CI jobs. > > But the completion test script (t9902-completion.sh) is really Bash-specific > and does 'exec bash' if it detects it is not running in Bash, so this would not > help us anyway... > > [1] https://lore.kernel.org/git/20240426221154.2194139-1-sandals@crustytoothpaste.net/
I have no real thoughts on the testing bit, but it would be nice to avoid. Long-term, it may be necessary to either split the completion scripts (as Bash/Zsh diverge?) or document non-portable constructs specific to the completion setup? Neither is automatic of the kind you had in mind :(