All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Justin Donnelly <justinrdonnelly@gmail.com>
Cc: "Joakim Petersen" <joak-pet@online.no>,
	git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH v3] git-prompt: make colourization consistent
Date: Fri, 03 Jun 2022 14:16:37 -0700	[thread overview]
Message-ID: <xmqqwndxcuru.fsf@gitster.g> (raw)
In-Reply-To: <CAGTqyRxkiGt7CRggV7VeXNRK2VmDMxDX3EpOr5cPcc5AdH8ZaA@mail.gmail.com> (Justin Donnelly's message of "Fri, 3 Jun 2022 15:43:11 -0400")

Justin Donnelly <justinrdonnelly@gmail.com> writes:

> Hi all. I'm the author of 0ec7c23c (git-prompt: make upstream state
> indicator location consistent, 2022-02-27). Sorry I'm a little late
> jumping in. I was also going to propose something more comprehensive
> and future-proof than what's there (adding the applicable color
> (including clear) to all the indicators), but I like Junio's idea
> better. The only other thing I have to add is that it's probably a
> good idea to include a comment in the function
> `__git_ps1_colorize_gitstring` explaining the design so future
> developers/reviewers know.

After thinking it again, I actually am OK with the original coloring
code structure.  The rule is "you always counter whatever color
settings left behind by somebody who came before you".

As long as the color effect you use is not additive (e.g. if the
final product is $a$b, and $a is prefixed with $c_red and $b is
prefixed with $c_blue, an additive coloring scheme may end up
painting b in purple), we'll save number of $c_clear we would need
to emit.  Plain colors are probably not additive, but some
attributes are, so this is more brittle than "always reset to the
base state" rule, but it may be more desirable in practice.

I have no strong preference either way.  But if we are to go that
route, we definitely need to make sure that the last element added
to gitstring is followed by $c_reset, by doing something like the
attached patch.  Currently, $r has unconditional $c_clear in front
of it, and $upstream is never colored, and that is the only thing
that is saving us from leftover color bleeding into whatever comes
after the prompt.

 contrib/completion/git-prompt.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git c/contrib/completion/git-prompt.sh w/contrib/completion/git-prompt.sh
index 87b2b916c0..c803b9fae5 100644
--- c/contrib/completion/git-prompt.sh
+++ w/contrib/completion/git-prompt.sh
@@ -287,6 +287,7 @@ __git_ps1_colorize_gitstring ()
 		u="$bad_color$u"
 	fi
 	r="$c_clear$r"
+	end_of_gitstring=$c_clear
 }
 
 # Helper function to read the first line of a file into a variable.
@@ -556,6 +557,7 @@ __git_ps1 ()
 
 	local z="${GIT_PS1_STATESEPARATOR-" "}"
 
+	local end_of_gitstring=
 	# NO color option unless in PROMPT_COMMAND mode or it's Zsh
 	if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
 		if [ $pcmode = yes ] || [ -n "${ZSH_VERSION-}" ]; then
@@ -570,7 +572,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}${end_of_gitstring}"
 
 	if [ $pcmode = yes ]; then
 		if [ "${__git_printf_supports_v-}" != yes ]; then

  reply	other threads:[~2022-06-03 21:16 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
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 [this message]
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=xmqqwndxcuru.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=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.