{"thread":{"id":"61157","subject":"[PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix","startedAt":"2024-03-19T20:51:12Z","lastAt":"2024-03-24T19:01:45Z","messageCount":4,"participants":["Michiel W. Beijen","Justin Donnelly","Michiel Beijen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"490955","messageId":"20240319203244.799796-1-mb@x14.nl","threadId":"61157","inReplyTo":null,"subject":"[PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix","fromName":"Michiel W. Beijen","fromEmail":"mb@x14.nl","sentAt":"2024-03-19T20:32:44Z","receivedAt":"2024-03-19T20:51:12Z","isPatch":true,"sender":{"key":"mb@x14.nl","avatar":"https://gravatar.com/avatar/077c66aba7ce687dc4f1f33e9deb9a5bbe921a506d3759a79ddb028326d9c265?d=mp&s=160"},"body":"There are a few environment variables that can influence the output for\nthe __git_ps1 macro in git-prompt.sh. All settings that are 'on/off'\ntypes such as GIT_PS1_SHOWUNTRACKEDFILES and GIT_PS1_SHOWDIRTYSTATE\njust take any value, and in the tests are tested with 'y', however\nGIT_PS1_SHOWCONFLICTSTATE must be set to 'yes' otherwise it will not\nwork.\n\nThis commit changes that behaviour, and makes sure\nGIT_PS1_SHOWCONFLICTSTATE is consistent with these other parameters.\n\nSigned-off-by: Michiel W. Beijen <mb@x14.nl>\n---\n contrib/completion/git-prompt.sh | 6 +++---\n t/t9903-bash-prompt.sh           | 2 +-\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex 71f179cba3..fd6141e463 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -85,8 +85,8 @@\n # by setting GIT_PS1_OMITSPARSESTATE.\n #\n # If you would like to see a notification on the prompt when there are\n-# unresolved conflicts, set GIT_PS1_SHOWCONFLICTSTATE to \"yes\". The\n-# prompt will include \"|CONFLICT\".\n+# unresolved conflicts, set GIT_PS1_SHOWCONFLICTSTATE to a nonempty\n+# value. The prompt will include \"|CONFLICT\".\n #\n # If you would like to see more information about the identity of\n # commits checked out as a detached HEAD, set GIT_PS1_DESCRIBE_STYLE\n@@ -528,7 +528,7 @@ __git_ps1 ()\n \tfi\n \n \tlocal conflict=\"\" # state indicator for unresolved conflicts\n-\tif [[ \"${GIT_PS1_SHOWCONFLICTSTATE}\" == \"yes\" ]] &&\n+\tif [ -n \"${GIT_PS1_SHOWCONFLICTSTATE-}\" ] &&\n \t   [[ $(git ls-files --unmerged 2>/dev/null) ]]; then\n \t\tconflict=\"|CONFLICT\"\n \tfi\ndiff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh\nindex d667dda654..6479a0d898 100755\n--- a/t/t9903-bash-prompt.sh\n+++ b/t/t9903-bash-prompt.sh\n@@ -769,7 +769,7 @@ test_expect_success 'prompt - conflict indicator' '\n \ttest_when_finished \"git reset --hard HEAD~\" &&\n \ttest_must_fail git stash apply &&\n \t(\n-\t\tGIT_PS1_SHOWCONFLICTSTATE=\"yes\" &&\n+\t\tGIT_PS1_SHOWCONFLICTSTATE=y &&\n \t\t__git_ps1 >\"$actual\"\n \t) &&\n \ttest_cmp expected \"$actual\"\n\nbase-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec\n-- \n2.43.0\n\n"},{"id":"490977","messageId":"CAGTqyRykxy2GJV6b4yavWnegbz0HwykaT7bExOXK4wENdoF+Qg@mail.gmail.com","threadId":"61157","inReplyTo":"20240319203244.799796-1-mb@x14.nl","subject":"Re: [PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix","fromName":"Justin Donnelly","fromEmail":"justinrdonnelly@gmail.com","sentAt":"2024-03-19T22:58:23Z","receivedAt":"2024-03-19T22:59:00Z","isPatch":true,"sender":{"key":"justinrdonnelly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24846452?v=4"},"body":"Hi Michiel,\nThis is my code, so I'm really glad somebody else finds it useful!\n\n\nOn Tue, Mar 19, 2024 at 4:33 PM Michiel W. Beijen <mb@x14.nl> wrote:\n>\n> There are a few environment variables that can influence the output for\n> the __git_ps1 macro in git-prompt.sh. All settings that are 'on/off'\n> types such as GIT_PS1_SHOWUNTRACKEDFILES and GIT_PS1_SHOWDIRTYSTATE\n> just take any value, and in the tests are tested with 'y', however\n> GIT_PS1_SHOWCONFLICTSTATE must be set to 'yes' otherwise it will not\n> work.\n\nI had actually considered using set/unset (for the same reason as you\n- consistency), but was advised to use a boolean flag.\n\nSee: https://marc.info/?l=git&m=165897458021238&w=2 and\nhttps://marc.info/?l=git&m=165903017715652&w=2\n\n\n>\n> This commit changes that behaviour, and makes sure\n> GIT_PS1_SHOWCONFLICTSTATE is consistent with these other parameters.\n>\n> Signed-off-by: Michiel W. Beijen <mb@x14.nl>\n> ---\n>  contrib/completion/git-prompt.sh | 6 +++---\n>  t/t9903-bash-prompt.sh           | 2 +-\n>  2 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\n> index 71f179cba3..fd6141e463 100644\n> --- a/contrib/completion/git-prompt.sh\n> +++ b/contrib/completion/git-prompt.sh\n> @@ -85,8 +85,8 @@\n>  # by setting GIT_PS1_OMITSPARSESTATE.\n>  #\n>  # If you would like to see a notification on the prompt when there are\n> -# unresolved conflicts, set GIT_PS1_SHOWCONFLICTSTATE to \"yes\". The\n> -# prompt will include \"|CONFLICT\".\n> +# unresolved conflicts, set GIT_PS1_SHOWCONFLICTSTATE to a nonempty\n> +# value. The prompt will include \"|CONFLICT\".\n>  #\n>  # If you would like to see more information about the identity of\n>  # commits checked out as a detached HEAD, set GIT_PS1_DESCRIBE_STYLE\n> @@ -528,7 +528,7 @@ __git_ps1 ()\n>         fi\n>\n>         local conflict=\"\" # state indicator for unresolved conflicts\n> -       if [[ \"${GIT_PS1_SHOWCONFLICTSTATE}\" == \"yes\" ]] &&\n> +       if [ -n \"${GIT_PS1_SHOWCONFLICTSTATE-}\" ] &&\n>            [[ $(git ls-files --unmerged 2>/dev/null) ]]; then\n>                 conflict=\"|CONFLICT\"\n>         fi\n> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh\n> index d667dda654..6479a0d898 100755\n> --- a/t/t9903-bash-prompt.sh\n> +++ b/t/t9903-bash-prompt.sh\n> @@ -769,7 +769,7 @@ test_expect_success 'prompt - conflict indicator' '\n>         test_when_finished \"git reset --hard HEAD~\" &&\n>         test_must_fail git stash apply &&\n>         (\n> -               GIT_PS1_SHOWCONFLICTSTATE=\"yes\" &&\n> +               GIT_PS1_SHOWCONFLICTSTATE=y &&\n>                 __git_ps1 >\"$actual\"\n>         ) &&\n>         test_cmp expected \"$actual\"\n>\n> base-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec\n> --\n> 2.43.0\n>\n"},{"id":"491366","messageId":"379141b4-d715-41b9-973f-ec0349ed11a5@x14.nl","threadId":"61157","inReplyTo":"CAGTqyRykxy2GJV6b4yavWnegbz0HwykaT7bExOXK4wENdoF+Qg@mail.gmail.com","subject":"Re: [PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix","fromName":"Michiel Beijen","fromEmail":"mb@x14.nl","sentAt":"2024-03-24T17:18:04Z","receivedAt":"2024-03-24T17:18:13Z","isPatch":true,"sender":{"key":"mb@x14.nl","avatar":"https://gravatar.com/avatar/077c66aba7ce687dc4f1f33e9deb9a5bbe921a506d3759a79ddb028326d9c265?d=mp&s=160"},"body":"On 19-03-2024 23:58, Justin Donnelly wrote:\n\n> Hi Michiel,\n> This is my code, so I'm really glad somebody else finds it useful!\n>\n>\n> On Tue, Mar 19, 2024 at 4:33 PM Michiel W. Beijen <mb@x14.nl> wrote:\n>> There are a few environment variables that can influence the output for\n>> the __git_ps1 macro in git-prompt.sh. All settings that are 'on/off'\n>> types such as GIT_PS1_SHOWUNTRACKEDFILES and GIT_PS1_SHOWDIRTYSTATE\n>> just take any value, and in the tests are tested with 'y', however\n>> GIT_PS1_SHOWCONFLICTSTATE must be set to 'yes' otherwise it will not\n>> work.\n> I had actually considered using set/unset (for the same reason as you\n> - consistency), but was advised to use a boolean flag.\n>\n> See: https://marc.info/?l=git&m=165897458021238&w=2 and\n> https://marc.info/?l=git&m=165903017715652&w=2\n\nI read the comments in that thread. While requiring the setting be set \nto 'yes' explicitly might make it possible to change it to a three-way \nswitch in some unknown future, I think right now it is confusing and \nstrange that of the many settings for GIT_PS1 only this one requires the \nexplicit value 'yes'.\n\nSo I would still request to consider this change.\n\n--\n\nMichiel\n\n"},{"id":"491376","messageId":"CAGTqyRywBKNooLuswSF9QDGgnS2J+R2cArdYyShKw6=s59WxpA@mail.gmail.com","threadId":"61157","inReplyTo":"379141b4-d715-41b9-973f-ec0349ed11a5@x14.nl","subject":"Re: [PATCH] git-prompt: GIT_PS1_SHOWCONFLICTSTATE variable fix","fromName":"Justin Donnelly","fromEmail":"justinrdonnelly@gmail.com","sentAt":"2024-03-24T19:01:08Z","receivedAt":"2024-03-24T19:01:45Z","isPatch":true,"sender":{"key":"justinrdonnelly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24846452?v=4"},"body":"On Sun, Mar 24, 2024 at 1:18 PM Michiel Beijen <mb@x14.nl> wrote:\n>\n> On 19-03-2024 23:58, Justin Donnelly wrote:\n>\n> > Hi Michiel,\n> > This is my code, so I'm really glad somebody else finds it useful!\n> >\n> >\n> > On Tue, Mar 19, 2024 at 4:33 PM Michiel W. Beijen <mb@x14.nl> wrote:\n> >> There are a few environment variables that can influence the output for\n> >> the __git_ps1 macro in git-prompt.sh. All settings that are 'on/off'\n> >> types such as GIT_PS1_SHOWUNTRACKEDFILES and GIT_PS1_SHOWDIRTYSTATE\n> >> just take any value, and in the tests are tested with 'y', however\n> >> GIT_PS1_SHOWCONFLICTSTATE must be set to 'yes' otherwise it will not\n> >> work.\n> > I had actually considered using set/unset (for the same reason as you\n> > - consistency), but was advised to use a boolean flag.\n> >\n> > See: https://marc.info/?l=git&m=165897458021238&w=2 and\n> > https://marc.info/?l=git&m=165903017715652&w=2\n>\n> I read the comments in that thread. While requiring the setting be set\n> to 'yes' explicitly might make it possible to change it to a three-way\n> switch in some unknown future, I think right now it is confusing and\n> strange that of the many settings for GIT_PS1 only this one requires the\n> explicit value 'yes'.\n>\n> So I would still request to consider this change.\n\nIt's not clear to me who besides Junio has the authority to approve\n(i.e. who you have to convince). But it isn't me. You might want to CC\na few others (maybe use `git-contacts` to determine who) and see if\nanyone is interested in discussing this. Good luck!\n\n>\n> --\n>\n> Michiel\n>\n"}]}