git-prompt: make colourization consistent

The short upstream state indicator inherits the colour of the last short state indicator before it (if there is one), and the sparsity state indicator inherits this colour as well. This behaviour was introduced by 0ec7c23cdc6 (git-prompt: make upstream state indicator location consistent, 2022-02-27), while before this change the aforementioned indicators were white/the default text colour. Some examples to illustrate this behaviour (assuming all indicators are enabled and colourization is on): * If there is something in the stash, both the '$' and the short upstream state indicator following it will be blue. * If the local tree has new, untracked files and there is nothing in the stash, both the '%' and the short upstream state indicator will be red. * If all local changes are added to the index and the stash is empty, both the '+' and the short upstream state indicator following it will be green. * If the local tree is clean and there is nothing in the stash, the short upstream state indicator will be white/${default text colour}. This appears to be an unintended side-effect of the change, and makes little sense semantically (e.g. why is it bad to be in sync with upstream when you have uncommitted local changes?). The cause of the change in colourization is that previously, the short upstream state indicator appeared immediately after the rebase/revert/bisect/merge state indicator (note the position of $p in $gitstring): local f="$h$w$i$s$u" local gitstring="$c$b${f:+$z$f}${sparse}$r$p" Said indicator is prepended with the clear colour code, and the short upstream state indicator is thus also uncoloured. Now, the short upstream state indicator follows the sequence of colourized indicators, without any clearing of colour (again note the position of $p, now in $f): local f="$h$w$i$s$u$p" local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}" If the user is in a sparse checkout, the sparsity state indicator follows a similar pattern to the short upstream state indicator. However, clearing colour of the colourized indicators changes how the sparsity state indicator is colourized, as it currently inherits (and before the change referenced also inherited) the colour of the last short state indicator before it. Reading the commit message of the change that introduced the sparsity state indicator, afda36dbf3b (git-prompt: include sparsity state as well, 2020-06-21), it appears this colourization also was unintended, so clearing the colour for said indicator further increases consistency. Make the colourization of these state indicators consistent by making all colourized indicators clear their own colour. Make colouring of $c dependent on it not being empty, as it is no longer being used to colour the branch name. Move clearing of $b's prefix to before colourization so it gets cleared properly when colour codes are inserted into it. These changes make changing the layout of the prompt less prone to unintended colour changes in the future. Change coloured Bash prompt tests to reflect the colourization changes: * Move the colour codes to wrap the expected content of the expanded $__git_ps1_branch_name in all tests. * Insert a clear-colour code after the symbol for the first indicator in "prompt - bash color pc mode - dirty status indicator - dirty index and worktree", to reflect that all indicators should clear their own colour. Signed-off-by: Joakim Petersen <joak-pet@online.no> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Joakim Petersen committed Jun 7, 2022 at 13:50 UTC 9470605a1b03dac8fc4f801132e36964b4fbb8c3
2 files changed +21 -19
contrib/completion/git-prompt.sh
+12 -10
@@ -245,7 +245,8 @@ __git_ps1_show_upstream ()
245
246 # Helper function that is meant to be called from __git_ps1. It
247 # injects color codes into the appropriate gitstring variables used
248 -# to build a gitstring.
248 +# to build a gitstring. Colored variables are responsible for clearing
249 +# their own color.
250 __git_ps1_colorize_gitstring ()
251 {
252 if [[ -n ${ZSH_VERSION-} ]]; then
@@ -271,22 +272,23 @@ __git_ps1_colorize_gitstring ()
272 else
273 branch_color="$bad_color"
274 fi
274 - c="$branch_color$c"
275 + if [ -n "$c" ]; then
276 + c="$branch_color$c$c_clear"
277 + fi
278 + b="$branch_color$b$c_clear"
279
276 - z="$c_clear$z"
277 - if [ "$w" = "*" ]; then
278 - w="$bad_color$w"
280 + if [ -n "$w" ]; then
281 + w="$bad_color$w$c_clear"
282 fi
283 if [ -n "$i" ]; then
281 - i="$ok_color$i"
284 + i="$ok_color$i$c_clear"
285 fi
286 if [ -n "$s" ]; then
284 - s="$flags_color$s"
287 + s="$flags_color$s$c_clear"
288 fi
289 if [ -n "$u" ]; then
287 - u="$bad_color$u"
290 + u="$bad_color$u$c_clear"
291 fi
289 - r="$c_clear$r"
292 }
293
294 # Helper function to read the first line of a file into a variable.
@@ -554,6 +556,7 @@ __git_ps1 ()
556 fi
557 fi
558
559 + b=${b##refs/heads/}
560 local z="${GIT_PS1_STATESEPARATOR-" "}"
561
562 # NO color option unless in PROMPT_COMMAND mode or it's Zsh
@@ -563,7 +566,6 @@ __git_ps1 ()
566 fi
567 fi
568
566 - b=${b##refs/heads/}
569 if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
570 __git_ps1_branch_name=$b
571 b="\${__git_ps1_branch_name}"
t/t9903-bash-prompt.sh
+9 -9
@@ -541,7 +541,7 @@ test_expect_success 'prompt - pc mode' '
541 '
542
543 test_expect_success 'prompt - bash color pc mode - branch name' '
544 - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmain" >expected &&
544 + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}main${c_clear}" >expected &&
545 (
546 GIT_PS1_SHOWCOLORHINTS=y &&
547 __git_ps1 "BEFORE:" ":AFTER" >"$actual" &&
@@ -551,7 +551,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' '
551 '
552
553 test_expect_success 'prompt - bash color pc mode - detached head' '
554 - printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected &&
554 + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_red}(%s...)"${c_clear} $(git log -1 --format="%h" b1^) >expected &&
555 git checkout b1^ &&
556 test_when_finished "git checkout main" &&
557 (
@@ -563,7 +563,7 @@ test_expect_success 'prompt - bash color pc mode - detached head' '
563 '
564
565 test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' '
566 - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmain" >expected &&
566 + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected &&
567 echo "dirty" >file &&
568 test_when_finished "git reset --hard" &&
569 (
@@ -576,7 +576,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt
576 '
577
578 test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' '
579 - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmain" >expected &&
579 + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected &&
580 echo "dirty" >file &&
581 test_when_finished "git reset --hard" &&
582 git add -u &&
@@ -590,7 +590,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt
590 '
591
592 test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' '
593 - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmain" >expected &&
593 + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected &&
594 echo "dirty index" >file &&
595 test_when_finished "git reset --hard" &&
596 git add -u &&
@@ -605,7 +605,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt
605 '
606
607 test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' '
608 - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmain" >expected &&
608 + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}#${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected &&
609 (
610 GIT_PS1_SHOWDIRTYSTATE=y &&
611 GIT_PS1_SHOWCOLORHINTS=y &&
@@ -617,7 +617,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - befo
617 '
618
619 test_expect_success 'prompt - bash color pc mode - inside .git directory' '
620 - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected &&
620 + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}GIT_DIR!${c_clear}" >expected &&
621 echo "dirty" >file &&
622 test_when_finished "git reset --hard" &&
623 (
@@ -631,7 +631,7 @@ test_expect_success 'prompt - bash color pc mode - inside .git directory' '
631 '
632
633 test_expect_success 'prompt - bash color pc mode - stash status indicator' '
634 - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmain" >expected &&
634 + printf "BEFORE: (\${__git_ps1_branch_name} ${c_lblue}\$${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected &&
635 echo 2 >file &&
636 git stash &&
637 test_when_finished "git stash drop" &&
@@ -645,7 +645,7 @@ test_expect_success 'prompt - bash color pc mode - stash status indicator' '
645 '
646
647 test_expect_success 'prompt - bash color pc mode - untracked files status indicator' '
648 - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmain" >expected &&
648 + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}%%${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected &&
649 (
650 GIT_PS1_SHOWUNTRACKEDFILES=y &&
651 GIT_PS1_SHOWCOLORHINTS=y &&