* [RFC PATCH] git-prompt: make colourization consistent @ 2022-06-01 13:44 Joakim Petersen 2022-06-01 14:47 ` Ævar Arnfjörð Bjarmason ` (2 more replies) 0 siblings, 3 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-01 13:44 UTC (permalink / raw) To: git; +Cc: Joakim Petersen 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 -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [RFC PATCH] git-prompt: make colourization consistent 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-02 14:59 ` [PATCH v2] " Joakim Petersen 2 siblings, 1 reply; 43+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2022-06-01 14:47 UTC (permalink / raw) To: Joakim Petersen; +Cc: git, Justin Donnelly 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! ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [RFC PATCH] git-prompt: make colourization consistent 2022-06-01 14:47 ` Ævar Arnfjörð Bjarmason @ 2022-06-01 18:26 ` Joakim Petersen 0 siblings, 0 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-01 18:26 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git On 01/06/2022 16:47, Ævar Arnfjörð Bjarmason wrote: > 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! > > Thanks for the pointers, I'll keep that in mind for the follow-up! I do have one question regarding the procedure for the follow-up, though: If there are no code changes, should it still be submitted as a "v2"? ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [RFC PATCH] git-prompt: make colourization consistent 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:07 ` Junio C Hamano 2022-06-01 18:32 ` Joakim Petersen 2022-06-02 14:59 ` [PATCH v2] " Joakim Petersen 2 siblings, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-01 18:07 UTC (permalink / raw) To: Joakim Petersen; +Cc: git Joakim Petersen <joak-pet@online.no> writes: > 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> > --- > 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" > } Hmph, am I correct to understand that the general flow of __git_ps1 is (1) various pieces of information like $h, $w, $i, $s, $r, $b, $p, etc. are declared "local" and values computed for them, either inside __git_ps1() itself, or by various helper functions it calls; (2) When GIT_PS1_SHOWCOLORHINTS is in effect, we may call the __git_ps1_colorize_gitstring helper (which is touched by the above hunk), that modifies these variables with color codes. Upon entry to this helper function, these variables prepared in (1) have no color effects. Upon leaving, they do. (3) Finally, the PS1 is asseembled by concatenating these variables, whose text was prepared in (1) and then prefixed by color codes in (2), one of the earliest steps begins like so: local f="$h$w$i$s$u" local gitstring="$c$b${f:+$z$f}${sparse}$r$p" In the final step of formulation, $p immediately follows $r in the resulting $PS1, and the existing code at the end of the (2) prefixes $c_clear before $r, and $r before such prefixing is free of coloring, so it is curious how this patch makes difference (other than emitting $c_clear one more time). Unless there is a use of $p that does not immediately follow $r, that is. Thanks. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [RFC PATCH] git-prompt: make colourization consistent 2022-06-01 18:07 ` Junio C Hamano @ 2022-06-01 18:32 ` Joakim Petersen 2022-06-01 20:45 ` Junio C Hamano 0 siblings, 1 reply; 43+ messages in thread From: Joakim Petersen @ 2022-06-01 18:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 01/06/2022 20:07, Junio C Hamano wrote: > Hmph, am I correct to understand that the general flow of __git_ps1 is > > (1) various pieces of information like $h, $w, $i, $s, $r, $b, $p, > etc. are declared "local" and values computed for them, > either inside __git_ps1() itself, or by various helper > functions it calls; > > (2) When GIT_PS1_SHOWCOLORHINTS is in effect, we may call the > __git_ps1_colorize_gitstring helper (which is touched by the > above hunk), that modifies these variables with color codes. > Upon entry to this helper function, these variables prepared in > (1) have no color effects. Upon leaving, they do. > > (3) Finally, the PS1 is asseembled by concatenating these > variables, whose text was prepared in (1) and then prefixed by > color codes in (2), one of the earliest steps begins like so: > > local f="$h$w$i$s$u" > local gitstring="$c$b${f:+$z$f}${sparse}$r$p" > > In the final step of formulation, $p immediately follows $r in the > resulting $PS1, and the existing code at the end of the (2) prefixes > $c_clear before $r, and $r before such prefixing is free of coloring, > so it is curious how this patch makes difference (other than emitting > $c_clear one more time). Unless there is a use of $p that does not > immediately follow $r, that is. > > Thanks. > Your understanding is correct for the flow before the change I referenced (0ec7c23cdc6 (git-prompt: make upstream state indicator location consistent, 2022-02-27)), however, that commit changed the definition of $f and $gitstring to local f="$h$w$i$s$u$p" local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}" This makes it so $p is no longer immediately preceded by $r, but rather $u, which, like all the preceding variables, except $h, will be colourized if enabled. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [RFC PATCH] git-prompt: make colourization consistent 2022-06-01 18:32 ` Joakim Petersen @ 2022-06-01 20:45 ` Junio C Hamano 0 siblings, 0 replies; 43+ messages in thread From: Junio C Hamano @ 2022-06-01 20:45 UTC (permalink / raw) To: Joakim Petersen; +Cc: git Joakim Petersen <joak-pet@online.no> writes: > Your understanding is correct for the flow before the change I > referenced (0ec7c23cdc6 (git-prompt: make upstream state > indicator location consistent, 2022-02-27)), however, that commit > changed the definition of $f and $gitstring to > > local f="$h$w$i$s$u$p" > local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}" > > This makes it so $p is no longer immediately preceded by $r, but rather > $u, which, like all the preceding variables, except $h, will be > colourized if enabled. Ah, OK. With the above explanation, the change does make sense. The mention of that commit does need to be in the proposed log message, not under the three-dash line, as it is essential to understand why the patch is not a no-op change. Thanks. ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2] git-prompt: make colourization consistent 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:07 ` Junio C Hamano @ 2022-06-02 14:59 ` Joakim Petersen 2022-06-02 21:56 ` joak-pet ` (2 more replies) 2 siblings, 3 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-02 14:59 UTC (permalink / raw) To: git Cc: Joakim Petersen, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly 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" } -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v2] git-prompt: make colourization consistent 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 14:25 ` [PATCH v3] " Joakim Petersen 2 siblings, 0 replies; 43+ messages in thread From: joak-pet @ 2022-06-02 21:56 UTC (permalink / raw) To: git Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] git-prompt: make colourization consistent 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 2 siblings, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-02 22:49 UTC (permalink / raw) To: Joakim Petersen Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly Joakim Petersen <joak-pet@online.no> writes: > 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" > } Has this been tested? It seems to break a handful of tests in t9903 for me. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] git-prompt: make colourization consistent 2022-06-02 22:49 ` Junio C Hamano @ 2022-06-03 13:55 ` Joakim Petersen 0 siblings, 0 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-03 13:55 UTC (permalink / raw) To: Junio C Hamano Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly On 03/06/2022 00:49, Junio C Hamano wrote: > Joakim Petersen <joak-pet@online.no> writes: > >> 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" >> } > > Has this been tested? > > It seems to break a handful of tests in t9903 for me. > Oh, no I hadn't run the test suite, sorry. I must've gotten too caught up in other parts of the submitting process and simply forgot to run them. After looking into it, the reason why the tests fail is that $p is no longer empty when it is not set, so $f is no longer empty, leading to both $z and $p being inserted into $gitstring. This causes there to be three clear-colour sequences in the final output instead of the expected one. Wrapping the clearing of $p's colour in a check for emptiness like the other short state indicators fixes the tests. I'll submit a v3 shortly. ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v3] git-prompt: make colourization consistent 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 14:25 ` Joakim Petersen 2022-06-03 16:38 ` Junio C Hamano 2022-06-04 16:13 ` [PATCH v4] " Joakim Petersen 2 siblings, 2 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-03 14:25 UTC (permalink / raw) To: git Cc: Joakim Petersen, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly 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}" If the user is in a sparse checkout, the sparsity state indicator follows a similar pattern to the short upstream state indicator.However, adding a clearing of colour before the short upstream state indicator will change how the sparsity state indicator is colourized only when the short upstream state indicator is present, 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> --- Changes since v2: * Wrapped clearing of $p's colour in a check for emptiness to avoid multiple colour clears in the final gitstring. * Added clearing of colour for $sparse, as it wouldn't acutally be cleared consistently with only the change from the previous bullet - Having the short upstream state indicator disabled would leave the sparse state indicator as it is without this patch. Range-diff against v2: 1: e235caa7a8 < -: ---------- git-prompt: make colourization consistent -: ---------- > 1: 0e107d0496 git-prompt: make colourization consistent contrib/completion/git-prompt.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 87b2b916c0..4997545ee5 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -286,6 +286,12 @@ __git_ps1_colorize_gitstring () if [ -n "$u" ]; then u="$bad_color$u" fi + if [ -n "$p" ]; then + p="$c_clear$p" + fi + if [ -n "$sparse" ]; then + sparse="$c_clear$sparse" + fi r="$c_clear$r" } -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v3] git-prompt: make colourization consistent 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-04 16:13 ` [PATCH v4] " Joakim Petersen 1 sibling, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-03 16:38 UTC (permalink / raw) To: Joakim Petersen Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly This is not a new issue, but seeing this: if [ $detached = no ]; then branch_color="$ok_color" else branch_color="$bad_color" fi c="$branch_color$c" z="$c_clear$z" if [ "$w" = "*" ]; then w="$bad_color$w" fi if [ -n "$i" ]; then i="$ok_color$i" fi if [ -n "$s" ]; then s="$flags_color$s" fi if [ -n "$u" ]; then u="$bad_color$u" fi + if [ -n "$p" ]; then + p="$c_clear$p" + fi + if [ -n "$sparse" ]; then + sparse="$c_clear$sparse" + fi r="$c_clear$r" } it makes me wonder if the more forward looking and future-proof way that is resistant to any future and random reshuffling like what 0ec7c23c (git-prompt: make upstream state indicator location consistent, 2022-02-27) did would be to make it a rule to maintain that there is no coloring by default, and when any of these tokens like w, i, s, ... are not empty, enclose them inside "color-on" and "color-off" sequence. For example, if [ "$w" = "*" ]; then w="$bad_color$w" fi would mean $w, when it is "*", would cause gitstring to contain an asterisk that is painted in $bad_color, but ALSO causes whatever that happens to come AFTER $w in gitstring to be painted in the same color UNLESS it tries to protect itself. Right now, $w may be immediately followed by $i, and $i does protect itself by prefixing with $ok_color, but if $i is empty, $w's coloring will extend to $s. So, if we did this instead: - z="$c_clear$z" if [ "$w" = "*" ]; then - w="$bad_color$w" + w="$bad_color$w$c_clear" fi and make similar changes to everything else we see above, we probably can lose the ones that prefix with $c_clear, because each token that paints itself in unusual color is now responsible for returning the terminal state to normal with the $c_clear sequence after it is done with it. We do not have to special case sparse, p, or r in this helper function at all if we go that route, no? If the helper were written that way, then reshuffling the order of the tokens done in 0ec7c23c (git-prompt: make upstream state indicator location consistent, 2022-02-27) wouldn't have made the patch under discussion necessary at all, which is what I see is valuable from the "maintainability" point of view. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v3] git-prompt: make colourization consistent 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 20:50 ` Junio C Hamano 0 siblings, 2 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-03 17:23 UTC (permalink / raw) To: Junio C Hamano Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly On 03/06/2022 18:38, Junio C Hamano wrote: > This is not a new issue, but seeing this: > > if [ $detached = no ]; then > branch_color="$ok_color" > else > branch_color="$bad_color" > fi > c="$branch_color$c" > > z="$c_clear$z" > if [ "$w" = "*" ]; then > w="$bad_color$w" > fi > if [ -n "$i" ]; then > i="$ok_color$i" > fi > if [ -n "$s" ]; then > s="$flags_color$s" > fi > if [ -n "$u" ]; then > u="$bad_color$u" > fi > + if [ -n "$p" ]; then > + p="$c_clear$p" > + fi > + if [ -n "$sparse" ]; then > + sparse="$c_clear$sparse" > + fi > r="$c_clear$r" > } > > it makes me wonder if the more forward looking and future-proof way > that is resistant to any future and random reshuffling like what > 0ec7c23c (git-prompt: make upstream state indicator location > consistent, 2022-02-27) did would be to make it a rule to maintain > that there is no coloring by default, and when any of these tokens > like w, i, s, ... are not empty, enclose them inside "color-on" and > "color-off" sequence. > > For example, > > if [ "$w" = "*" ]; then > w="$bad_color$w" > fi > > would mean $w, when it is "*", would cause gitstring to contain an > asterisk that is painted in $bad_color, but ALSO causes whatever > that happens to come AFTER $w in gitstring to be painted in the same > color UNLESS it tries to protect itself. Right now, $w may be > immediately followed by $i, and $i does protect itself by prefixing > with $ok_color, but if $i is empty, $w's coloring will extend to $s. > > So, if we did this instead: > > - z="$c_clear$z" > if [ "$w" = "*" ]; then > - w="$bad_color$w" > + w="$bad_color$w$c_clear" > fi > > and make similar changes to everything else we see above, we > probably can lose the ones that prefix with $c_clear, because each > token that paints itself in unusual color is now responsible for > returning the terminal state to normal with the $c_clear sequence > after it is done with it. We do not have to special case sparse, p, > or r in this helper function at all if we go that route, no? > > If the helper were written that way, then reshuffling the order of > the tokens done in 0ec7c23c (git-prompt: make upstream state > indicator location consistent, 2022-02-27) wouldn't have made the > patch under discussion necessary at all, which is what I see is > valuable from the "maintainability" point of view. > That does seem like a much better idea for maintainability, I can change the patch to do this instead. I have one question, though: the sequence $c$b (bare state and branch name) is a special case, where they're intended to have the same colour, should I wrap both in colour set, colour clear, or only clear after $b? The former requires rewriting the tests or changing $gitstring to not include $c when $c is empty, while the latter keeps the tests unchanged, but may pose a problem if "BARE:" should at any point not appear immediately before the branch name. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v3] git-prompt: make colourization consistent 2022-06-03 17:23 ` Joakim Petersen @ 2022-06-03 18:51 ` Joakim Petersen 2022-06-03 19:43 ` Justin Donnelly 2022-06-03 20:50 ` Junio C Hamano 1 sibling, 1 reply; 43+ messages in thread From: Joakim Petersen @ 2022-06-03 18:51 UTC (permalink / raw) To: Junio C Hamano Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly On 03/06/2022 19:23, Joakim Petersen wrote: > That does seem like a much better idea for maintainability, I can > change the patch to do this instead. I have one question, though: the > sequence $c$b (bare state and branch name) is a special case, where > they're intended to have the same colour, should I wrap both in colour > set, colour clear, or only clear after $b? The former requires rewriting > the tests or changing $gitstring to not include $c when $c is empty, > while the latter keeps the tests unchanged, but may pose a problem if > "BARE:" should at any point not appear immediately before the branch > name. Sorry, the former (colourizing and clearing $c and $b individually) requires rewriting tests no matter what. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v3] git-prompt: make colourization consistent 2022-06-03 18:51 ` Joakim Petersen @ 2022-06-03 19:43 ` Justin Donnelly 2022-06-03 21:16 ` Junio C Hamano 0 siblings, 1 reply; 43+ messages in thread From: Justin Donnelly @ 2022-06-03 19:43 UTC (permalink / raw) To: Joakim Petersen Cc: Junio C Hamano, git, Ævar Arnfjörð Bjarmason 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. Thanks, Justin On Fri, Jun 3, 2022 at 2:52 PM Joakim Petersen <joak-pet@online.no> wrote: > > On 03/06/2022 19:23, Joakim Petersen wrote: > > That does seem like a much better idea for maintainability, I can > > change the patch to do this instead. I have one question, though: the > > sequence $c$b (bare state and branch name) is a special case, where > > they're intended to have the same colour, should I wrap both in colour > > set, colour clear, or only clear after $b? The former requires rewriting > > the tests or changing $gitstring to not include $c when $c is empty, > > while the latter keeps the tests unchanged, but may pose a problem if > > "BARE:" should at any point not appear immediately before the branch > > name. > > Sorry, the former (colourizing and clearing $c and $b individually) > requires rewriting tests no matter what. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v3] git-prompt: make colourization consistent 2022-06-03 19:43 ` Justin Donnelly @ 2022-06-03 21:16 ` Junio C Hamano 2022-06-04 9:42 ` Joakim Petersen 0 siblings, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-03 21:16 UTC (permalink / raw) To: Justin Donnelly Cc: Joakim Petersen, git, Ævar Arnfjörð Bjarmason 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v3] git-prompt: make colourization consistent 2022-06-03 21:16 ` Junio C Hamano @ 2022-06-04 9:42 ` Joakim Petersen 2022-06-06 16:13 ` Junio C Hamano 0 siblings, 1 reply; 43+ messages in thread From: Joakim Petersen @ 2022-06-04 9:42 UTC (permalink / raw) To: Junio C Hamano, Justin Donnelly Cc: git, Ævar Arnfjörð Bjarmason On 03/06/2022 23:16, Junio C Hamano wrote: > 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. There might be something I'm not seeing, but having it so each element counters whatever colour left by the preceding element seems less intuitive when adding or moving elements in the final $gitstring. Adding an element will then require going into __git_ps1_colorize_gitstring, even when it is not intended to be colourized. All existing uncoloured elements will also need to be prefixed to protect against colour bleed from being moved around. I'm partial to the idea of each coloured element clearing its own colour. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v3] git-prompt: make colourization consistent 2022-06-04 9:42 ` Joakim Petersen @ 2022-06-06 16:13 ` Junio C Hamano 0 siblings, 0 replies; 43+ messages in thread From: Junio C Hamano @ 2022-06-06 16:13 UTC (permalink / raw) To: Joakim Petersen Cc: Justin Donnelly, git, Ævar Arnfjörð Bjarmason Joakim Petersen <joak-pet@online.no> writes: > There might be something I'm not seeing, but having it so each element > counters whatever colour left by the preceding element seems less > intuitive when adding or moving elements in the final $gitstring. Adding > an element will then require going into __git_ps1_colorize_gitstring, > even when it is not intended to be colourized. All existing uncoloured > elements will also need to be prefixed to protect against colour bleed > from being moved around. I'm partial to the idea of each coloured > element clearing its own colour. I think that each makes sense in its own way. Depending on what assumption we can make on the use of terminal attributes, one can produce shorter output than the other. For example, if you have 3 things, A, B, and C, that are shown in this order, the "clear after yourselves" scheme would give gitstring=<red>A<clear><blue>B<clear><green>C<clear> while "clear the slate for yourself before you draw, the framework will clear the effect of the last one" scheme can give gitstring=<red>A<blue>B<green>C<clear> if we know that no additive terminal attributes are used, and the latter gives a shorter output. If we need to support some additive ones (like "reverse"), on the other hand, and if each element is independent (i.e. "clear the slate for me" cannot use the knowledge of what the previous one did), then we have to write gitstring=<red>A<clear><blue>B<clear><green>C<clear> for the latter, which becomes more verbose (but is the same as the "each is on its own, clear after yourselves" version). I have no strong preference either way myself. "each on its own" might be conceptually simpler and easier to understand and explain what is going on. Thanks. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v3] git-prompt: make colourization consistent 2022-06-03 17:23 ` Joakim Petersen 2022-06-03 18:51 ` Joakim Petersen @ 2022-06-03 20:50 ` Junio C Hamano 1 sibling, 0 replies; 43+ messages in thread From: Junio C Hamano @ 2022-06-03 20:50 UTC (permalink / raw) To: Joakim Petersen Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly Joakim Petersen <joak-pet@online.no> writes: > That does seem like a much better idea for maintainability, I can > change the patch to do this instead. I have one question, though: the > sequence $c$b (bare state and branch name) is a special case, where > they're intended to have the same colour, should I wrap both in colour > set, colour clear, or only clear after $b? If we want to allow $c and $b appear in different places (which I have no opinion on), I would say we should just color them independently and fix the test that expects the close linkage between the two. I offhand see no reason that they _must_ stay together myself, though. Thanks. ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v4] git-prompt: make colourization consistent 2022-06-03 14:25 ` [PATCH v3] " Joakim Petersen 2022-06-03 16:38 ` Junio C Hamano @ 2022-06-04 16:13 ` Joakim Petersen 2022-06-04 17:30 ` Justin Donnelly 2022-06-04 19:26 ` [PATCH v5] " Joakim Petersen 1 sibling, 2 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-04 16:13 UTC (permalink / raw) To: git Cc: Joakim Petersen, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly 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 making all colourized indicators clear their own colour. 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 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 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. Colouring of $c was made dependent on it not being empty, as it is no longer being used to colour the branch name. Removal of $b's prefix was moved to before the colourization so it gets cleared properly now that colour codes are inserted into it. Due to colour clearing being moved into the variables for each coloured indicator, the tests for the coloured Bash prompt had to be changed: * All colour tests now have the colour codes around the expected content of the expanded $__git_ps1_branch_name variable instead of the unexpanded variable in the string. * The test with two indicators had a clear-colour code inserted after the symbol for the first indicator, since all indicators clear their own colours now. Signed-off-by: Joakim Petersen <joak-pet@online.no> --- Changes since v3: * All colourized variables now also clear their own colour. * Variables are only coloured if they are not empty, except $b (branch name), which is not an optional indicator. * Updated tests to reflect the new colourization behaviour. * Fixed a mistake in two of the examples; the stash indicator is the last of the short state indicators preceding the short upstream state indicator. Range-diff against v3: 1: 0e107d0496 < -: ---------- git-prompt: make colourization consistent -: ---------- > 1: 98ce78ddc5 git-prompt: make colourization consistent contrib/completion/git-prompt.sh | 20 +++++++++++--------- t/t9903-bash-prompt.sh | 18 +++++++++--------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 87b2b916c0..32bb98bb8d 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -245,7 +245,8 @@ __git_ps1_show_upstream () # Helper function that is meant to be called from __git_ps1. It # injects color codes into the appropriate gitstring variables used -# to build a gitstring. +# to build a gitstring. Colored variables are responsible for clearing +# their own color. __git_ps1_colorize_gitstring () { if [[ -n ${ZSH_VERSION-} ]]; then @@ -271,22 +272,23 @@ __git_ps1_colorize_gitstring () else branch_color="$bad_color" fi - c="$branch_color$c" + if [ -n "$c" ]; then + c="$branch_color$c$c_clear" + fi + b="$branch_color$b$c_clear" - z="$c_clear$z" if [ "$w" = "*" ]; then - w="$bad_color$w" + w="$bad_color$w$c_clear" fi if [ -n "$i" ]; then - i="$ok_color$i" + i="$ok_color$i$c_clear" fi if [ -n "$s" ]; then - s="$flags_color$s" + s="$flags_color$s$c_clear" fi if [ -n "$u" ]; then - u="$bad_color$u" + u="$bad_color$u$c_clear" fi - r="$c_clear$r" } # Helper function to read the first line of a file into a variable. @@ -554,6 +556,7 @@ __git_ps1 () fi fi + b=${b##refs/heads/} local z="${GIT_PS1_STATESEPARATOR-" "}" # NO color option unless in PROMPT_COMMAND mode or it's Zsh @@ -563,7 +566,6 @@ __git_ps1 () fi fi - b=${b##refs/heads/} if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then __git_ps1_branch_name=$b b="\${__git_ps1_branch_name}" diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index bbd513bab0..abd82eec35 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -541,7 +541,7 @@ test_expect_success 'prompt - pc mode' ' ' test_expect_success 'prompt - bash color pc mode - branch name' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWCOLORHINTS=y && __git_ps1 "BEFORE:" ":AFTER" >"$actual" && @@ -551,7 +551,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' ' ' test_expect_success 'prompt - bash color pc mode - detached head' ' - printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_red}(%s...)"${c_clear} $(git log -1 --format="%h" b1^) >expected && git checkout b1^ && test_when_finished "git checkout main" && ( @@ -563,7 +563,7 @@ test_expect_success 'prompt - bash color pc mode - detached head' ' ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -576,7 +576,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && git add -u && @@ -590,7 +590,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty index" >file && test_when_finished "git reset --hard" && git add -u && @@ -605,7 +605,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}#${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWDIRTYSTATE=y && GIT_PS1_SHOWCOLORHINTS=y && @@ -617,7 +617,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - befo ' test_expect_success 'prompt - bash color pc mode - inside .git directory' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}GIT_DIR!${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -631,7 +631,7 @@ test_expect_success 'prompt - bash color pc mode - inside .git directory' ' ' test_expect_success 'prompt - bash color pc mode - stash status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_lblue}\$${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo 2 >file && git stash && test_when_finished "git stash drop" && @@ -645,7 +645,7 @@ test_expect_success 'prompt - bash color pc mode - stash status indicator' ' ' test_expect_success 'prompt - bash color pc mode - untracked files status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}%%${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWUNTRACKEDFILES=y && GIT_PS1_SHOWCOLORHINTS=y && -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v4] git-prompt: make colourization consistent 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 1 sibling, 1 reply; 43+ messages in thread From: Justin Donnelly @ 2022-06-04 17:30 UTC (permalink / raw) To: Joakim Petersen Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano On Sat, Jun 4, 2022 at 12:13 PM Joakim Petersen <joak-pet@online.no> 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 making all colourized indicators clear > their own colour. > > 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 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 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. > > Colouring of $c was made dependent on it not being empty, as it is no > longer being used to colour the branch name. Removal of $b's prefix was > moved to before the colourization so it gets cleared properly now that > colour codes are inserted into it. > > Due to colour clearing being moved into the variables for each coloured > indicator, the tests for the coloured Bash prompt had to be changed: > * All colour tests now have the colour codes around the expected > content of the expanded $__git_ps1_branch_name variable instead of > the unexpanded variable in the string. > * The test with two indicators had a clear-colour code inserted after > the symbol for the first indicator, since all indicators clear their > own colours now. > > Signed-off-by: Joakim Petersen <joak-pet@online.no> > --- > Changes since v3: > * All colourized variables now also clear their own colour. > * Variables are only coloured if they are not empty, except $b (branch > name), which is not an optional indicator. > * Updated tests to reflect the new colourization behaviour. > * Fixed a mistake in two of the examples; the stash indicator is the > last of the short state indicators preceding the short upstream state > indicator. > > Range-diff against v3: > 1: 0e107d0496 < -: ---------- git-prompt: make colourization consistent > -: ---------- > 1: 98ce78ddc5 git-prompt: make colourization consistent > > contrib/completion/git-prompt.sh | 20 +++++++++++--------- > t/t9903-bash-prompt.sh | 18 +++++++++--------- > 2 files changed, 20 insertions(+), 18 deletions(-) > > diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh > index 87b2b916c0..32bb98bb8d 100644 > --- a/contrib/completion/git-prompt.sh > +++ b/contrib/completion/git-prompt.sh > @@ -245,7 +245,8 @@ __git_ps1_show_upstream () > > # Helper function that is meant to be called from __git_ps1. It > # injects color codes into the appropriate gitstring variables used > -# to build a gitstring. > +# to build a gitstring. Colored variables are responsible for clearing > +# their own color. > __git_ps1_colorize_gitstring () > { > if [[ -n ${ZSH_VERSION-} ]]; then > @@ -271,22 +272,23 @@ __git_ps1_colorize_gitstring () > else > branch_color="$bad_color" > fi > - c="$branch_color$c" > + if [ -n "$c" ]; then > + c="$branch_color$c$c_clear" > + fi > + b="$branch_color$b$c_clear" > > - z="$c_clear$z" > if [ "$w" = "*" ]; then > - w="$bad_color$w" > + w="$bad_color$w$c_clear" > fi > if [ -n "$i" ]; then > - i="$ok_color$i" > + i="$ok_color$i$c_clear" > fi > if [ -n "$s" ]; then > - s="$flags_color$s" > + s="$flags_color$s$c_clear" > fi > if [ -n "$u" ]; then > - u="$bad_color$u" > + u="$bad_color$u$c_clear" > fi > - r="$c_clear$r" > } > > # Helper function to read the first line of a file into a variable. > @@ -554,6 +556,7 @@ __git_ps1 () > fi > fi > > + b=${b##refs/heads/} > local z="${GIT_PS1_STATESEPARATOR-" "}" > > # NO color option unless in PROMPT_COMMAND mode or it's Zsh > @@ -563,7 +566,6 @@ __git_ps1 () > fi > fi > > - b=${b##refs/heads/} > if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then > __git_ps1_branch_name=$b > b="\${__git_ps1_branch_name}" > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > index bbd513bab0..abd82eec35 100755 > --- a/t/t9903-bash-prompt.sh > +++ b/t/t9903-bash-prompt.sh > @@ -541,7 +541,7 @@ test_expect_success 'prompt - pc mode' ' > ' > > test_expect_success 'prompt - bash color pc mode - branch name' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}main${c_clear}" >expected && > ( > GIT_PS1_SHOWCOLORHINTS=y && > __git_ps1 "BEFORE:" ":AFTER" >"$actual" && > @@ -551,7 +551,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' ' > ' > > test_expect_success 'prompt - bash color pc mode - detached head' ' > - printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected && > + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_red}(%s...)"${c_clear} $(git log -1 --format="%h" b1^) >expected && > git checkout b1^ && > test_when_finished "git checkout main" && > ( > @@ -563,7 +563,7 @@ test_expect_success 'prompt - bash color pc mode - detached head' ' > ' > > test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > echo "dirty" >file && > test_when_finished "git reset --hard" && > ( > @@ -576,7 +576,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt > ' > > test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > echo "dirty" >file && > test_when_finished "git reset --hard" && > git add -u && > @@ -590,7 +590,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt > ' > > test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > echo "dirty index" >file && > test_when_finished "git reset --hard" && > git add -u && > @@ -605,7 +605,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt > ' > > test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}#${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > ( > GIT_PS1_SHOWDIRTYSTATE=y && > GIT_PS1_SHOWCOLORHINTS=y && > @@ -617,7 +617,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - befo > ' > > test_expect_success 'prompt - bash color pc mode - inside .git directory' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}GIT_DIR!${c_clear}" >expected && > echo "dirty" >file && > test_when_finished "git reset --hard" && > ( > @@ -631,7 +631,7 @@ test_expect_success 'prompt - bash color pc mode - inside .git directory' ' > ' > > test_expect_success 'prompt - bash color pc mode - stash status indicator' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_lblue}\$${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > echo 2 >file && > git stash && > test_when_finished "git stash drop" && > @@ -645,7 +645,7 @@ test_expect_success 'prompt - bash color pc mode - stash status indicator' ' > ' > > test_expect_success 'prompt - bash color pc mode - untracked files status indicator' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}%%${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > ( > GIT_PS1_SHOWUNTRACKEDFILES=y && > GIT_PS1_SHOWCOLORHINTS=y && > -- > 2.36.1 > I like this solution. This isn't new, but does anybody know if there is a reason why `$w` is compared for equality to "*" as opposed to just checking whether it's a nonempty value (`-n`)? I think I'd generally prefer it to be consistent with the others, which has the added benefit of continuing to work if the asterisk is ever changed to something else. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v4] git-prompt: make colourization consistent 2022-06-04 17:30 ` Justin Donnelly @ 2022-06-04 19:18 ` Joakim Petersen 0 siblings, 0 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-04 19:18 UTC (permalink / raw) To: Justin Donnelly Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano On 04/06/2022 19:30, Justin Donnelly wrote: > I like this solution. This isn't new, but does anybody know if there > is a reason why `$w` is compared for equality to "*" as opposed to > just checking whether it's a nonempty value (`-n`)? I think I'd > generally prefer it to be consistent with the others, which has the > added benefit of continuing to work if the asterisk is ever changed to > something else. > Looking at the commit that introduced colourization, 9b7e776c0a5 (show color hints based on state of the git tree, 2012-10-10), it looks like the author wanted the be as specific as possible in the check, with $w only being empty or holding a '*', while $i could hold multiple different indicators. Since the layout of the script has changed significantly since then, I'll submit a v5 shortly with the $w check altered. ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v5] git-prompt: make colourization consistent 2022-06-04 16:13 ` [PATCH v4] " Joakim Petersen 2022-06-04 17:30 ` Justin Donnelly @ 2022-06-04 19:26 ` Joakim Petersen 2022-06-06 7:23 ` Bagas Sanjaya ` (2 more replies) 1 sibling, 3 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-04 19:26 UTC (permalink / raw) To: git Cc: Joakim Petersen, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly 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 making all colourized indicators clear their own colour. 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 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 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. Colouring of $c was made dependent on it not being empty, as it is no longer being used to colour the branch name. Removal of $b's prefix was moved to before the colourization so it gets cleared properly now that colour codes are inserted into it. Due to colour clearing being moved into the variables for each coloured indicator, the tests for the coloured Bash prompt had to be changed: * All colour tests now have the colour codes around the expected content of the expanded $__git_ps1_branch_name variable instead of the unexpanded variable in the string. * The test with two indicators had a clear-colour code inserted after the symbol for the first indicator, since all indicators clear their own colours now. Signed-off-by: Joakim Petersen <joak-pet@online.no> --- Changes since v4: * The check for whether to colourize $w has been altered to match the checks for the other indicators. Range-diff against v4: 1: 98ce78ddc5 ! 1: fffef5f73d git-prompt: make colourization consistent @@ contrib/completion/git-prompt.sh: __git_ps1_colorize_gitstring () + b="$branch_color$b$c_clear" - z="$c_clear$z" - if [ "$w" = "*" ]; then +- if [ "$w" = "*" ]; then - w="$bad_color$w" ++ if [ -n "$w" ]; then + w="$bad_color$w$c_clear" fi if [ -n "$i" ]; then contrib/completion/git-prompt.sh | 22 ++++++++++++---------- t/t9903-bash-prompt.sh | 18 +++++++++--------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 87b2b916c0..cb01c2fd5d 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -245,7 +245,8 @@ __git_ps1_show_upstream () # Helper function that is meant to be called from __git_ps1. It # injects color codes into the appropriate gitstring variables used -# to build a gitstring. +# to build a gitstring. Colored variables are responsible for clearing +# their own color. __git_ps1_colorize_gitstring () { if [[ -n ${ZSH_VERSION-} ]]; then @@ -271,22 +272,23 @@ __git_ps1_colorize_gitstring () else branch_color="$bad_color" fi - c="$branch_color$c" + if [ -n "$c" ]; then + c="$branch_color$c$c_clear" + fi + b="$branch_color$b$c_clear" - z="$c_clear$z" - if [ "$w" = "*" ]; then - w="$bad_color$w" + if [ -n "$w" ]; then + w="$bad_color$w$c_clear" fi if [ -n "$i" ]; then - i="$ok_color$i" + i="$ok_color$i$c_clear" fi if [ -n "$s" ]; then - s="$flags_color$s" + s="$flags_color$s$c_clear" fi if [ -n "$u" ]; then - u="$bad_color$u" + u="$bad_color$u$c_clear" fi - r="$c_clear$r" } # Helper function to read the first line of a file into a variable. @@ -554,6 +556,7 @@ __git_ps1 () fi fi + b=${b##refs/heads/} local z="${GIT_PS1_STATESEPARATOR-" "}" # NO color option unless in PROMPT_COMMAND mode or it's Zsh @@ -563,7 +566,6 @@ __git_ps1 () fi fi - b=${b##refs/heads/} if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then __git_ps1_branch_name=$b b="\${__git_ps1_branch_name}" diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index bbd513bab0..abd82eec35 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -541,7 +541,7 @@ test_expect_success 'prompt - pc mode' ' ' test_expect_success 'prompt - bash color pc mode - branch name' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWCOLORHINTS=y && __git_ps1 "BEFORE:" ":AFTER" >"$actual" && @@ -551,7 +551,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' ' ' test_expect_success 'prompt - bash color pc mode - detached head' ' - printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_red}(%s...)"${c_clear} $(git log -1 --format="%h" b1^) >expected && git checkout b1^ && test_when_finished "git checkout main" && ( @@ -563,7 +563,7 @@ test_expect_success 'prompt - bash color pc mode - detached head' ' ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -576,7 +576,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && git add -u && @@ -590,7 +590,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty index" >file && test_when_finished "git reset --hard" && git add -u && @@ -605,7 +605,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}#${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWDIRTYSTATE=y && GIT_PS1_SHOWCOLORHINTS=y && @@ -617,7 +617,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - befo ' test_expect_success 'prompt - bash color pc mode - inside .git directory' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}GIT_DIR!${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -631,7 +631,7 @@ test_expect_success 'prompt - bash color pc mode - inside .git directory' ' ' test_expect_success 'prompt - bash color pc mode - stash status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_lblue}\$${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo 2 >file && git stash && test_when_finished "git stash drop" && @@ -645,7 +645,7 @@ test_expect_success 'prompt - bash color pc mode - stash status indicator' ' ' test_expect_success 'prompt - bash color pc mode - untracked files status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}%%${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWUNTRACKEDFILES=y && GIT_PS1_SHOWCOLORHINTS=y && -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v5] git-prompt: make colourization consistent 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-06 16:29 ` Junio C Hamano 2022-06-06 17:50 ` [PATCH v6] " Joakim Petersen 2 siblings, 1 reply; 43+ messages in thread From: Bagas Sanjaya @ 2022-06-06 7:23 UTC (permalink / raw) To: Joakim Petersen, git Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly On 6/5/22 02:26, Joakim Petersen wrote: > 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. > colourization? I have never heared that. Did you mean "colorization" (en-US) or "colourisation" (en-UK)? I assumed the former. -- An old man doll... just what I always wanted! - Clara ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v5] git-prompt: make colourization consistent 2022-06-06 7:23 ` Bagas Sanjaya @ 2022-06-07 16:04 ` Junio C Hamano 2022-06-09 11:25 ` Joakim Petersen 0 siblings, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-07 16:04 UTC (permalink / raw) To: Bagas Sanjaya Cc: Joakim Petersen, git, Ævar Arnfjörð Bjarmason, Justin Donnelly Bagas Sanjaya <bagasdotme@gmail.com> writes: > On 6/5/22 02:26, Joakim Petersen wrote: >> 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. >> > > colourization? I have never heared that. Did you mean "colorization" (en-US) > or "colourisation" (en-UK)? I assumed the former. ;-) Either way, that word is a mouthful. Using verb "to color" and "coloring" might be easier to read, perhaps? ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v5] git-prompt: make colourization consistent 2022-06-07 16:04 ` Junio C Hamano @ 2022-06-09 11:25 ` Joakim Petersen 0 siblings, 0 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-09 11:25 UTC (permalink / raw) To: Junio C Hamano, Bagas Sanjaya Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly On 07/06/2022 18:04, Junio C Hamano wrote: > Bagas Sanjaya <bagasdotme@gmail.com> writes: > >> On 6/5/22 02:26, Joakim Petersen wrote: >>> 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. >>> >> >> colourization? I have never heared that. Did you mean "colorization" (en-US) >> or "colourisation" (en-UK)? I assumed the former. > > ;-) > > Either way, that word is a mouthful. Using verb "to color" and > "coloring" might be easier to read, perhaps? > I went with "colourization" as that is what the process is referred to in the code, however, I do agree it is a mouthful compared to "colour". Since I'm already submitting a v8 for other reasons, I'll make this change in the message as well. Since the comment about the choice of suffix spelling got picked up, I'll repeat what I told Bagas off list: "-ize" being American English is a misconception; most uses of "z" instead of "s" are AE, but the verb suffix is closer to its origin in Greek with "z", and is preferred by Oxford for this reason. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v5] git-prompt: make colourization consistent 2022-06-04 19:26 ` [PATCH v5] " Joakim Petersen 2022-06-06 7:23 ` Bagas Sanjaya @ 2022-06-06 16:29 ` Junio C Hamano 2022-06-06 17:31 ` Joakim Petersen 2022-06-06 17:50 ` [PATCH v6] " Joakim Petersen 2 siblings, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-06 16:29 UTC (permalink / raw) To: Joakim Petersen Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly Joakim Petersen <joak-pet@online.no> writes: > 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 making all colourized indicators clear > their own colour. > 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 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 looong space? > 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}" > > 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. Here, after explaining how bad the current situation is, like the above, is a good place to say what we do, i.e. "teach indicators to clear after themselves". > Colouring of $c was made dependent on it not being empty, as it is no > longer being used to colour the branch name. Removal of $b's prefix was > moved to before the colourization so it gets cleared properly now that > colour codes are inserted into it. > > Due to colour clearing being moved into the variables for each coloured > indicator, the tests for the coloured Bash prompt had to be changed: > * All colour tests now have the colour codes around the expected > content of the expanded $__git_ps1_branch_name variable instead of > the unexpanded variable in the string. > * The test with two indicators had a clear-colour code inserted after > the symbol for the first indicator, since all indicators clear their > own colours now. > > Signed-off-by: Joakim Petersen <joak-pet@online.no> > --- Nicely written. Will queue. Thanks. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v5] git-prompt: make colourization consistent 2022-06-06 16:29 ` Junio C Hamano @ 2022-06-06 17:31 ` Joakim Petersen 2022-06-06 17:41 ` Junio C Hamano 0 siblings, 1 reply; 43+ messages in thread From: Joakim Petersen @ 2022-06-06 17:31 UTC (permalink / raw) To: Junio C Hamano Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly On 06/06/2022 18:29, Junio C Hamano wrote: > Joakim Petersen <joak-pet@online.no> writes: >> * If the local tree has new, untracked files and there is nothing in >> the stash, both the '%' and the short upstream state indicator > > looong space? > That is quite the long space indeed, I'll get that fixed. >> 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. > > Here, after explaining how bad the current situation is, like the > above, is a good place to say what we do, i.e. "teach indicators to > clear after themselves". I'll add a clear statement of what this patch does as well. > > Nicely written. > > Will queue. > > Thanks. > Alright, great! ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v5] git-prompt: make colourization consistent 2022-06-06 17:31 ` Joakim Petersen @ 2022-06-06 17:41 ` Junio C Hamano 2022-06-07 11:49 ` Joakim Petersen 0 siblings, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-06 17:41 UTC (permalink / raw) To: Joakim Petersen Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly Joakim Petersen <joak-pet@online.no> writes: > On 06/06/2022 18:29, Junio C Hamano wrote: >> Joakim Petersen <joak-pet@online.no> writes: >>> * If the local tree has new, untracked files and there is nothing in >>> the stash, both the '%' and the short upstream state indicator >> looong space? >> > > That is quite the long space indeed, I'll get that fixed. > >>> 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. >> Here, after explaining how bad the current situation is, like the >> above, is a good place to say what we do, i.e. "teach indicators to >> clear after themselves". > > I'll add a clear statement of what this patch does as well. I think you already have "Make the coloring of these ... consistent by making ..." much earlier, and moving it here would be sufficient. >> Nicely written. >> Will queue. >> Thanks. >> > Alright, great! ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v5] git-prompt: make colourization consistent 2022-06-06 17:41 ` Junio C Hamano @ 2022-06-07 11:49 ` Joakim Petersen 0 siblings, 0 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-07 11:49 UTC (permalink / raw) To: Junio C Hamano Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly On 06/06/2022 19:41, Junio C Hamano wrote: > Joakim Petersen <joak-pet@online.no> writes: >> I'll add a clear statement of what this patch does as well. > > I think you already have "Make the coloring of these ... consistent > by making ..." much earlier, and moving it here would be sufficient. > This comment made me realize there is some additional redundancy in the message; I'll submit a v7 with a slightly clearer one, incorporating your suggestion as well. ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v6] git-prompt: make colourization consistent 2022-06-04 19:26 ` [PATCH v5] " Joakim Petersen 2022-06-06 7:23 ` Bagas Sanjaya 2022-06-06 16:29 ` Junio C Hamano @ 2022-06-06 17:50 ` Joakim Petersen 2022-06-07 11:50 ` [PATCH v7] " Joakim Petersen 2 siblings, 1 reply; 43+ messages in thread From: Joakim Petersen @ 2022-06-06 17:50 UTC (permalink / raw) To: git Cc: Joakim Petersen, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly 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 making all colourized indicators clear their own colour. 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 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. Teach indicators to clear their own colours. 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. 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> --- Changes since v5: * Rephrase the explanation of what this patch does to be in the imperative mood and add a clear summarizing statement of what the patch does. * Fixed the long space and removed another stray space. Range-diff against v5: 1: fffef5f73d = 1: 50765eeb95 git-prompt: make colourization consistent contrib/completion/git-prompt.sh | 22 ++++++++++++---------- t/t9903-bash-prompt.sh | 18 +++++++++--------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 87b2b916c0..cb01c2fd5d 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -245,7 +245,8 @@ __git_ps1_show_upstream () # Helper function that is meant to be called from __git_ps1. It # injects color codes into the appropriate gitstring variables used -# to build a gitstring. +# to build a gitstring. Colored variables are responsible for clearing +# their own color. __git_ps1_colorize_gitstring () { if [[ -n ${ZSH_VERSION-} ]]; then @@ -271,22 +272,23 @@ __git_ps1_colorize_gitstring () else branch_color="$bad_color" fi - c="$branch_color$c" + if [ -n "$c" ]; then + c="$branch_color$c$c_clear" + fi + b="$branch_color$b$c_clear" - z="$c_clear$z" - if [ "$w" = "*" ]; then - w="$bad_color$w" + if [ -n "$w" ]; then + w="$bad_color$w$c_clear" fi if [ -n "$i" ]; then - i="$ok_color$i" + i="$ok_color$i$c_clear" fi if [ -n "$s" ]; then - s="$flags_color$s" + s="$flags_color$s$c_clear" fi if [ -n "$u" ]; then - u="$bad_color$u" + u="$bad_color$u$c_clear" fi - r="$c_clear$r" } # Helper function to read the first line of a file into a variable. @@ -554,6 +556,7 @@ __git_ps1 () fi fi + b=${b##refs/heads/} local z="${GIT_PS1_STATESEPARATOR-" "}" # NO color option unless in PROMPT_COMMAND mode or it's Zsh @@ -563,7 +566,6 @@ __git_ps1 () fi fi - b=${b##refs/heads/} if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then __git_ps1_branch_name=$b b="\${__git_ps1_branch_name}" diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index bbd513bab0..abd82eec35 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -541,7 +541,7 @@ test_expect_success 'prompt - pc mode' ' ' test_expect_success 'prompt - bash color pc mode - branch name' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWCOLORHINTS=y && __git_ps1 "BEFORE:" ":AFTER" >"$actual" && @@ -551,7 +551,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' ' ' test_expect_success 'prompt - bash color pc mode - detached head' ' - printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_red}(%s...)"${c_clear} $(git log -1 --format="%h" b1^) >expected && git checkout b1^ && test_when_finished "git checkout main" && ( @@ -563,7 +563,7 @@ test_expect_success 'prompt - bash color pc mode - detached head' ' ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -576,7 +576,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && git add -u && @@ -590,7 +590,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty index" >file && test_when_finished "git reset --hard" && git add -u && @@ -605,7 +605,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}#${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWDIRTYSTATE=y && GIT_PS1_SHOWCOLORHINTS=y && @@ -617,7 +617,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - befo ' test_expect_success 'prompt - bash color pc mode - inside .git directory' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}GIT_DIR!${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -631,7 +631,7 @@ test_expect_success 'prompt - bash color pc mode - inside .git directory' ' ' test_expect_success 'prompt - bash color pc mode - stash status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_lblue}\$${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo 2 >file && git stash && test_when_finished "git stash drop" && @@ -645,7 +645,7 @@ test_expect_success 'prompt - bash color pc mode - stash status indicator' ' ' test_expect_success 'prompt - bash color pc mode - untracked files status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}%%${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWUNTRACKEDFILES=y && GIT_PS1_SHOWCOLORHINTS=y && -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH v7] git-prompt: make colourization consistent 2022-06-06 17:50 ` [PATCH v6] " Joakim Petersen @ 2022-06-07 11:50 ` Joakim Petersen 2022-06-07 16:22 ` Junio C Hamano ` (3 more replies) 0 siblings, 4 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-07 11:50 UTC (permalink / raw) To: git Cc: Joakim Petersen, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly 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> --- Changes since v6: * Remove repeated statements and move all explanation of what the patch does to the latter part of the message. * Add a short statement about other benefits of the behavioural change. Range-diff against v6: 1: 50765eeb95 = 1: e25738c667 git-prompt: make colourization consistent contrib/completion/git-prompt.sh | 22 ++++++++++++---------- t/t9903-bash-prompt.sh | 18 +++++++++--------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 87b2b916c0..cb01c2fd5d 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -245,7 +245,8 @@ __git_ps1_show_upstream () # Helper function that is meant to be called from __git_ps1. It # injects color codes into the appropriate gitstring variables used -# to build a gitstring. +# to build a gitstring. Colored variables are responsible for clearing +# their own color. __git_ps1_colorize_gitstring () { if [[ -n ${ZSH_VERSION-} ]]; then @@ -271,22 +272,23 @@ __git_ps1_colorize_gitstring () else branch_color="$bad_color" fi - c="$branch_color$c" + if [ -n "$c" ]; then + c="$branch_color$c$c_clear" + fi + b="$branch_color$b$c_clear" - z="$c_clear$z" - if [ "$w" = "*" ]; then - w="$bad_color$w" + if [ -n "$w" ]; then + w="$bad_color$w$c_clear" fi if [ -n "$i" ]; then - i="$ok_color$i" + i="$ok_color$i$c_clear" fi if [ -n "$s" ]; then - s="$flags_color$s" + s="$flags_color$s$c_clear" fi if [ -n "$u" ]; then - u="$bad_color$u" + u="$bad_color$u$c_clear" fi - r="$c_clear$r" } # Helper function to read the first line of a file into a variable. @@ -554,6 +556,7 @@ __git_ps1 () fi fi + b=${b##refs/heads/} local z="${GIT_PS1_STATESEPARATOR-" "}" # NO color option unless in PROMPT_COMMAND mode or it's Zsh @@ -563,7 +566,6 @@ __git_ps1 () fi fi - b=${b##refs/heads/} if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then __git_ps1_branch_name=$b b="\${__git_ps1_branch_name}" diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index bbd513bab0..abd82eec35 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -541,7 +541,7 @@ test_expect_success 'prompt - pc mode' ' ' test_expect_success 'prompt - bash color pc mode - branch name' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWCOLORHINTS=y && __git_ps1 "BEFORE:" ":AFTER" >"$actual" && @@ -551,7 +551,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' ' ' test_expect_success 'prompt - bash color pc mode - detached head' ' - printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_red}(%s...)"${c_clear} $(git log -1 --format="%h" b1^) >expected && git checkout b1^ && test_when_finished "git checkout main" && ( @@ -563,7 +563,7 @@ test_expect_success 'prompt - bash color pc mode - detached head' ' ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -576,7 +576,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && git add -u && @@ -590,7 +590,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty index" >file && test_when_finished "git reset --hard" && git add -u && @@ -605,7 +605,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}#${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWDIRTYSTATE=y && GIT_PS1_SHOWCOLORHINTS=y && @@ -617,7 +617,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - befo ' test_expect_success 'prompt - bash color pc mode - inside .git directory' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}GIT_DIR!${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -631,7 +631,7 @@ test_expect_success 'prompt - bash color pc mode - inside .git directory' ' ' test_expect_success 'prompt - bash color pc mode - stash status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_lblue}\$${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo 2 >file && git stash && test_when_finished "git stash drop" && @@ -645,7 +645,7 @@ test_expect_success 'prompt - bash color pc mode - stash status indicator' ' ' test_expect_success 'prompt - bash color pc mode - untracked files status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}%%${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWUNTRACKEDFILES=y && GIT_PS1_SHOWCOLORHINTS=y && -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v7] git-prompt: make colourization consistent 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 ` (2 subsequent siblings) 3 siblings, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-07 16:22 UTC (permalink / raw) To: Joakim Petersen Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly Joakim Petersen <joak-pet@online.no> writes: > 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> > --- > Changes since v6: > * Remove repeated statements and move all explanation of what the patch > does to the latter part of the message. > * Add a short statement about other benefits of the behavioural change. The handling of $w is different from the original (it used to be that only '*' was painted in red, now any non-empty strings do), but '*' is the only value that can be assigned to $w, so there is no material difference. Looking good. Will queue. Thanks. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v7] git-prompt: make colourization consistent 2022-06-07 16:22 ` Junio C Hamano @ 2022-06-09 11:16 ` Joakim Petersen 0 siblings, 0 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-09 11:16 UTC (permalink / raw) To: Junio C Hamano Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly On 07/06/2022 18:22, Junio C Hamano wrote: > Joakim Petersen <joak-pet@online.no> writes: > >> 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> >> --- >> Changes since v6: >> * Remove repeated statements and move all explanation of what the patch >> does to the latter part of the message. >> * Add a short statement about other benefits of the behavioural change. > > The handling of $w is different from the original (it used to be > that only '*' was painted in red, now any non-empty strings do), but > '*' is the only value that can be assigned to $w, so there is no > material difference. > > Looking good. Will queue. Thanks. > The change regarding $w was mentioned below --- for v5: > Changes since v4: > * The check for whether to colourize $w has been altered to match the > checks for the other indicators. I'll add a mention of this to the commit message as well. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v7] git-prompt: make colourization consistent 2022-06-07 11:50 ` [PATCH v7] " Joakim Petersen 2022-06-07 16:22 ` Junio C Hamano @ 2022-06-09 9:03 ` SZEDER Gábor 2022-06-09 11:13 ` Joakim Petersen 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 3 siblings, 1 reply; 43+ messages in thread From: SZEDER Gábor @ 2022-06-09 9:03 UTC (permalink / raw) To: Joakim Petersen Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly On Tue, Jun 07, 2022 at 01:50:24PM +0200, 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. 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. This patch seems to break colorization when __git_ps1() is invoked from $PROMPT_COMMAND: ~/src/git (master)$ echo $PROMPT_COMMAND __git_ps1 "\[\e]0;\w - Terminal\a\e[01;32m\]\h\[\e[01;34m\] \w" "\[\e[01;34m\]\$\[\e[00m\] " " \[\e[01;34m\](%s\[\e[01;34m\])" ~/src/git (master)$ git checkout 9470605a1b HEAD is now at 9470605a1b git-prompt: make colourization consistent ~/src/git ((9470605a1b...))$ source contrib/completion/git-prompt.sh ~/src/git (\[\e[31m\](9470605a1b...)\[\e[0m\])$ # uh-oh ~/src/git (\[\e[31m\](9470605a1b...)\[\e[0m\])$ git checkout 9470605a1b^ Previous HEAD position was 9470605a1b git-prompt: make colourization consistent HEAD is now at 2668e3608e Sixth batch ~/src/git (\[\e[31m\](2668e3608e...)\[\e[0m\])$ source contrib/completion/git-prompt.sh ~/src/git ((2668e3608e...))$ # Looks good. > Signed-off-by: Joakim Petersen <joak-pet@online.no> > --- > Changes since v6: > * Remove repeated statements and move all explanation of what the patch > does to the latter part of the message. > * Add a short statement about other benefits of the behavioural change. > > Range-diff against v6: > 1: 50765eeb95 = 1: e25738c667 git-prompt: make colourization consistent > > contrib/completion/git-prompt.sh | 22 ++++++++++++---------- > t/t9903-bash-prompt.sh | 18 +++++++++--------- > 2 files changed, 21 insertions(+), 19 deletions(-) > > diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh > index 87b2b916c0..cb01c2fd5d 100644 > --- a/contrib/completion/git-prompt.sh > +++ b/contrib/completion/git-prompt.sh > @@ -245,7 +245,8 @@ __git_ps1_show_upstream () > > # Helper function that is meant to be called from __git_ps1. It > # injects color codes into the appropriate gitstring variables used > -# to build a gitstring. > +# to build a gitstring. Colored variables are responsible for clearing > +# their own color. > __git_ps1_colorize_gitstring () > { > if [[ -n ${ZSH_VERSION-} ]]; then > @@ -271,22 +272,23 @@ __git_ps1_colorize_gitstring () > else > branch_color="$bad_color" > fi > - c="$branch_color$c" > + if [ -n "$c" ]; then > + c="$branch_color$c$c_clear" > + fi > + b="$branch_color$b$c_clear" > > - z="$c_clear$z" > - if [ "$w" = "*" ]; then > - w="$bad_color$w" > + if [ -n "$w" ]; then > + w="$bad_color$w$c_clear" > fi > if [ -n "$i" ]; then > - i="$ok_color$i" > + i="$ok_color$i$c_clear" > fi > if [ -n "$s" ]; then > - s="$flags_color$s" > + s="$flags_color$s$c_clear" > fi > if [ -n "$u" ]; then > - u="$bad_color$u" > + u="$bad_color$u$c_clear" > fi > - r="$c_clear$r" > } > > # Helper function to read the first line of a file into a variable. > @@ -554,6 +556,7 @@ __git_ps1 () > fi > fi > > + b=${b##refs/heads/} > local z="${GIT_PS1_STATESEPARATOR-" "}" > > # NO color option unless in PROMPT_COMMAND mode or it's Zsh > @@ -563,7 +566,6 @@ __git_ps1 () > fi > fi > > - b=${b##refs/heads/} > if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then > __git_ps1_branch_name=$b > b="\${__git_ps1_branch_name}" > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > index bbd513bab0..abd82eec35 100755 > --- a/t/t9903-bash-prompt.sh > +++ b/t/t9903-bash-prompt.sh > @@ -541,7 +541,7 @@ test_expect_success 'prompt - pc mode' ' > ' > > test_expect_success 'prompt - bash color pc mode - branch name' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}main${c_clear}" >expected && > ( > GIT_PS1_SHOWCOLORHINTS=y && > __git_ps1 "BEFORE:" ":AFTER" >"$actual" && > @@ -551,7 +551,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' ' > ' > > test_expect_success 'prompt - bash color pc mode - detached head' ' > - printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected && > + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_red}(%s...)"${c_clear} $(git log -1 --format="%h" b1^) >expected && > git checkout b1^ && > test_when_finished "git checkout main" && > ( > @@ -563,7 +563,7 @@ test_expect_success 'prompt - bash color pc mode - detached head' ' > ' > > test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > echo "dirty" >file && > test_when_finished "git reset --hard" && > ( > @@ -576,7 +576,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt > ' > > test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > echo "dirty" >file && > test_when_finished "git reset --hard" && > git add -u && > @@ -590,7 +590,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt > ' > > test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > echo "dirty index" >file && > test_when_finished "git reset --hard" && > git add -u && > @@ -605,7 +605,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt > ' > > test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}#${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > ( > GIT_PS1_SHOWDIRTYSTATE=y && > GIT_PS1_SHOWCOLORHINTS=y && > @@ -617,7 +617,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - befo > ' > > test_expect_success 'prompt - bash color pc mode - inside .git directory' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}GIT_DIR!${c_clear}" >expected && > echo "dirty" >file && > test_when_finished "git reset --hard" && > ( > @@ -631,7 +631,7 @@ test_expect_success 'prompt - bash color pc mode - inside .git directory' ' > ' > > test_expect_success 'prompt - bash color pc mode - stash status indicator' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_lblue}\$${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > echo 2 >file && > git stash && > test_when_finished "git stash drop" && > @@ -645,7 +645,7 @@ test_expect_success 'prompt - bash color pc mode - stash status indicator' ' > ' > > test_expect_success 'prompt - bash color pc mode - untracked files status indicator' ' > - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmain" >expected && > + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}%%${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && > ( > GIT_PS1_SHOWUNTRACKEDFILES=y && > GIT_PS1_SHOWCOLORHINTS=y && > -- > 2.36.1 > ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v7] git-prompt: make colourization consistent 2022-06-09 9:03 ` SZEDER Gábor @ 2022-06-09 11:13 ` Joakim Petersen 2022-06-09 18:29 ` Junio C Hamano 0 siblings, 1 reply; 43+ messages in thread From: Joakim Petersen @ 2022-06-09 11:13 UTC (permalink / raw) To: SZEDER Gábor Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly On 09/06/2022 11:03, SZEDER Gábor wrote: > This patch seems to break colorization when __git_ps1() is invoked > from $PROMPT_COMMAND: > > ~/src/git (master)$ echo $PROMPT_COMMAND > __git_ps1 "\[\e]0;\w - Terminal\a\e[01;32m\]\h\[\e[01;34m\] \w" "\[\e[01;34m\]\$\[\e[00m\] " " \[\e[01;34m\](%s\[\e[01;34m\])" > ~/src/git (master)$ git checkout 9470605a1b > HEAD is now at 9470605a1b git-prompt: make colourization consistent > ~/src/git ((9470605a1b...))$ source contrib/completion/git-prompt.sh > ~/src/git (\[\e[31m\](9470605a1b...)\[\e[0m\])$ # uh-oh > ~/src/git (\[\e[31m\](9470605a1b...)\[\e[0m\])$ git checkout 9470605a1b^ > Previous HEAD position was 9470605a1b git-prompt: make colourization consistent > HEAD is now at 2668e3608e Sixth batch > ~/src/git (\[\e[31m\](2668e3608e...)\[\e[0m\])$ source contrib/completion/git-prompt.sh > ~/src/git ((2668e3608e...))$ # Looks good. > While I did test this on my own prompt for v6 (which is identical to v7 in terms of code) and not see any breakage, I have the same issue with v7. Maybe I forgot to re-source the changed git-prompt.sh. Either way, The issue stems from $b being wrapped in $__git_ps1_branch_name and then back into itself after colouring. Moving this wrapping to before colour is applied fixes this. I will submit a v8 shortly. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v7] git-prompt: make colourization consistent 2022-06-09 11:13 ` Joakim Petersen @ 2022-06-09 18:29 ` Junio C Hamano 2022-06-11 9:01 ` SZEDER Gábor 0 siblings, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-09 18:29 UTC (permalink / raw) To: Joakim Petersen Cc: SZEDER Gábor, git, Ævar Arnfjörð Bjarmason, Justin Donnelly Joakim Petersen <joak-pet@online.no> writes: > On 09/06/2022 11:03, SZEDER Gábor wrote: >> This patch seems to break colorization when __git_ps1() is invoked >> from $PROMPT_COMMAND: >> ~/src/git (master)$ echo $PROMPT_COMMAND >> __git_ps1 "\[\e]0;\w - Terminal\a\e[01;32m\]\h\[\e[01;34m\] \w" "\[\e[01;34m\]\$\[\e[00m\] " " \[\e[01;34m\](%s\[\e[01;34m\])" >> ~/src/git (master)$ git checkout 9470605a1b >> HEAD is now at 9470605a1b git-prompt: make colourization consistent >> ~/src/git ((9470605a1b...))$ source contrib/completion/git-prompt.sh >> ~/src/git (\[\e[31m\](9470605a1b...)\[\e[0m\])$ # uh-oh >> ~/src/git (\[\e[31m\](9470605a1b...)\[\e[0m\])$ git checkout 9470605a1b^ >> Previous HEAD position was 9470605a1b git-prompt: make colourization consistent >> HEAD is now at 2668e3608e Sixth batch >> ~/src/git (\[\e[31m\](2668e3608e...)\[\e[0m\])$ source contrib/completion/git-prompt.sh >> ~/src/git ((2668e3608e...))$ # Looks good. >> > > While I did test this on my own prompt for v6 (which is identical to v7 > in terms of code) and not see any breakage, I have the same issue with > v7. Maybe I forgot to re-source the changed git-prompt.sh. Either way, > The issue stems from $b being wrapped in $__git_ps1_branch_name and then > back into itself after colouring. Moving this wrapping to before colour > is applied fixes this. I will submit a v8 shortly. As the topic is already in 'next' (and presumably that is how SZEDER noticed the breakage), please make it an incremental fix-up. Thanks. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v7] git-prompt: make colourization consistent 2022-06-09 18:29 ` Junio C Hamano @ 2022-06-11 9:01 ` SZEDER Gábor 0 siblings, 0 replies; 43+ messages in thread From: SZEDER Gábor @ 2022-06-11 9:01 UTC (permalink / raw) To: Junio C Hamano Cc: Joakim Petersen, git, Ævar Arnfjörð Bjarmason, Justin Donnelly On Thu, Jun 09, 2022 at 11:29:25AM -0700, Junio C Hamano wrote: > Joakim Petersen <joak-pet@online.no> writes: > > > On 09/06/2022 11:03, SZEDER Gábor wrote: > >> This patch seems to break colorization when __git_ps1() is invoked > >> from $PROMPT_COMMAND: > >> ~/src/git (master)$ echo $PROMPT_COMMAND > >> __git_ps1 "\[\e]0;\w - Terminal\a\e[01;32m\]\h\[\e[01;34m\] \w" "\[\e[01;34m\]\$\[\e[00m\] " " \[\e[01;34m\](%s\[\e[01;34m\])" > >> ~/src/git (master)$ git checkout 9470605a1b > >> HEAD is now at 9470605a1b git-prompt: make colourization consistent > >> ~/src/git ((9470605a1b...))$ source contrib/completion/git-prompt.sh > >> ~/src/git (\[\e[31m\](9470605a1b...)\[\e[0m\])$ # uh-oh > >> ~/src/git (\[\e[31m\](9470605a1b...)\[\e[0m\])$ git checkout 9470605a1b^ > >> Previous HEAD position was 9470605a1b git-prompt: make colourization consistent > >> HEAD is now at 2668e3608e Sixth batch > >> ~/src/git (\[\e[31m\](2668e3608e...)\[\e[0m\])$ source contrib/completion/git-prompt.sh > >> ~/src/git ((2668e3608e...))$ # Looks good. > >> > > > > While I did test this on my own prompt for v6 (which is identical to v7 > > in terms of code) and not see any breakage, I have the same issue with > > v7. Maybe I forgot to re-source the changed git-prompt.sh. Either way, > > The issue stems from $b being wrapped in $__git_ps1_branch_name and then > > back into itself after colouring. Moving this wrapping to before colour > > is applied fixes this. I will submit a v8 shortly. > > As the topic is already in 'next' (and presumably that is how SZEDER > noticed the breakage), Indeed. I usually use a custom git built from 'next' with a couple of my forever-WIP topics merged on top, and I just happened to build and deploy a version with this patch already merged the other day, with the additional stroke of luck that I opened a new terminal window (what I normally rarely do) whose shell sourced the buggy prompt script. I did notice this patch being discussed on the ML, and found the amount of changes to the expected output in the tests somewhat suspicious, but, alas, haven't managed to take a closer look before the patch went into 'next'. Still hasn't, actually, but FWIW Joakim's fix (as 0e5d9ef395 in 'seen') does work for me. Thanks, Gábor ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v8] git-prompt: make colouring consistent 2022-06-07 11:50 ` [PATCH v7] " Joakim Petersen 2022-06-07 16:22 ` Junio C Hamano 2022-06-09 9:03 ` SZEDER Gábor @ 2022-06-09 11:44 ` Joakim Petersen 2022-06-09 20:44 ` [PATCH] git-prompt: fix expansion of branch colour codes Joakim Petersen 3 siblings, 0 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-09 11:44 UTC (permalink / raw) To: git Cc: Joakim Petersen, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly, SZEDER Gábor, Bagas Sanjaya 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 colour 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 coloured 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 coloured indicators changes how the sparsity state indicator is coloured, 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 colouring also was unintended, so clearing the colour for said indicator further increases consistency. Make the colouring of these state indicators consistent by making all coloured 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. Change $w to follow the same behaviour so changes to its content don't change how it is coloured. Move clearing of $b's prefix to before colouring 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. Having each indicator clear its own colour lessens the cost of refactoring the prompt layout, as the outcome of colouring becomes independent on the position of indicators in the final prompt. Signed-off-by: Joakim Petersen <joak-pet@online.no> --- Changes since v7: * Fixed branch name ($b) not being coloured, but rather having its colour codes printed as text. * Added mention of the change to the check for whether to colour $w. * Replaced use of "colourize" and derivatives in the message with "colour". Range-diff against v7: 1: 50765eeb95 ! 1: 1082f46b4c git-prompt: make colourization consistent @@ contrib/completion/git-prompt.sh: __git_ps1_colorize_gitstring () # Helper function to read the first line of a file into a variable. @@ contrib/completion/git-prompt.sh: __git_ps1 () - fi fi -+ b=${b##refs/heads/} local z="${GIT_PS1_STATESEPARATOR-" "}" ++ b=${b##refs/heads/} ++ if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then ++ __git_ps1_branch_name=$b ++ b="\${__git_ps1_branch_name}" ++ fi ++ # NO color option unless in PROMPT_COMMAND mode or it's Zsh + if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then @@ contrib/completion/git-prompt.sh: __git_ps1 () fi fi - b=${b##refs/heads/} - if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then - __git_ps1_branch_name=$b - b="\${__git_ps1_branch_name}" +- if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then +- __git_ps1_branch_name=$b +- b="\${__git_ps1_branch_name}" +- fi +- + local f="$h$w$i$s$u$p" + local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}" + ## t/t9903-bash-prompt.sh ## @@ t/t9903-bash-prompt.sh: test_expect_success 'prompt - pc mode' ' contrib/completion/git-prompt.sh | 32 +++++++++++++++++--------------- t/t9903-bash-prompt.sh | 18 +++++++++--------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 87b2b916c0..44bfc7972e 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -245,7 +245,8 @@ __git_ps1_show_upstream () # Helper function that is meant to be called from __git_ps1. It # injects color codes into the appropriate gitstring variables used -# to build a gitstring. +# to build a gitstring. Colored variables are responsible for clearing +# their own color. __git_ps1_colorize_gitstring () { if [[ -n ${ZSH_VERSION-} ]]; then @@ -271,22 +272,23 @@ __git_ps1_colorize_gitstring () else branch_color="$bad_color" fi - c="$branch_color$c" + if [ -n "$c" ]; then + c="$branch_color$c$c_clear" + fi + b="$branch_color$b$c_clear" - z="$c_clear$z" - if [ "$w" = "*" ]; then - w="$bad_color$w" + if [ -n "$w" ]; then + w="$bad_color$w$c_clear" fi if [ -n "$i" ]; then - i="$ok_color$i" + i="$ok_color$i$c_clear" fi if [ -n "$s" ]; then - s="$flags_color$s" + s="$flags_color$s$c_clear" fi if [ -n "$u" ]; then - u="$bad_color$u" + u="$bad_color$u$c_clear" fi - r="$c_clear$r" } # Helper function to read the first line of a file into a variable. @@ -555,6 +557,12 @@ __git_ps1 () fi local z="${GIT_PS1_STATESEPARATOR-" "}" + b=${b##refs/heads/} + if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then + __git_ps1_branch_name=$b + b="\${__git_ps1_branch_name}" + fi + # NO color option unless in PROMPT_COMMAND mode or it's Zsh if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then @@ -563,12 +571,6 @@ __git_ps1 () fi fi - b=${b##refs/heads/} - if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then - __git_ps1_branch_name=$b - b="\${__git_ps1_branch_name}" - fi - local f="$h$w$i$s$u$p" local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}" diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index bbd513bab0..abd82eec35 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -541,7 +541,7 @@ test_expect_success 'prompt - pc mode' ' ' test_expect_success 'prompt - bash color pc mode - branch name' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWCOLORHINTS=y && __git_ps1 "BEFORE:" ":AFTER" >"$actual" && @@ -551,7 +551,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' ' ' test_expect_success 'prompt - bash color pc mode - detached head' ' - printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_red}(%s...)"${c_clear} $(git log -1 --format="%h" b1^) >expected && git checkout b1^ && test_when_finished "git checkout main" && ( @@ -563,7 +563,7 @@ test_expect_success 'prompt - bash color pc mode - detached head' ' ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -576,7 +576,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && git add -u && @@ -590,7 +590,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo "dirty index" >file && test_when_finished "git reset --hard" && git add -u && @@ -605,7 +605,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}#${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWDIRTYSTATE=y && GIT_PS1_SHOWCOLORHINTS=y && @@ -617,7 +617,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - befo ' test_expect_success 'prompt - bash color pc mode - inside .git directory' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected && + printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}GIT_DIR!${c_clear}" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -631,7 +631,7 @@ test_expect_success 'prompt - bash color pc mode - inside .git directory' ' ' test_expect_success 'prompt - bash color pc mode - stash status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_lblue}\$${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && echo 2 >file && git stash && test_when_finished "git stash drop" && @@ -645,7 +645,7 @@ test_expect_success 'prompt - bash color pc mode - stash status indicator' ' ' test_expect_success 'prompt - bash color pc mode - untracked files status indicator' ' - printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmain" >expected && + printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}%%${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && ( GIT_PS1_SHOWUNTRACKEDFILES=y && GIT_PS1_SHOWCOLORHINTS=y && -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH] git-prompt: fix expansion of branch colour codes 2022-06-07 11:50 ` [PATCH v7] " Joakim Petersen ` (2 preceding siblings ...) 2022-06-09 11:44 ` [PATCH v8] git-prompt: make colouring consistent Joakim Petersen @ 2022-06-09 20:44 ` Joakim Petersen 2022-06-10 0:05 ` Junio C Hamano 2022-06-10 0:47 ` [PATCH v2] " Joakim Petersen 3 siblings, 2 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-09 20:44 UTC (permalink / raw) To: git Cc: Joakim Petersen, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly, SZEDER Gábor, Bagas Sanjaya Because of the wrapping of the branch name variable $b, the colour codes in the variable don't get applied, but are instead printed directly in the output. Move the wrapping of $b to before colour codes are inserted to correct this. Signed-off-by: Joakim Petersen <joak-pet@online.no> --- contrib/completion/git-prompt.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index cb01c2fd5d..1435548e00 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -556,9 +556,14 @@ __git_ps1 () fi fi - b=${b##refs/heads/} local z="${GIT_PS1_STATESEPARATOR-" "}" + b=${b##refs/heads/} + if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then + __git_ps1_branch_name=$b + b="\${__git_ps1_branch_name}" + fi + # 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 @@ -566,11 +571,6 @@ __git_ps1 () fi fi - if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then - __git_ps1_branch_name=$b - b="\${__git_ps1_branch_name}" - fi - local f="$h$w$i$s$u$p" local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}" base-commit: 9470605a1b03dac8fc4f801132e36964b4fbb8c3 -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH] git-prompt: fix expansion of branch colour codes 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 1 sibling, 1 reply; 43+ messages in thread From: Junio C Hamano @ 2022-06-10 0:05 UTC (permalink / raw) To: Joakim Petersen Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly, SZEDER Gábor, Bagas Sanjaya Joakim Petersen <joak-pet@online.no> writes: > Because of the wrapping of the branch name variable $b, the colour codes > in the variable don't get applied, but are instead printed directly in > the output. Move the wrapping of $b to before colour codes are inserted > to correct this. > > Signed-off-by: Joakim Petersen <joak-pet@online.no> > --- > contrib/completion/git-prompt.sh | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) t9903 seems to fail with this, though...? > diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh > index cb01c2fd5d..1435548e00 100644 > --- a/contrib/completion/git-prompt.sh > +++ b/contrib/completion/git-prompt.sh > @@ -556,9 +556,14 @@ __git_ps1 () > fi > fi > > - b=${b##refs/heads/} > local z="${GIT_PS1_STATESEPARATOR-" "}" > > + b=${b##refs/heads/} > + if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then > + __git_ps1_branch_name=$b > + b="\${__git_ps1_branch_name}" > + fi > + > # 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 > @@ -566,11 +571,6 @@ __git_ps1 () > fi > fi > > - if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then > - __git_ps1_branch_name=$b > - b="\${__git_ps1_branch_name}" > - fi > - > local f="$h$w$i$s$u$p" > local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}" > > > base-commit: 9470605a1b03dac8fc4f801132e36964b4fbb8c3 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] git-prompt: fix expansion of branch colour codes 2022-06-10 0:05 ` Junio C Hamano @ 2022-06-10 0:33 ` Joakim Petersen 0 siblings, 0 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-10 0:33 UTC (permalink / raw) To: Junio C Hamano Cc: git, Ævar Arnfjörð Bjarmason, Justin Donnelly, SZEDER Gábor, Bagas Sanjaya On 10/06/2022 02:05, Junio C Hamano wrote: > t9903 seems to fail with this, though...? > Ah yes, sorry about that, I seem to have gotten a bit ahead of myself. Since the wrapping of $b was entirely moved to before colouring happens, the tests need to be reverted back to their state from before the previous commit, with the exception of the extra $c_clear in the test with two active indicators. I'll submit a v2 with this change shortly. ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2] git-prompt: fix expansion of branch colour codes 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:47 ` Joakim Petersen 1 sibling, 0 replies; 43+ messages in thread From: Joakim Petersen @ 2022-06-10 0:47 UTC (permalink / raw) To: git Cc: Joakim Petersen, Ævar Arnfjörð Bjarmason, Junio C Hamano, Justin Donnelly, SZEDER Gábor, Bagas Sanjaya Because of the wrapping of the branch name variable $b, the colour codes in the variable don't get applied, but are instead printed directly in the output. Move the wrapping of $b to before colour codes are inserted to correct this. Revert move of branch name colour codes in tests, as the branch name is now coloured after the wrapping instead of before. Signed-off-by: Joakim Petersen <joak-pet@online.no> --- Changes since v1: * Tests have been reverted to how they were before the initial colouring change in 9470605a1b0 (git-prompt: make colourization consistent, 2022-06-07), with the exception of the added $c_clear for the test with two active indicators. Range-diff against v1: 1: f4e46d37a5 < -: ---------- git-prompt: fix expansion of branch colour codes -: ---------- > 1: 2520d60f0f git-prompt: fix expansion of branch colour codes contrib/completion/git-prompt.sh | 12 ++++++------ t/t9903-bash-prompt.sh | 18 +++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index cb01c2fd5d..1435548e00 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -556,9 +556,14 @@ __git_ps1 () fi fi - b=${b##refs/heads/} local z="${GIT_PS1_STATESEPARATOR-" "}" + b=${b##refs/heads/} + if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then + __git_ps1_branch_name=$b + b="\${__git_ps1_branch_name}" + fi + # 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 @@ -566,11 +571,6 @@ __git_ps1 () fi fi - if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then - __git_ps1_branch_name=$b - b="\${__git_ps1_branch_name}" - fi - local f="$h$w$i$s$u$p" local gitstring="$c$b${f:+$z$f}${sparse}$r${upstream}" diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index abd82eec35..6a30f5719c 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -541,7 +541,7 @@ test_expect_success 'prompt - pc mode' ' ' test_expect_success 'prompt - bash color pc mode - branch name' ' - printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}main${c_clear}" >expected && + printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmain" >expected && ( GIT_PS1_SHOWCOLORHINTS=y && __git_ps1 "BEFORE:" ":AFTER" >"$actual" && @@ -551,7 +551,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' ' ' test_expect_success 'prompt - bash color pc mode - detached head' ' - printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_red}(%s...)"${c_clear} $(git log -1 --format="%h" b1^) >expected && + printf "BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...)" $(git log -1 --format="%h" b1^) >expected && git checkout b1^ && test_when_finished "git checkout main" && ( @@ -563,7 +563,7 @@ test_expect_success 'prompt - bash color pc mode - detached head' ' ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' - printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && + printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmain" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -576,7 +576,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' - printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && + printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmain" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && git add -u && @@ -590,7 +590,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' ' - printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && + printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}${c_green}+${c_clear}):AFTER\\nmain" >expected && echo "dirty index" >file && test_when_finished "git reset --hard" && git add -u && @@ -605,7 +605,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirt ' test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' ' - printf "BEFORE: (\${__git_ps1_branch_name} ${c_green}#${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && + printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmain" >expected && ( GIT_PS1_SHOWDIRTYSTATE=y && GIT_PS1_SHOWCOLORHINTS=y && @@ -617,7 +617,7 @@ test_expect_success 'prompt - bash color pc mode - dirty status indicator - befo ' test_expect_success 'prompt - bash color pc mode - inside .git directory' ' - printf "BEFORE: (\${__git_ps1_branch_name}):AFTER\\n${c_green}GIT_DIR!${c_clear}" >expected && + printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR!" >expected && echo "dirty" >file && test_when_finished "git reset --hard" && ( @@ -631,7 +631,7 @@ test_expect_success 'prompt - bash color pc mode - inside .git directory' ' ' test_expect_success 'prompt - bash color pc mode - stash status indicator' ' - printf "BEFORE: (\${__git_ps1_branch_name} ${c_lblue}\$${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && + printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmain" >expected && echo 2 >file && git stash && test_when_finished "git stash drop" && @@ -645,7 +645,7 @@ test_expect_success 'prompt - bash color pc mode - stash status indicator' ' ' test_expect_success 'prompt - bash color pc mode - untracked files status indicator' ' - printf "BEFORE: (\${__git_ps1_branch_name} ${c_red}%%${c_clear}):AFTER\\n${c_green}main${c_clear}" >expected && + printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmain" >expected && ( GIT_PS1_SHOWUNTRACKEDFILES=y && GIT_PS1_SHOWCOLORHINTS=y && -- 2.36.1 ^ permalink raw reply related [flat|nested] 43+ messages in thread
end of thread, other threads:[~2022-06-11 9:01 UTC | newest] Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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.