Thanks for the feedback.


On Wed, Jul 27, 2022 at 4:13 PM Junio C Hamano [off-list ref] wrote:

"Justin Donnelly via GitGitGadget" [off-list ref] writes:

quoted

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 1435548e004..2d30eb5c17e 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh

@@ -84,6 +84,9 @@
 # single '?' character by setting GIT_PS1_COMPRESSSPARSESTATE, or omitted
 # by setting GIT_PS1_OMITSPARSESTATE.
 #
+# When there is a conflict, the prompt will include "|CONFLICT". This can
+# be omitted by setting GIT_PS1_OMITCONFLICTSTATE.
+#

It is unusual to subject our unsuspecting users to new features in a
way that is done by this patch.  A more usual practice, I think, is
to tell the users that they can set GIT_PS1_INCLUDECONFLICTSTATE to
"yes" if they want to opt in, and trigger the new feature only to
them.  Later, we may decide that the feature is useful and widely
apprlicable enough, at which time it may be turned on by default and
tell the users to set GIT_PS1_INCLUDECONFLICTSTATE to "no" if they
do not want to see it.  But one step at a time.

I see that most of the state indicators are disabled by default, so it
makes sense to be consistent. Should I make a variable with 'yes'/'no'
values, or set/unset? 'yes'/'no' has the benefit that if the default
is later changed, existing setups will continue to work. Set/unset
makes it harder to change the default later (I guess you could just
use a new variable with a different name), but is more consistent with
the other state indicators. I'm leaning towards set/unset, but it's
not a strong preference. Let me know if you disagree.

quoted

@@ -508,6 +511,12 @@ __git_ps1 ()
              r="$r $step/$total"
      fi

+     local conflict="" # state indicator for unresolved conflicts
+     if [[ -z "${GIT_PS1_OMITCONFLICTSTATE-}" ]] &&

And flipping the polarity and disabling it by default would be a
simple change, I would imagine, that can be made here.

quoted

+        [[ $(git ls-files --unmerged 2>/dev/null) ]]; then
+             conflict="|CONFLICT"
+     fi
+
      local w=""
      local i=""
      local s=""

@@ -572,7 +581,7 @@ __git_ps1 ()
      fi

      local f="$h$w$i$s$u$p"
-     local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}"
+     local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}${conflict}"

      if [ $pcmode = yes ]; then
              if [ "${__git_printf_supports_v-}" != yes ]; then

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 6a30f5719c3..9314776211c 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh

@@ -183,7 +183,7 @@ test_expect_success 'prompt - interactive rebase' '
 '

 test_expect_success 'prompt - rebase merge' '

Of course, this needs to be tweaked if we did so.  Running the test
in the default state will not need this change, but then ...

quoted

-     printf " (b2|REBASE 1/3)" >expected &&
+     printf " (b2|REBASE 1/3|CONFLICT)" >expected &&
      git checkout b2 &&
      test_when_finished "git checkout main" &&
      test_must_fail git rebase --merge b1 b2 &&

... a new test that runs the same thing but with the new feature
asked by setting the environment variable would show the new |CONFLICT
marker.

I'll add these.

Thanks.