On Fri, Jan 3, 2025 at 12:21 PM Philippe Blain
<levraiphilippeblain@xxxxxxxxx> wrote:
>
> Hi Ben,
>
> Le 2024-12-29 à 19:00, D. Ben Knoble via GitGitGadget a écrit :
> > From: "D. Ben Knoble" <ben.knoble+github@xxxxxxxxx>
> >
> > 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.
>
> > [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!
>
> 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@xxxxxxxxxxxxxxxxxxxx/
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 :(
