[PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix

[PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix

[Michiel W. Beijen]

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

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

Re: [PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix

[Justin Donnelly]

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
>

Re: [PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix

[Michiel Beijen]

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

Re: [PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix

[Justin Donnelly]

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
>