Subject: Re: [RFC PATCH] git-prompt: make colourization consistent

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 01 Jun 2022 16:47:46 +0200

On Wed, Jun 01 2022, Joakim Petersen wrote:

> 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. Make the colourization of these
> state indicators consistent by clearing any colour before printing the
> short upstream state indicator, as this immediately follows the last
> coloured indicator.
>
> Signed-off-by: Joakim Petersen <[email-masked]>
> ---
> As of 0ec7c23cdc6bde5af3039c59e21507adf7579a99, colourization of the
> output of __git_ps1 has changed such that the short upstream state
> indicator inherits the colour of the last short state indicator before
> it (if there is one), while before this change it was white/the default
> text colour. Some examples of what I mean are (assuming all indicators
> are enabled):
>  * If the local tree is clean and 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, 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 is that previously, the short upstream state indicator appeared
> immediately after the rebase/revert/bisect/merge state indicator, which
> is prepended with the clear colour code, while it now follows the
> sequence of colourized indicators, without any clearing of colour.
> However, adding a clearing of colour before the short upstream state
> indicator will change 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, it appears this colourization also was unintended.
>
>  contrib/completion/git-prompt.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index 87b2b916c0..dfd6cef35f 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -286,6 +286,7 @@ __git_ps1_colorize_gitstring ()
>  	if [ -n "$u" ]; then
>  		u="$bad_color$u"
>  	fi
> +	p="$c_clear$p"
>  	r="$c_clear$r"
>  }
>  
>
> base-commit: e54793a95afeea1e10de1e5ad7eab914e7416250

This seems to make sense to me, but I haven't looked deeply into it. But
let's CC the author of 0ec7c23cdc6 (git-prompt: make upstream state
indicator location consistent, 2022-02-27) (which I've done here).

For a non-RFC patch I think a rephrasing of most of what yo uhave below
"--" should be part of the message. Note how I referred to the
0ec... commit above, you should reference the commit like that (see
SubmittingPatches).

Thanks for working on this fix!