completion: fix zsh parsing $GIT_PS1_SHOWUPSTREAM
PATCH 1/1 completion: fix zsh parsing $GIT_PS1_SHOWUPSTREAM
From: Thomas Queiroz <[email protected]>
Since GIT_PS1_SHOWUPSTREAM is a variable with space separated values and zsh for loops do no split by space by default, parsing of the options wasn't actually being done. The -d' ' is a hacky solution that works in both bash and zsh. The correct way to do that in zsh would be do use read -rA and loop over the resulting array but -A isn't defined in bash.
---
completion: Fix zsh parsing $GIT_PS1_SHOWUPSTREAM
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1710%2Fthomasqueirozb%2Fzsh-completion-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1710/thomasqueirozb/zsh-completion-fix-v1
Pull-Request: https://github.com/git/git/pull/1710
contrib/completion/git-prompt.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
contrib/completion/git-prompt.sh +2 -2
@@ -141,14 +141,14 @@ __git_ps1_show_upstream () # parse configuration values local option - for option in ${GIT_PS1_SHOWUPSTREAM-}; do + while read -r -d' ' option; do case "$option" in git|svn) upstream_type="$option" ;; verbose) verbose=1 ;; legacy) legacy=1 ;; name) name=1 ;; esac - done + done <<< "${GIT_PS1_SHOWUPSTREAM-} " # Find our upstream type case "$upstream_type" in
base-commit: 21306a098c3f174ad4c2a5cddb9069ee27a548b0
Signature · 1 line
gitgitgadget
Quoted 7 lines · Thomas via GitGitGadget
From: Thomas Queiroz <[email protected]>
Since
GIT_PS1_SHOWUPSTREAMis a variable with space separated values and zsh for loops do no split by space by default, parsing of the options wasn't actually being done. The-d' 'is a hacky solution that works in both bash and zsh. The correct way to do that in zsh would be do use read -rA and loop over the resulting array but -A isn't defined in bash.
I wonder if it might actually be better to adjust the shell options when we call into __git_ps1. We could write this like so:
[ -z "${ZSH_VERSION-}" ] || setopt localoptions shwordsplitThat will turn on shell word splitting for just that function (and the functions it calls), so the existing code will work fine and we won't tamper with the user's preferred shell options.
My concern is that changing the way we write the code here might result in someone unintentionally changing it back because it's less intuitive. By specifically asking zsh to use shell word splitting, we get consistent behaviour between bash and zsh, which is really what we want anyway.
I use the above syntax (minus the shell check) in my zsh prompt and can confirm it works as expected.
Signature · 2 lines
brian m. carlson (they/them or he/him) Toronto, Ontario, CA
Quoted 8 lines · brian m. carlson
I wonder if it might actually be better to adjust the shell options when we call into
__git_ps1. We could write this like so:[ -z "${ZSH_VERSION-}" ] || setopt localoptions shwordsplitThat will turn on shell word splitting for just that function (and the functions it calls), so the existing code will work fine and we won't tamper with the user's preferred shell options.
Nice. I did
$ git grep -e 'for [a-z0-9_]* in ' contrib/completion/
and wondered why other hits were OK. The completion one seems to have "emulate" all over the place to hide zsh-ness from functions it borrows from git-completion.bash, but git-prompt side seems to lack necessary "compatibility" stuff.
Quoted 5 lines
My concern is that changing the way we write the code here might result in someone unintentionally changing it back because it's less intuitive. By specifically asking zsh to use shell word splitting, we get consistent behaviour between bash and zsh, which is really what we want anyway.
Very well said.
Quoted 2 lines
I use the above syntax (minus the shell check) in my zsh prompt and can confirm it works as expected.
Thanks.
By the way, I notice that the title of the patch talks about "completion", but this is about a prompt. It needs to be updated in a future iteration.