| Message ID | 20240319203244.799796-1-mb@x14.nl (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix | expand |
Hi Michiel, This is my code, so I'm really glad somebody else finds it useful! On Tue, Mar 19, 2024 at 4:33 PM Michiel W. Beijen <mb@x14.nl> wrote: > > There are a few environment variables that can influence the output for > the __git_ps1 macro in git-prompt.sh. All settings that are 'on/off' > types such as GIT_PS1_SHOWUNTRACKEDFILES and GIT_PS1_SHOWDIRTYSTATE > just take any value, and in the tests are tested with 'y', however > GIT_PS1_SHOWCONFLICTSTATE must be set to 'yes' otherwise it will not > work. I had actually considered using set/unset (for the same reason as you - consistency), but was advised to use a boolean flag. See: https://marc.info/?l=git&m=165897458021238&w=2 and https://marc.info/?l=git&m=165903017715652&w=2 > > This commit changes that behaviour, and makes sure > GIT_PS1_SHOWCONFLICTSTATE is consistent with these other parameters. > > Signed-off-by: Michiel W. Beijen <mb@x14.nl> > --- > contrib/completion/git-prompt.sh | 6 +++--- > t/t9903-bash-prompt.sh | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh > index 71f179cba3..fd6141e463 100644 > --- a/contrib/completion/git-prompt.sh > +++ b/contrib/completion/git-prompt.sh > @@ -85,8 +85,8 @@ > # by setting GIT_PS1_OMITSPARSESTATE. > # > # If you would like to see a notification on the prompt when there are > -# unresolved conflicts, set GIT_PS1_SHOWCONFLICTSTATE to "yes". The > -# prompt will include "|CONFLICT". > +# unresolved conflicts, set GIT_PS1_SHOWCONFLICTSTATE to a nonempty > +# value. The prompt will include "|CONFLICT". > # > # If you would like to see more information about the identity of > # commits checked out as a detached HEAD, set GIT_PS1_DESCRIBE_STYLE > @@ -528,7 +528,7 @@ __git_ps1 () > fi > > local conflict="" # state indicator for unresolved conflicts > - if [[ "${GIT_PS1_SHOWCONFLICTSTATE}" == "yes" ]] && > + if [ -n "${GIT_PS1_SHOWCONFLICTSTATE-}" ] && > [[ $(git ls-files --unmerged 2>/dev/null) ]]; then > conflict="|CONFLICT" > fi > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > index d667dda654..6479a0d898 100755 > --- a/t/t9903-bash-prompt.sh > +++ b/t/t9903-bash-prompt.sh > @@ -769,7 +769,7 @@ test_expect_success 'prompt - conflict indicator' ' > test_when_finished "git reset --hard HEAD~" && > test_must_fail git stash apply && > ( > - GIT_PS1_SHOWCONFLICTSTATE="yes" && > + GIT_PS1_SHOWCONFLICTSTATE=y && > __git_ps1 >"$actual" > ) && > test_cmp expected "$actual" > > base-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec > -- > 2.43.0 >
On 19-03-2024 23:58, Justin Donnelly wrote: > Hi Michiel, > This is my code, so I'm really glad somebody else finds it useful! > > > On Tue, Mar 19, 2024 at 4:33 PM Michiel W. Beijen <mb@x14.nl> wrote: >> There are a few environment variables that can influence the output for >> the __git_ps1 macro in git-prompt.sh. All settings that are 'on/off' >> types such as GIT_PS1_SHOWUNTRACKEDFILES and GIT_PS1_SHOWDIRTYSTATE >> just take any value, and in the tests are tested with 'y', however >> GIT_PS1_SHOWCONFLICTSTATE must be set to 'yes' otherwise it will not >> work. > I had actually considered using set/unset (for the same reason as you > - consistency), but was advised to use a boolean flag. > > See: https://marc.info/?l=git&m=165897458021238&w=2 and > https://marc.info/?l=git&m=165903017715652&w=2 I read the comments in that thread. While requiring the setting be set to 'yes' explicitly might make it possible to change it to a three-way switch in some unknown future, I think right now it is confusing and strange that of the many settings for GIT_PS1 only this one requires the explicit value 'yes'. So I would still request to consider this change. -- Michiel
On Sun, Mar 24, 2024 at 1:18 PM Michiel Beijen <mb@x14.nl> wrote: > > On 19-03-2024 23:58, Justin Donnelly wrote: > > > Hi Michiel, > > This is my code, so I'm really glad somebody else finds it useful! > > > > > > On Tue, Mar 19, 2024 at 4:33 PM Michiel W. Beijen <mb@x14.nl> wrote: > >> There are a few environment variables that can influence the output for > >> the __git_ps1 macro in git-prompt.sh. All settings that are 'on/off' > >> types such as GIT_PS1_SHOWUNTRACKEDFILES and GIT_PS1_SHOWDIRTYSTATE > >> just take any value, and in the tests are tested with 'y', however > >> GIT_PS1_SHOWCONFLICTSTATE must be set to 'yes' otherwise it will not > >> work. > > I had actually considered using set/unset (for the same reason as you > > - consistency), but was advised to use a boolean flag. > > > > See: https://marc.info/?l=git&m=165897458021238&w=2 and > > https://marc.info/?l=git&m=165903017715652&w=2 > > I read the comments in that thread. While requiring the setting be set > to 'yes' explicitly might make it possible to change it to a three-way > switch in some unknown future, I think right now it is confusing and > strange that of the many settings for GIT_PS1 only this one requires the > explicit value 'yes'. > > So I would still request to consider this change. It's not clear to me who besides Junio has the authority to approve (i.e. who you have to convince). But it isn't me. You might want to CC a few others (maybe use `git-contacts` to determine who) and see if anyone is interested in discussing this. Good luck! > > -- > > Michiel >
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 71f179cba3..fd6141e463 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -85,8 +85,8 @@ # by setting GIT_PS1_OMITSPARSESTATE. # # If you would like to see a notification on the prompt when there are -# unresolved conflicts, set GIT_PS1_SHOWCONFLICTSTATE to "yes". The -# prompt will include "|CONFLICT". +# unresolved conflicts, set GIT_PS1_SHOWCONFLICTSTATE to a nonempty +# value. The prompt will include "|CONFLICT". # # If you would like to see more information about the identity of # commits checked out as a detached HEAD, set GIT_PS1_DESCRIBE_STYLE @@ -528,7 +528,7 @@ __git_ps1 () fi local conflict="" # state indicator for unresolved conflicts - if [[ "${GIT_PS1_SHOWCONFLICTSTATE}" == "yes" ]] && + if [ -n "${GIT_PS1_SHOWCONFLICTSTATE-}" ] && [[ $(git ls-files --unmerged 2>/dev/null) ]]; then conflict="|CONFLICT" fi diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index d667dda654..6479a0d898 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -769,7 +769,7 @@ test_expect_success 'prompt - conflict indicator' ' test_when_finished "git reset --hard HEAD~" && test_must_fail git stash apply && ( - GIT_PS1_SHOWCONFLICTSTATE="yes" && + GIT_PS1_SHOWCONFLICTSTATE=y && __git_ps1 >"$actual" ) && test_cmp expected "$actual"
There are a few environment variables that can influence the output for the __git_ps1 macro in git-prompt.sh. All settings that are 'on/off' types such as GIT_PS1_SHOWUNTRACKEDFILES and GIT_PS1_SHOWDIRTYSTATE just take any value, and in the tests are tested with 'y', however GIT_PS1_SHOWCONFLICTSTATE must be set to 'yes' otherwise it will not work. This commit changes that behaviour, and makes sure GIT_PS1_SHOWCONFLICTSTATE is consistent with these other parameters. Signed-off-by: Michiel W. Beijen <mb@x14.nl> --- contrib/completion/git-prompt.sh | 6 +++--- t/t9903-bash-prompt.sh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) base-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec