All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Joakim Petersen <joak-pet@online.no>
Cc: git@vger.kernel.org, Justin Donnelly <justinrdonnelly@gmail.com>
Subject: Re: [RFC PATCH] git-prompt: make colourization consistent
Date: Wed, 01 Jun 2022 16:47:46 +0200	[thread overview]
Message-ID: <220601.864k141ls0.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20220601134414.66825-1-joak-pet@online.no>


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 <joak-pet@online.no>
> ---
> 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!
 

  reply	other threads:[~2022-06-01 14:49 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-01 13:44 [RFC PATCH] git-prompt: make colourization consistent Joakim Petersen
2022-06-01 14:47 ` Ævar Arnfjörð Bjarmason [this message]
2022-06-01 18:26   ` Joakim Petersen
2022-06-01 18:07 ` Junio C Hamano
2022-06-01 18:32   ` Joakim Petersen
2022-06-01 20:45     ` Junio C Hamano
2022-06-02 14:59 ` [PATCH v2] " Joakim Petersen
2022-06-02 21:56   ` joak-pet
2022-06-02 22:49   ` Junio C Hamano
2022-06-03 13:55     ` Joakim Petersen
2022-06-03 14:25   ` [PATCH v3] " Joakim Petersen
2022-06-03 16:38     ` Junio C Hamano
2022-06-03 17:23       ` Joakim Petersen
2022-06-03 18:51         ` Joakim Petersen
2022-06-03 19:43           ` Justin Donnelly
2022-06-03 21:16             ` Junio C Hamano
2022-06-04  9:42               ` Joakim Petersen
2022-06-06 16:13                 ` Junio C Hamano
2022-06-03 20:50         ` Junio C Hamano
2022-06-04 16:13     ` [PATCH v4] " Joakim Petersen
2022-06-04 17:30       ` Justin Donnelly
2022-06-04 19:18         ` Joakim Petersen
2022-06-04 19:26       ` [PATCH v5] " Joakim Petersen
2022-06-06  7:23         ` Bagas Sanjaya
2022-06-07 16:04           ` Junio C Hamano
2022-06-09 11:25             ` Joakim Petersen
2022-06-06 16:29         ` Junio C Hamano
2022-06-06 17:31           ` Joakim Petersen
2022-06-06 17:41             ` Junio C Hamano
2022-06-07 11:49               ` Joakim Petersen
2022-06-06 17:50         ` [PATCH v6] " Joakim Petersen
2022-06-07 11:50           ` [PATCH v7] " Joakim Petersen
2022-06-07 16:22             ` Junio C Hamano
2022-06-09 11:16               ` Joakim Petersen
2022-06-09  9:03             ` SZEDER Gábor
2022-06-09 11:13               ` Joakim Petersen
2022-06-09 18:29                 ` Junio C Hamano
2022-06-11  9:01                   ` SZEDER Gábor
2022-06-09 11:44             ` [PATCH v8] git-prompt: make colouring consistent Joakim Petersen
2022-06-09 20:44             ` [PATCH] git-prompt: fix expansion of branch colour codes Joakim Petersen
2022-06-10  0:05               ` Junio C Hamano
2022-06-10  0:33                 ` Joakim Petersen
2022-06-10  0:47               ` [PATCH v2] " Joakim Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=220601.864k141ls0.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=joak-pet@online.no \
    --cc=justinrdonnelly@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.