On Fri, Jan 3, 2025 at 12:21 PM Philippe Blain
[off-list ref] wrote:

Hi Ben,

Le 2024-12-29 à 19:00, D. Ben Knoble via GitGitGadget a écrit :

quoted

From: "D. Ben Knoble" <redacted>

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.

quoted

[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@crustytoothpaste.net/ (local)

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 :(