From: joak-pet@online.no
Date: Thu, 02 Jun 2022 21:56:02 GMT
Subject: Re: [PATCH v2] git-prompt: make colourization consistent
Message-ID: <db1c01f8413fbbfa3e19755afdec4f71@online.no>
In-Reply-To: <20220602145935.10512-1-joak-pet@online.no>

On 02/06/2022 16:59, 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.
> 
> As of 0ec7c23cdc6 (git-prompt: make upstream state indicator location
> consistent, 2022-02-27), colourization in 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
> to illustrate this behaviour (assuming all indicators are enabled and
> colourization is on):
>  * 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 (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}"
> 
> 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, 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.
> 
> Signed-off-by: Joakim Petersen <joak-pet@online.no>
> ---
> 
> Range-diff against v1:
> 1:  e235caa7a8 = 1:  e235caa7a8 git-prompt: make colourization 
> consistent
> 
>  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"
>  }

I just realized I forgot to write what changed between the RFC patch
and v2:

  * Clarify the reason why 0ec7c23cdc6 (git-prompt: make upstream state
    indicator location consistent, 2022-02-27) changed the colourization
    of the short upstream state indicator.
  * Explain the rationale for changing the sparsity state colourization.
  * Include examples of how the short upstream state indicator is
    currently colourized