bash completion with colour hints

bash completion with colour hints

[Simon Oosthoek]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

Hi Shawn

I only recently found the __git_ps1 function, but it wasn't directly 
able to replace my own contraption. I've modified the version I found 
after installing bash-completion in debian 6.

The patch is attached, it contains an escape character, so it is hard to 
include in plain text. I've gzipped it for convenience...

This is only a first step, I had a hard time figuring out what exactly 
the one-letter variables were doing (and still a bit unclear), so I'm 
sure this can be improved!

Anyway, the functionality of this patch is to show the output in green 
if the repo is up to date and red or other colours if it isn't.

Cheers

Simon

[-- Attachment #2: bash_git_completion_colour.patch.gz --]
[-- Type: application/gzip, Size: 645 bytes --]

Re: bash completion with colour hints

[Ramkumar Ramachandra]

Hi Simon,

Could you follow the guidelines in Documentation/SubmittingPatches, so
that the patch can be considered for inclusion?

> --- git-orig-bak	2012-09-26 16:39:47.000000000 +0200
> +++ git-bashcompletion	2012-09-26 16:50:57.000000000 +0200
> @@ -59,6 +59,9 @@
>  #       per-repository basis by setting the bash.showUpstream config
>  #       variable.
>  #
> +#       If you would like an additional hint in colour in your prompt
> +#       set GIT_PS1_SHOWCOLORHINT to a nonempty value. Currently
> +#       the colours are hardcoded in the function...

Nit: I think it's spelt "color" everywhere else in git.

>  #
>  # To submit patches:
>  #
> @@ -302,9 +305,35 @@
>  				__git_ps1_show_upstream
>  			fi
>  		fi
> -
> +	

Whitespace damage.

> +		local c_red=' [31m'
> +		local c_yellow=' [33m'
> +		local c_lblue=' [1,34m'
> +		local c_green=' [32m'
> +		local c_purple=' [35m'
> +		local c_cyan=' [36m'
> +		local c_clear=' [0m'
> +		local printf_format="${1:- (%s)}"
> +
> +		if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
> +			if [ "$w" != "*" ]; then
> +				printf_format="$c_green$printf_format$c_clear"
> +			else
> +				printf_format="$c_red$printf_format$c_clear"
> +			fi
> +			if [ -n "$i" ]; then
> +				i="$c_yellow$i$c_clear"
> +			fi
> +			if [ -n "$s" ]; then
> +				s="$c_lblue$i$c_clear"
> +			fi
> +			if [ -n "$u" ]; then
> +				u="$c_purple$i$c_clear"
> +			fi
> +		fi
> +			
>  		local f="$w$i$s$u"
> -		printf "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
> +		echo $(printf "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p")
>  	fi
>  }

Looks okay from a glance.

Ram

Re: bash completion with colour hints

[Simon Oosthoek]

On 26/09/12 17:24, Ramkumar Ramachandra wrote:
> Hi Simon,
> 
> Could you follow the guidelines in Documentation/SubmittingPatches, so
> that the patch can be considered for inclusion?

Hi Ram, thanks for your feedback.

I gather now that this file is part of the entire git tree ;-)

this is my first time to submit a patch to git and frankly I coded this
up very quickly and did only a small test.

I read the guide and now I have some questions:

- It suggests to use the oldest commit that contains the "bug" and can
support the fix. This would be the very first mention of __git_ps1
function I think commit d3d717a4ad0c8d7329e79f7d0313baec57c6b585
However, I guess that although I have been using something similar since
about 2009, I should at least base it on the relatively new
git-prompt.sh file, is this a correct interpretation of the guide?
(BTW, I wonder how this will affect ultimately the current master?)

- I read that git-prompt.sh is meant to support bash and zsh, I have
only tested it on bash. Should I attempt to test it on zsh or is there a
kind person with zsh as his/her shell to test it for me?

My instinct is to just apply my patch to the current master, but I'm
open to starting from a different base, but I'm too new to the tree to
know which one, any suggestions?



> 
>> --- git-orig-bak	2012-09-26 16:39:47.000000000 +0200
>> +++ git-bashcompletion	2012-09-26 16:50:57.000000000 +0200
>> @@ -59,6 +59,9 @@
>>  #       per-repository basis by setting the bash.showUpstream config
>>  #       variable.
>>  #
>> +#       If you would like an additional hint in colour in your prompt
>> +#       set GIT_PS1_SHOWCOLORHINT to a nonempty value. Currently
>> +#       the colours are hardcoded in the function...
> 
> Nit: I think it's spelt "color" everywhere else in git.
> 

I can adapt ;-)

>> +		local c_red=' [31m'
>> +		local c_yellow=' [33m'
>> +		local c_lblue=' [1,34m'
>> +		local c_green=' [32m'
>> +		local c_purple=' [35m'
>> +		local c_cyan=' [36m'
>> +		local c_clear=' [0m'
>> +		local printf_format="${1:- (%s)}"
>> +
>> +		if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
>> +			if [ "$w" != "*" ]; then
>> +				printf_format="$c_green$printf_format$c_clear"
>> +			else
>> +				printf_format="$c_red$printf_format$c_clear"
>> +			fi
>> +			if [ -n "$i" ]; then
>> +				i="$c_yellow$i$c_clear"
>> +			fi
>> +			if [ -n "$s" ]; then
>> +				s="$c_lblue$i$c_clear"
>> +			fi
>> +			if [ -n "$u" ]; then
>> +				u="$c_purple$i$c_clear"
>> +			fi
>> +		fi
>> +			
>>  		local f="$w$i$s$u"
>> -		printf "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
>> +		echo $(printf "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p")

I'm still in some doubt over this last line, including the color codes
is confusing me...

I'll go ahead and try some more polishing and whatever more comes as
suggestions. Is it ok to bother the list with intermediate stuff in this
thread?

Cheers

Simon

Re: bash completion with colour hints

[Junio C Hamano]

Simon Oosthoek <soosthoek@nieuwland.nl> writes:

> I read the guide and now I have some questions:
>
> - It suggests to use the oldest commit that contains the "bug" and can
> support the fix. This would be the very first mention of __git_ps1
> function I think commit d3d717a4ad0c8d7329e79f7d0313baec57c6b585

You could claim that the lack of coloring is a bug, but nobody
complained about it so far (and I personally hate coloring in
prompts as they are distracting noise, and would reject a patch if
it weren't made conditional), so I would think this is more about
"adding a feature the users can choose to use but they do not have
to".

We do not usually add new features to maintenance tracks, so the
result of applying the patch does not have to be merge-able to maint
or amything older.  I would base the patch on v1.7.12 (the latest
stable release) if I were you.

> - I read that git-prompt.sh is meant to support bash and zsh, I have
> only tested it on bash. Should I attempt to test it on zsh or is there a
> kind person with zsh as his/her shell to test it for me?

That is something you should ask on list, like you did here, but the
most effective way to do so is do so when you send a patch you
worked on and tested with bash.  Say "I've tested it only with bash;
can you please take a look?" and Cc the folks you find in the output
of "git log contrib/completion/" who worked on making it workable
with zsh.

Re: bash completion with colour hints

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 27.09.2012 08:53:
> Simon Oosthoek <soosthoek@nieuwland.nl> writes:
> 
>> I read the guide and now I have some questions:
>>
>> - It suggests to use the oldest commit that contains the "bug" and can
>> support the fix. This would be the very first mention of __git_ps1
>> function I think commit d3d717a4ad0c8d7329e79f7d0313baec57c6b585
> 
> You could claim that the lack of coloring is a bug, but nobody
> complained about it so far (and I personally hate coloring in
> prompts as they are distracting noise, and would reject a patch if
> it weren't made conditional), so I would think this is more about
> "adding a feature the users can choose to use but they do not have
> to".
> 
> We do not usually add new features to maintenance tracks, so the
> result of applying the patch does not have to be merge-able to maint
> or amything older.  I would base the patch on v1.7.12 (the latest
> stable release) if I were you.
> 
>> - I read that git-prompt.sh is meant to support bash and zsh, I have
>> only tested it on bash. Should I attempt to test it on zsh or is there a
>> kind person with zsh as his/her shell to test it for me?

Actually, the instructions in the prompt script are not complete for zsh
(you need to activate expansion in the prompt), and I think there is
some breakage because bash uses "\h" etc. for PS1 format specifiers
whereas zsh uses '%h', and so the '%' in the prompt is a problem for zsh.

> That is something you should ask on list, like you did here, but the
> most effective way to do so is do so when you send a patch you
> worked on and tested with bash.  Say "I've tested it only with bash;
> can you please take a look?" and Cc the folks you find in the output
> of "git log contrib/completion/" who worked on making it workable
> with zsh.
> 

Additionally, there have been several attempts already for prompt. So
I'd suggest you check the list archives to see whether you addressed the
issues which were raised back then. I've actually been toying with that
myself lately. A few hints:

- You need to escape the escapes '\[', i.e. tell the shell that color
escapes produce zero length output, or else line wrapping is distorted.

- Bash interpretes '\' only when PS1 is assigned, not when an expansion
in PS1 produces it.

- Some don't like it colorful, so the coloring needs to depend on a
config setting or env variable.

- Coloring should be consistent with other coloring in Git, such as
'status -s -b'.

- Coloring provides the way to make the prompt characters analogous to
'status -s -b' because a green 'M' is different from red 'M', and is
much clearer then having to remember '+' vs. '*' (so there is a logic in
that).

>From trying myself, I'm convinced that you need a clever combination of
PROMPT_COMMAND and PS1 to make this work. Setting PS1 in PROMPT_COMMAND
is probably a no-go because that makes it difficult to customize PS1. I
have something in the works which reproduces the current prompt but need
to clean it up further. The actual coloring would require setting a lot
of variables which communicate data from PROMPT_COMMAND to PS1, and I
actually don't like that.

An alternative approach would be:

- Tell users to activate git prompt by doing something like

PROMPT_COMMAND='__git_prompt "[\u@\h \W (%s)]\$ '

rather than the current

PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '

- set PS1 from __git_prompt

I'm not sure about the performance implications of re-setting PS1 on
(before) each prompt invocation, though. Would that be OK?

Michael

Re: bash completion with colour hints

[Michael J Gruber]

Michael J Gruber venit, vidit, dixit 27.09.2012 10:53:
...and I just checked that gzipped attached patch. No, it doesn't work
that way at all.

Michael

Re: bash completion with colour hints

[Simon Oosthoek]

On 09/27/2012 10:53 AM, Michael J Gruber wrote:
>> We do not usually add new features to maintenance tracks, so the
>> result of applying the patch does not have to be merge-able to maint
>> or amything older.  I would base the patch on v1.7.12 (the latest
>> stable release) if I were you.
>>

I now have a patch based on 1.7.12

>>> - I read that git-prompt.sh is meant to support bash and zsh, I have
>>> only tested it on bash. Should I attempt to test it on zsh or is there a
>>> kind person with zsh as his/her shell to test it for me?
>
> Actually, the instructions in the prompt script are not complete for zsh
> (you need to activate expansion in the prompt), and I think there is
> some breakage because bash uses "\h" etc. for PS1 format specifiers
> whereas zsh uses '%h', and so the '%' in the prompt is a problem for zsh.

The only feature using it is untracked files, so that should probably be 
changed to work with zsh?

>
> Additionally, there have been several attempts already for prompt. So
> I'd suggest you check the list archives to see whether you addressed the
> issues which were raised back then. I've actually been toying with that
> myself lately. A few hints:

This would take me too much time

>
> - You need to escape the escapes '\[', i.e. tell the shell that color
> escapes produce zero length output, or else line wrapping is distorted.

A: this is something I've run into with my own contraption and I'm not 
sure I fixed it with my patch :-(

Can you give a more complete example of how that should work?

>
> - Bash interpretes '\' only when PS1 is assigned, not when an expansion
> in PS1 produces it.

is that true? how can \w produce the current working dir in the prompt?
Or do you mean it is translated to $PWD in some form internally?

>
> - Some don't like it colorful, so the coloring needs to depend on a
> config setting or env variable.

It is.

>
> - Coloring should be consistent with other coloring in Git, such as
> 'status -s -b'.

That would be nice, I suppose the coloring is configurable for git, so 
the values would need to be dynamically imported from the git config. I 
don't know how to do that.

>
> - Coloring provides the way to make the prompt characters analogous to
> 'status -s -b' because a green 'M' is different from red 'M', and is
> much clearer then having to remember '+' vs. '*' (so there is a logic in
> that).

Are you suggesting to use M instead of * or +? wouldn't that interfere 
with branchnames?

>
>  From trying myself, I'm convinced that you need a clever combination of
> PROMPT_COMMAND and PS1 to make this work. Setting PS1 in PROMPT_COMMAND
> is probably a no-go because that makes it difficult to customize PS1. I
> have something in the works which reproduces the current prompt but need
> to clean it up further. The actual coloring would require setting a lot
> of variables which communicate data from PROMPT_COMMAND to PS1, and I
> actually don't like that.

I think it should be possible in PS1 only.

>
> An alternative approach would be:
>
> - Tell users to activate git prompt by doing something like
>
> PROMPT_COMMAND='__git_prompt "[\u@\h \W (%s)]\$ '
>
> rather than the current
>
> PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
>
> - set PS1 from __git_prompt
>
> I'm not sure about the performance implications of re-setting PS1 on
> (before) each prompt invocation, though. Would that be OK?

I doubt you can set PS1 from a function (effectively a subshell?)

Thanks for your comments

I will send an updated patch later....

/Simon

Re: bash completion with colour hints

[Andreas Schwab]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> - Bash interpretes '\' only when PS1 is assigned, not when an expansion
> in PS1 produces it.

What you probably mean is that bash does not rescan expansions for
backslash escapes, thus only literal occurences in PS1 are processed
(which is consistent with ordinary expansion).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: bash completion with colour hints

[Simon Oosthoek]

On 09/27/2012 10:53 AM, Michael J Gruber wrote:

>
>  From trying myself, I'm convinced that you need a clever combination of
> PROMPT_COMMAND and PS1 to make this work. Setting PS1 in PROMPT_COMMAND
> is probably a no-go because that makes it difficult to customize PS1. I
> have something in the works which reproduces the current prompt but need
> to clean it up further. The actual coloring would require setting a lot
> of variables which communicate data from PROMPT_COMMAND to PS1, and I
> actually don't like that.
>
> An alternative approach would be:
>
> - Tell users to activate git prompt by doing something like
>
> PROMPT_COMMAND='__git_prompt "[\u@\h \W (%s)]\$ '
>
> rather than the current
>
> PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
>
> - set PS1 from __git_prompt
>
> I'm not sure about the performance implications of re-setting PS1 on
> (before) each prompt invocation, though. Would that be OK?

After some research, I completely agree with your assessment!

I don't think the performance issue is that big and it's the only way to 
get the colors in there without messing up word wrapping.

I'm afraid that either this will need a new function just to get the 
coloring hints or that it will break the usage of __git_ps1 entirely (if 
you'd require the user to use it as PROMPT_COMMAND instead of via PS1)

And I suppose this will not be compatible with zsh?

/Simon

PS, I'd rather have colours with wrapping issues, than no colour hints. 
But I realise this has to be fixed before inclusion into the main tree.

[PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Simon Oosthoek]

Hi again

After the previous comments, I decided to attempt it using 
PROMPT_COMMAND rather than calling a function from command substitution 
in PS1. This new code works and doesn't have the wrapping issue anymore.
I've simplified some of the coloring stuff and for now there's no 
parameters to customize the PS1. Users will have to customize the 
function itself for now...
As a small observation, I think it's more logical to use PROMPT_COMMAND, 
since that explicitly uses a function, rather than using command 
substitution when setting PS1.
Obviously, as I didn't remove __git_ps1, the older use is unchanged, but 
now there is about 80% duplication of code between __git_ps1 and 
__git-ps1_pc
And AFAICT zsh is not supported here due to different escape characters 
(\ in bash, % in zsh)
Please let me know if and how this can/should be integrated!

Cheers

Simon




The function can set the PS1 varible optionally with
Colored hints about the state of the tree when
GIT_PS1_SHOWCOLORHINTS is set to a nonempty value.
This version doesn't accept arguments to customize the
prompt. And it has not been tested with zsh.

Signed-off-by: Simon Oosthoek <soosthoek@nieuwland.nl>
---
  contrib/completion/git-prompt.sh |  135 
++++++++++++++++++++++++++++++++++++++
  1 file changed, 135 insertions(+)

diff --git a/contrib/completion/git-prompt.sh 
b/contrib/completion/git-prompt.sh
index 29b1ec9..8ed6b84 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -48,6 +48,16 @@
  # find one, or @{upstream} otherwise.  Once you have set
  # GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
  # setting the bash.showUpstream config variable.
+#
+# If you would like colored hints in the branchname shown in the prompt
+# you can set PROMPT_COMMAND (bash) to __git_ps1_pc and export the +# 
variables GIT_PS1_SHOWDIRTYSTATE and GIT_PS1_SHOWCOLORHINT:
+# example:
+# export GIT_PS1_SHOWDIRTYSTATE=true
+# export GIT_PS1_SHOWCOLORHINT=true
+# export PROMPT_COMMAND=__git_ps1_pc
+# The prompt command function will directly set PS1, in order to +# 
insert color commands in a way that doesn't mess up wrapping.
   # __gitdir accepts 0 or 1 arguments (i.e., location)
  # returns location of .git repo
@@ -287,3 +297,128 @@ __git_ps1 ()
  		printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
  	fi
  }
+
+
+# __git_ps1_pc accepts 0 arguments (for now)
+# It is meant to be used as PROMPT_COMMAND, it sets PS1
+__git_ps1_pc ()
+{
+	local g="$(__gitdir)"
+	if [ -n "$g" ]; then
+		local r=""
+		local b=""
+		if [ -f "$g/rebase-merge/interactive" ]; then
+			r="|REBASE-i"
+			b="$(cat "$g/rebase-merge/head-name")"
+		elif [ -d "$g/rebase-merge" ]; then
+			r="|REBASE-m"
+			b="$(cat "$g/rebase-merge/head-name")"
+		else
+			if [ -d "$g/rebase-apply" ]; then
+				if [ -f "$g/rebase-apply/rebasing" ]; then
+					r="|REBASE"
+				elif [ -f "$g/rebase-apply/applying" ]; then
+					r="|AM"
+				else
+					r="|AM/REBASE"
+				fi
+			elif [ -f "$g/MERGE_HEAD" ]; then
+				r="|MERGING"
+			elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
+				r="|CHERRY-PICKING"
+			elif [ -f "$g/BISECT_LOG" ]; then
+				r="|BISECTING"
+			fi
+
+			b="$(git symbolic-ref HEAD 2>/dev/null)" || {
+
+				b="$(
+				case "${GIT_PS1_DESCRIBE_STYLE-}" in
+				(contains)
+					git describe --contains HEAD ;;
+				(branch)
+					git describe --contains --all HEAD ;;
+				(describe)
+					git describe HEAD ;;
+				(* | default)
+					git describe --tags --exact-match HEAD ;;
+				esac 2>/dev/null)" ||
+
+				b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
+				b="unknown"
+				b="($b)"
+			}
+		fi
+
+		local w=""
+		local i=""
+		local s=""
+		local u=""
+		local c=""
+		local p=""
+
+		if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then
+			if [ "true" = "$(git rev-parse --is-bare-repository 2>/dev/null)" ]; 
then
+				c="BARE:"
+			else
+				b="GIT_DIR!"
+			fi
+		elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" 
]; then
+			if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
+				if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
+					git diff --no-ext-diff --quiet || w="*"
+					if git rev-parse --quiet --verify HEAD >/dev/null; then
+						git diff-index --cached --quiet HEAD -- || i="+"
+					else
+						i="#"
+					fi
+				fi
+			fi
+			if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
+				git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
+			fi
+
+			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
+				if [ -n "$(git ls-files --others --exclude-standard)" ]; then
+					u="%"
+				fi
+			fi
+
+			if [ -n "${GIT_PS1_SHOWUPSTREAM-}" ]; then
+				__git_ps1_show_upstream
+			fi
+		fi
+
+		PS1='\u@\h:\w ('
+		if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
+			local c_red='\e[31m'
+			local c_green='\e[32m'
+			local c_yellow='\e[33m'
+			local c_lblue='\e[1;34m'
+			local c_purple='\e[35m'
+			local c_cyan='\e[36m'
+			local c_clear='\e[0m'
+			local branchstring="$c${b##refs/heads/}"
+			local branch_color="$c_green"
+			local flags_color="$c_cyan"
+
+			if [ "$w" = "*" ]; then
+				branch_color="$c_red"
+			elif [ -n "$i" ]; then
+				branch_color="$c_yellow"
+			fi
+			
+			# Setting PS1 directly with \[ and \] around colors
+			# is necessary to prevent wrapping issues!
+			PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
+			if [ -n "$w$i$s$u" ]; then
+				PS1="$PS1 \[$flags_color\]$w$i$s$u\[$c_clear\]"
+			fi
+		else
+			local f="$w$i$s$u"
+			PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+		fi
+		PS1="$PS1)\$ "
+
+	fi
+}
-- 
1.7.9.5

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Junio C Hamano]

Simon Oosthoek <soosthoek@nieuwland.nl> writes:

> +# __git_ps1_pc accepts 0 arguments (for now)
> +# It is meant to be used as PROMPT_COMMAND, it sets PS1
> +__git_ps1_pc ()
> +{
> +	local g="$(__gitdir)"
> +	if [ -n "$g" ]; then
> +...
> +	fi
> +}

This looks awfully similar to the existing code in __git_ps1
function.  Without refactoring to share the logic between them, it
won't be maintainable.

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Simon Oosthoek]

On 09/28/2012 07:58 PM, Junio C Hamano wrote:
> Simon Oosthoek <soosthoek@nieuwland.nl> writes:
>
>> +# __git_ps1_pc accepts 0 arguments (for now)
>> +# It is meant to be used as PROMPT_COMMAND, it sets PS1
>> +__git_ps1_pc ()
>> +{
>> +	local g="$(__gitdir)"
>> +	if [ -n "$g" ]; then
>> +...
>> +	fi
>> +}
>
> This looks awfully similar to the existing code in __git_ps1
> function.  Without refactoring to share the logic between them, it
> won't be maintainable.
>

I agree that it's ugly. How about the following:

I modified __git_ps1 to work both in PROMPT_COMMAND mode and in that 
mode support color hints.

This way there's one function, so no overlap.

Shall I send patches for the two changes separately (to support 
PROMPT_COMMAND mode and another to support color hints) or in one?

And what about zsh support? I doubt the PROMPT_COMMAND thing is 
compatible with zsh, but the command substitution mode should probably 
work, unless it is already broken by the use of % to indicate untracked 
files (when GIT_PS1_SHOWUNTRACKEDFILES is set). Unless it is tested 
further in zsh, I'd say it might be better not to claim zsh is supported.

Cheers

Simon

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Junio C Hamano]

Simon Oosthoek <soosthoek@nieuwland.nl> writes:

> On 09/28/2012 07:58 PM, Junio C Hamano wrote:
>> Simon Oosthoek <soosthoek@nieuwland.nl> writes:
>>
>>> +# __git_ps1_pc accepts 0 arguments (for now)
>>> +# It is meant to be used as PROMPT_COMMAND, it sets PS1
>>> +__git_ps1_pc ()
>>> +{
>>> +	local g="$(__gitdir)"
>>> +	if [ -n "$g" ]; then
>>> +...
>>> +	fi
>>> +}
>>
>> This looks awfully similar to the existing code in __git_ps1
>> function.  Without refactoring to share the logic between them, it
>> won't be maintainable.
>>
>
> I agree that it's ugly. How about the following:
>
> I modified __git_ps1 to work both in PROMPT_COMMAND mode and in that
> mode support color hints.
>
> This way there's one function, so no overlap.

I think the logical progression would be

 - there are parts of __git_ps1 you want to reuse for your
   __git_ps1_pc; separate that part out as a helper function,
   and make __git_ps1 call it, without changing what __git_ps1
   does (i.e. no colors, etc.)

 - add __git_ps1_pc that uses the helper function you separated
   out.

 - add whatever bells and whistles that are useful for users of
   either __git_ps1 or __git_ps1_pc to that helper function.

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Simon Oosthoek]

On 01/10/12 19:16, Junio C Hamano wrote:
> Simon Oosthoek <soosthoek@nieuwland.nl> writes:
> 
>> On 09/28/2012 07:58 PM, Junio C Hamano wrote:
>>> Simon Oosthoek <soosthoek@nieuwland.nl> writes:
>>>
>>>> +# __git_ps1_pc accepts 0 arguments (for now)
>>>> +# It is meant to be used as PROMPT_COMMAND, it sets PS1
>>>> +__git_ps1_pc ()
>>>> +{
>>>> +	local g="$(__gitdir)"
>>>> +	if [ -n "$g" ]; then
>>>> +...
>>>> +	fi
>>>> +}
>>>
>>> This looks awfully similar to the existing code in __git_ps1
>>> function.  Without refactoring to share the logic between them, it
>>> won't be maintainable.
>>>
>>
>> I agree that it's ugly. How about the following:
>>
>> I modified __git_ps1 to work both in PROMPT_COMMAND mode and in that
>> mode support color hints.
>>
>> This way there's one function, so no overlap.
> 
> I think the logical progression would be
> 
>  - there are parts of __git_ps1 you want to reuse for your
>    __git_ps1_pc; separate that part out as a helper function,
>    and make __git_ps1 call it, without changing what __git_ps1
>    does (i.e. no colors, etc.)
> 
>  - add __git_ps1_pc that uses the helper function you separated
>    out.
> 
>  - add whatever bells and whistles that are useful for users of
>    either __git_ps1 or __git_ps1_pc to that helper function.
> 
> 

Since this is the shell, it could turn out to be ugly this way:

function ps1_common {
	set global variable (branchname, dirty state, untracked files,
divergence from upstream, etc.)
}

function __git_ps1 {
	call ps1_common
	print PS1 string based on format string or default (color in prompt
isn't an option) and include global variables where necessary
}

function __git_ps1_pc {
	call ps1_common
	set PS1=....
	based on hardcoded PS1 definition, expand the PS1 using global
variables (add color if requested)
}

The problem I see is that it would involve a lot of global variables
visible in the shell (OTOH, they could be used by other scripts, but
only when using this prompt ;-).
Or you could print a string, which can be caught in the relevant _ps1
function, but then this string needs to be chopped up and processed (by
a bunch of awk oneliners or bash-voodoo)

The latter introduces so much overhead (in processing and
maintainability) that I think it should not be considered.

That leaves passing the variables from one function to another using
globally scoped variables in the shell.

It can be done, but the current combined implementation has only the
global variables set by the user (GIT_PS1_SHOWDIRTYSTATE and so on) and
the rest remains local to the function.

The main obstacle to better code here is the need to work around the
shell's oddities. :-(

Cheers

Simon

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> I agree that it's ugly. How about the following:
>>
>> I modified __git_ps1 to work both in PROMPT_COMMAND mode and in that
>> mode support color hints.
>>
>> This way there's one function, so no overlap.
>
> I think the logical progression would be
>
>  - there are parts of __git_ps1 you want to reuse for your
>    __git_ps1_pc; separate that part out as a helper function,
>    and make __git_ps1 call it, without changing what __git_ps1
>    does (i.e. no colors, etc.)
>
>  - add __git_ps1_pc that uses the helper function you separated
>    out.
>
>  - add whatever bells and whistles that are useful for users of
>    either __git_ps1 or __git_ps1_pc to that helper function.

Hrm, let me ask a stupid question.  Why do we even need __git_ps1_pc
in the first place?  Wouldn't it be just the matter of

	PROMPT_COMMAND='__git_ps1 "%s"'

once you have __git_ps1 that works?

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Simon Oosthoek]

On 01/10/12 21:13, Junio C Hamano wrote:

> Hrm, let me ask a stupid question.  Why do we even need __git_ps1_pc
> in the first place?  Wouldn't it be just the matter of
> 
> 	PROMPT_COMMAND='__git_ps1 "%s"'
> 
> once you have __git_ps1 that works?

Apart from one small detail, PS1 must be set directly when __git_ps1 is
called as a PROMPT_COMMAND, while in command substitution mode,
__git_ps1 needs to put out a string value to substitute...

This is the way it works now (I'll send the patch later this week, I
want to test it some more...)

/Simon

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Junio C Hamano]

Simon Oosthoek <soosthoek@nieuwland.nl> writes:

> On 01/10/12 21:13, Junio C Hamano wrote:
>
>> Hrm, let me ask a stupid question.  Why do we even need __git_ps1_pc
>> in the first place?  Wouldn't it be just the matter of
>> 
>> 	PROMPT_COMMAND='__git_ps1 "%s"'
>> 
>> once you have __git_ps1 that works?
>
> Apart from one small detail, PS1 must be set directly when __git_ps1 is
> called as a PROMPT_COMMAND, while in command substitution mode,
> __git_ps1 needs to put out a string value to substitute...

Now you lost me.  The documentation of PROMPT_COMMAND in "man bash"
says this:

       PROMPT_COMMAND
              If set, the value is executed as a command prior to
              issuing each primary prompt.

So yes, if you say "PROMPT_COMMAND='whatever'", you will get output
from 'whatever' followed by what $PS1 would normally give you.  
If you do not want to see PS1 after 'whatever' gives you, you have
to set it to an empty string.

On the other hand, they way people have been using __git_ps1 is (as
described in the prompt script) to do something like this:

	PS1='...cruft... $(__git_ps1 "%s") ...cruft...'

To keep supporting them, __git_ps1 has to be a function that writes
the prompt string to its standard output.  The external interface of
PROMPT_COMMAND also is that it wants a command that emits the string
desired for the prompt to its standard output.  I do not see any
"when it is used like this, X, but when it is used like that, Y"
kind of issue around it, either.

So what is the problem????

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Simon Oosthoek]

On 01/10/12 21:54, Junio C Hamano wrote:
> Now you lost me.  The documentation of PROMPT_COMMAND in "man bash"
> says this:
> 
>        PROMPT_COMMAND
>               If set, the value is executed as a command prior to
>               issuing each primary prompt.
> 
> So yes, if you say "PROMPT_COMMAND='whatever'", you will get output
> from 'whatever' followed by what $PS1 would normally give you.  
> If you do not want to see PS1 after 'whatever' gives you, you have
> to set it to an empty string.
> 
> On the other hand, they way people have been using __git_ps1 is (as
> described in the prompt script) to do something like this:
> 
> 	PS1='...cruft... $(__git_ps1 "%s") ...cruft...'
> 
> To keep supporting them, __git_ps1 has to be a function that writes
> the prompt string to its standard output.  The external interface of
> PROMPT_COMMAND also is that it wants a command that emits the string
> desired for the prompt to its standard output.  I do not see any
> "when it is used like this, X, but when it is used like that, Y"
> kind of issue around it, either.
> 
> So what is the problem????
> 

Well, I hadn't thought about that way of using it. It works in a way...

But PS1 is set and interpreted in a special way by bash (I gather from
examples, I'm kind of confused by it).

It's possible to set PS1 to nothing and print a string from
PROMPT_COMMAND, but then you miss out on all the features of the PS1
interpretation by bash and compared to the use of __git_ps1 at the
moment, it has to put out quite a different string. Because if you like
to see user@host+workdir (git-status)[$#]
the current users of __git_ps1 say PS1="\u@host+\w $(__git_ps1 "%s")\$
", but all __git+ps1 has to put out is "(branch)" or "(branch *)", etc.

If it has to print the same prompt in PC mode, it has to add all the
user/host/workdir/[$#] data as well, withouth being able to use the bash
internal interpretation (because that is only working when PS1 is set).

The example(s) I found when googling for a solution were to set PS1
inside the PC function, in a way that it was possible to add color
encodings, without messing up the wrapping. This is _impossible_ using
command substitution, because then bash doesn't interpret the \[ and \]
around the color codes, and that messes up the accounting of how long
the prompt string is.

/Simon

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Junio C Hamano]

Simon Oosthoek <s.oosthoek@xs4all.nl> writes:

> It's possible to set PS1 to nothing and print a string from
> PROMPT_COMMAND, but then you miss out on all the features of the PS1
> interpretation by bash and compared to the use of __git_ps1 at the
> moment, it has to put out quite a different string. Because if you like
> to see user@host+workdir (git-status)[$#]
> the current users of __git_ps1 say PS1="\u@host+\w $(__git_ps1 "%s")\$
> ", but all __git+ps1 has to put out is "(branch)" or "(branch *)", etc.
>
> If it has to print the same prompt in PC mode, it has to add all the
> user/host/workdir/[$#] data as well, withouth being able to use the bash
> internal interpretation (because that is only working when PS1 is set).

The longer I read your explanation, the less useful the "PC mode"
sounds like, at least to me.  So why does an user even want to use
such a mechanism, instead of PS1?  And even if the user wants to use
it by doing \w, \u etc. himself, she can do that with

	PROMPT_COMMAND='
		PS1=$(printf "\u \h \w %s$ " $(__git_ps1 "%s"))
        '

just fine, no?

So I still do not see the problem, even taking your "Set PS1 in the
command, without spitting anything out of the command" use case into
account.

Confused....

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 01.10.2012 23:09:
> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
> 
>> It's possible to set PS1 to nothing and print a string from
>> PROMPT_COMMAND, but then you miss out on all the features of the PS1
>> interpretation by bash and compared to the use of __git_ps1 at the
>> moment, it has to put out quite a different string. Because if you like
>> to see user@host+workdir (git-status)[$#]
>> the current users of __git_ps1 say PS1="\u@host+\w $(__git_ps1 "%s")\$
>> ", but all __git+ps1 has to put out is "(branch)" or "(branch *)", etc.
>>
>> If it has to print the same prompt in PC mode, it has to add all the
>> user/host/workdir/[$#] data as well, withouth being able to use the bash
>> internal interpretation (because that is only working when PS1 is set).
> 
> The longer I read your explanation, the less useful the "PC mode"
> sounds like, at least to me.  So why does an user even want to use
> such a mechanism, instead of PS1?  And even if the user wants to use
> it by doing \w, \u etc. himself, she can do that with
> 
> 	PROMPT_COMMAND='
> 		PS1=$(printf "\u \h \w %s$ " $(__git_ps1 "%s"))
>         '
> 
> just fine, no?
> 
> So I still do not see the problem, even taking your "Set PS1 in the
> command, without spitting anything out of the command" use case into
> account.
> 
> Confused....
> 

The "problem" (as far as I see) is only: What user interface do we want
to expose to the user, or rather, do we want to expose the user to ;)

So far, we say:

#    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
#    2) Add the following line to your .bashrc/.zshrc:
#        source ~/.git-prompt.sh
#    3) Change your PS1 to also show the current branch:
#         Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
#         ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '

(That's incomplete for zsh, but I'm digressing...). With the above, we
would change the bash instruction to

PROMPT_COMMAND='
	PS1=$(printf "\u \h \w %s$ " $(__git_ps1 "%s"))
       '

which may look more complicated to some. I think we can aim for
something like

PROMPT_COMMAND='__git_ps1_color "\u \h \w%s$ " " (%s)"'

so that the first arg is a PS1 string, %s there gets replaced by the git
prompt (if any), and the second arg defines the formatting of the git
prompt. This makes the "ui" (what you have to set the shells vars to) as
simple as possible.

As I said, I had done some fixes and refactoring to that effect already,
but I'm not going to race Simon.

Michael

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Simon Oosthoek]

On 10/02/2012 09:38 AM, Michael J Gruber wrote:
>> The longer I read your explanation, the less useful the "PC mode"
>> sounds like, at least to me.  So why does an user even want to use
>> such a mechanism, instead of PS1?  And even if the user wants to use
>> it by doing \w, \u etc. himself, she can do that with
>>
>> 	PROMPT_COMMAND='
>> 		PS1=$(printf "\u \h \w %s$ " $(__git_ps1 "%s"))
>>          '
>>
>> just fine, no?

Well, in that way, it doesn't work and setting PS1 directly would be 
better. But this way you cannot do colors.

>>
>> Confused....

I know the feeling ;-)

>
> The "problem" (as far as I see) is only: What user interface do we want
> to expose to the user, or rather, do we want to expose the user to ;)
>
> So far, we say:
>
> #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
> #    2) Add the following line to your .bashrc/.zshrc:
> #        source ~/.git-prompt.sh
> #    3) Change your PS1 to also show the current branch:
> #         Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
> #         ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
>
> (That's incomplete for zsh, but I'm digressing...). With the above, we
> would change the bash instruction to
>
> PROMPT_COMMAND='
> 	PS1=$(printf "\u \h \w %s$ " $(__git_ps1 "%s"))
>         '
>
> which may look more complicated to some. I think we can aim for
> something like
>
> PROMPT_COMMAND='__git_ps1_color "\u \h \w%s$ " " (%s)"'
>
> so that the first arg is a PS1 string, %s there gets replaced by the git
> prompt (if any), and the second arg defines the formatting of the git
> prompt. This makes the "ui" (what you have to set the shells vars to) as
> simple as possible.
>
> As I said, I had done some fixes and refactoring to that effect already,
> but I'm not going to race Simon.

I don't quite see how you're going to smurf the formatted string into 
the PS1 string at %s...

Wouldn't it be easier in this case to have the user provide 3 arguments:

PROMPT_COMMAND='__git_ps1 "\u@\h:\w " "(%s)" "\$ "'

The first and last can be directly assigned to PS1 and the format string 
can be put in the middle.

BUT. In this way, it is still impossible to change the colors inside the 
format string, unless you ignore that when the user requested color 
hints. Because the output of printf will not be interpreted by bash when 
put in via command substitution.

The whole point of this exercise (for me at least) was to be able to 
color different parts of the bit that __git_ps1 put in, based on the 
current state of the tree.

I think the user interface could be something like:

use either:
PROMPT_COMMAND='__git_ps1 "\u@\h:\w " "\$ "' # 2 args!
or
PS1="\u@\h:\w $(__git_ps1 '(%s)')\$ "

(I just realised that I still need to handle the case where both PS1 and 
PROMPT_COMMAND contain __git_ps1 ;-)

Note that in the PS1 case, no color codes should be put into the string, 
as they will mangle the prompt length and wrapping problems start happening.

Cheers

Simon.

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Junio C Hamano venit, vidit, dixit 01.10.2012 23:09:
>
>> Confused....
>> 
>
> The "problem" (as far as I see) is only: What user interface do we want
> to expose to the user, or rather, do we want to expose the user to ;)
>
> So far, we say:
>
> #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
> #    2) Add the following line to your .bashrc/.zshrc:
> #        source ~/.git-prompt.sh
> #    3) Change your PS1 to also show the current branch:
> #         Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
> #         ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
>
> (That's incomplete for zsh, but I'm digressing...). With the above, we
> would change the bash instruction to
>
> PROMPT_COMMAND='
> 	PS1=$(printf "\u \h \w %s$ " $(__git_ps1 "%s"))
>        '
>
> which may look more complicated to some. I think we can aim for
> something like
>
> PROMPT_COMMAND='__git_ps1_color "\u \h \w%s$ " " (%s)"'

If your goal is to use PROMPT_COMMAND and not PS1, then yes between
the two definitions of PROMPT_COMMAND above, the latter may look
simpler.  But that does not explain why you want to prefer it over
PS1 in the first place, which was the central point of my question
that still has not been answered.

Even more confused...

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Simon Oosthoek]

On 02/10/12 19:01, Junio C Hamano wrote:
> If your goal is to use PROMPT_COMMAND and not PS1, then yes between
> the two definitions of PROMPT_COMMAND above, the latter may look
> simpler.  But that does not explain why you want to prefer it over
> PS1 in the first place, which was the central point of my question
> that still has not been answered.
> 
> Even more confused...
> 

The specific answer to your question is that without using
PROMPT_COMMAND it doesn't seem to be possible to use colors based on the
state of the git tree. The reason being that in order to prevent
wrapping problems on the command line (specifically going up/down the
history list in bash or line wrapping on a long command). This is
prevented (and quite the norm in static PS1 strings) by enclosing the
terminal code for color inside \[ and \] so bash doesn't count these and
what is in between them in the length of the prompt string.

The only way to get the colors in without messing up normal functioning
of the prompt is to use PROMPT_COMMAND to set PS1, with colors based
dynamically on the state of the tree.

In my current version (which I haven't had time to properly send out,
sorry!) it can do both using the same __git_ps1 function. But only color
in the PROMPT_COMMAND mode for the reason laid out above.

Having said that, I think there's another benefit to using
PROMPT_COMMAND; I think it is more elegant to use a function via
PROMPT_COMMAND (now that I know of it), than using a function via
command substitution inside the PS1.

I guess your (and my) confusion is more caused by unfamiliarity with
this feature of bash than it being used. Is that correct?

*desperately trying to get out of confused mode*

Cheers

Simon

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Junio C Hamano]

Simon Oosthoek <s.oosthoek@xs4all.nl> writes:

> ... This is
> prevented (and quite the norm in static PS1 strings) by enclosing the
> terminal code for color inside \[ and \] so bash doesn't count these and
> what is in between them in the length of the prompt string.

Ah, OK, and these \[ things \] behave like other magics like \h and \W
in PS1 in that PS1='\h $(echo_bs_w)' will not work as expected with

	echo_bs_w () {
		printf "%s" "\\W"
	}

hence cannot be used in __git_ps1 function that is called as $(command
substitution).

That was what I was missing.

> The only way to get the colors in without messing up normal functioning
> of the prompt is to use PROMPT_COMMAND to set PS1, with colors based
> dynamically on the state of the tree.

OK.  Thanks for explanation.

[PATCH 1/2] Allow __git_ps1 to be used in PROMPT_COMMAND

[Simon Oosthoek]

changes __git_ps1 to not just allow use in setting PS1
with __git_ps1 in a command substitution, but also allows
__git_ps1 to be used as PROMPT_COMMAND in bash.
This has advantages for using color and I think it is more
elegant

Signed-off-by: Simon Oosthoek <soosthoek@nieuwland.nl>
---
 contrib/completion/git-prompt.sh |   51 +++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 29b1ec9..c50c94a 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -10,9 +10,14 @@
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
 #    2) Add the following line to your .bashrc/.zshrc:
 #        source ~/.git-prompt.sh
-#    3) Change your PS1 to also show the current branch:
-#         Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
-#         ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
+#    3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
+#        To customize the prompt, provide start/end arguments
+#        PROMPT_COMMAND="__git_ps1 '\u@\h:\w (' ')\$ '"
+#    3b) Alternatively change your PS1 to call __git_ps1 as
+#        command-substitution:
+#        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
+#        ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
+#        the optional argument will be used as format string
 #
 # The argument to __git_ps1 will be displayed only if you are currently
 # in a git repository.  The %s token will be the name of the current
@@ -194,11 +199,41 @@ __git_ps1_show_upstream ()
 
 
 # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
-# returns text to add to bash PS1 prompt (includes branch name)
+# when called from PS1 using command substitution
+# in this mode it returns text to add to bash PS1 prompt (includes branch name) or
+# __git_ps1 accepts 0 or 2 arguments when called from PROMPT_COMMAND
+# in that case it _sets_ PS1. The arguments are parts of a PS1 string.
+# when both arguments are given, the first is prepended and the second appended
+# to the state string when assigned to PS1, otherwise default start/end strings
+# are used.
 __git_ps1 ()
 {
+	local pcmode=yes
+	local ps1pc_start='\u@\h:\w '
+	local ps1pc_end='\$ '
+
+	case "$PROMPT_COMMAND" in
+		__git_ps1*)
+			if [ $# = 2 ]; then
+				ps1pc_start="$1"
+				ps1pc_end="$2"
+			fi
+			case "$PS1" in
+			*__git_ps1*)
+				echo '^[[31m__git_ps1: overwriting PS1 due to PROMPT_COMMAND^[[0m'
+			;;
+			esac
+		;;
+		*)  pcmode=no ;; #no output
+	esac
+
 	local g="$(__gitdir)"
-	if [ -n "$g" ]; then
+	if [ -z "$g" ]; then
+		if [ $pcmode = yes ]; then
+			#In PC mode PS1 always needs to be set
+			PS1="$ps1pc_start$ps1pc_end"
+		fi
+	else
 		local r=""
 		local b=""
 		if [ -f "$g/rebase-merge/interactive" ]; then
@@ -284,6 +319,10 @@ __git_ps1 ()
 		fi
 
 		local f="$w$i$s$u"
-		printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
+		if [ $pcmode = yes ]; then
+			PS1="$ps1pc_start($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
+		elif [ $pcmode = no ]; then
+			printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
+		fi
 	fi
 }
-- 
1.7.9.5

Re: [PATCH 1/2] Allow __git_ps1 to be used in PROMPT_COMMAND

[Junio C Hamano]

Simon Oosthoek <s.oosthoek@xs4all.nl> writes:

> changes __git_ps1 to not just allow use in setting PS1
> with __git_ps1 in a command substitution, but also allows
> __git_ps1 to be used as PROMPT_COMMAND in bash.
> This has advantages for using color and I think it is more
> elegant

Is "and I think" necessary?  I do not think it matters what _you_
think when judging it is worth including in the future releases ;-)

A lot more important thing to say is why it has advantages for using
color (remember, it took a few rounds of back and forth with me).
Unless you are going to explain the same to all the people who are
reading the "git log" output 6 months down the road, that is a more
appropriate thing to write here.

>  contrib/completion/git-prompt.sh |   51 +++++++++++++++++++++++++++++++++-----
>  1 file changed, 45 insertions(+), 6 deletions(-)
>
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index 29b1ec9..c50c94a 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -10,9 +10,14 @@
>  #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
>  #    2) Add the following line to your .bashrc/.zshrc:
>  #        source ~/.git-prompt.sh
> -#    3) Change your PS1 to also show the current branch:
> -#         Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
> -#         ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
> +#    3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
> +#        To customize the prompt, provide start/end arguments
> +#        PROMPT_COMMAND="__git_ps1 '\u@\h:\w (' ')\$ '"
> +#    3b) Alternatively change your PS1 to call __git_ps1 as
> +#        command-substitution:
> +#        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
> +#        ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
> +#        the optional argument will be used as format string
>  #
>  # The argument to __git_ps1 will be displayed only if you are currently
>  # in a git repository.  The %s token will be the name of the current
> @@ -194,11 +199,41 @@ __git_ps1_show_upstream ()
>  
>  
>  # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> +# when called from PS1 using command substitution
> +# in this mode it returns text to add to bash PS1 prompt (includes branch name) or
> +# __git_ps1 accepts 0 or 2 arguments when called from PROMPT_COMMAND
> +# in that case it _sets_ PS1. The arguments are parts of a PS1 string.
> +# when both arguments are given, the first is prepended and the second appended
> +# to the state string when assigned to PS1, otherwise default start/end strings
> +# are used.

Sorry, but I cannot parse this.  Is this meant to be a two-item list,
one describing the command substitution mode (zero or 1 arguments) and
the other describing the prompt command mode?  If so, perhaps replacing
the " or" at the end of the first item with ".\n#\n" would make it readable.

>  __git_ps1 ()
>  {
> +	local pcmode=yes
> +	local ps1pc_start='\u@\h:\w '
> +	local ps1pc_end='\$ '
> +
> +	case "$PROMPT_COMMAND" in
> +		__git_ps1*)
> +			if [ $# = 2 ]; then
> +				ps1pc_start="$1"
> +				ps1pc_end="$2"
> +			fi
> +			case "$PS1" in
> +			*__git_ps1*)
> +				echo '__git_ps1: overwriting PS1 due to PROMPT_COMMAND'

Is this supposed to be an error and/or warning message?  Why is it
worth warning only when you are overwriting __git_ps1 style of PS1
and not other user customization?

> +			;;
> +			esac
> +		;;
> +		*)  pcmode=no ;; #no output
> +	esac

Please align outer "case" "its arms)" and "esac" at the same column,
like you did for the inner "case/esac".

Auto-detetction based on PROMPT_COMMAND is a flaky approach.  In
practice, nobody will call PROMPT_COMMAND with the __git_ps1 without
any parameter (100% people want their prompt to end with some sort
of whitespace so they want the "what comes after what is computed",
aka $2), and even more importantly, nobody has been relying on use
of 0 argument form of __git_ps1 in PROMPT_COMMAND.  So why not
always require 2 args and take that as a cue to go into the pc mode?

> +
>  	local g="$(__gitdir)"
> -	if [ -n "$g" ]; then
> +	if [ -z "$g" ]; then
> +		if [ $pcmode = yes ]; then
> +			#In PC mode PS1 always needs to be set
> +			PS1="$ps1pc_start$ps1pc_end"
> +		fi
> +	else
>  		local r=""
>  		local b=""
>  		if [ -f "$g/rebase-merge/interactive" ]; then
> @@ -284,6 +319,10 @@ __git_ps1 ()
>  		fi
>  
>  		local f="$w$i$s$u"
> -		printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
> +		if [ $pcmode = yes ]; then
> +			PS1="$ps1pc_start($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
> +		elif [ $pcmode = no ]; then
> +			printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
> +		fi

Are there $pcmode other than yes or no?  Why not just "else",
instead of performing the test twice?

>  	fi
>  }

Re: [PATCH 1/2] Allow __git_ps1 to be used in PROMPT_COMMAND

[Simon Oosthoek]

Hi Junio

thanks for your feedback!

On 08/10/12 20:12, Junio C Hamano wrote:
> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
> 
>> changes __git_ps1 to not just allow use in setting PS1
>> with __git_ps1 in a command substitution, but also allows
>> __git_ps1 to be used as PROMPT_COMMAND in bash.
>> This has advantages for using color and I think it is more
>> elegant
> 
> Is "and I think" necessary?  I do not think it matters what _you_
> think when judging it is worth including in the future releases ;-)

Hmm, right :-P

How about "This has advantages for using color without running into
prompt-wrapping issues. Only by assigning \[ and \] to PS1 directly can
bash know these and the color codes in between don't count in the length
of the prompt."?

I'll rewrite the patch later on...

Isn't it confusing that the color codes don't figure in this patch at all?

>>  
>>  # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
>> +# when called from PS1 using command substitution
>> +# in this mode it returns text to add to bash PS1 prompt (includes branch name) or
>> +# __git_ps1 accepts 0 or 2 arguments when called from PROMPT_COMMAND
>> +# in that case it _sets_ PS1. The arguments are parts of a PS1 string.
>> +# when both arguments are given, the first is prepended and the second appended
>> +# to the state string when assigned to PS1, otherwise default start/end strings
>> +# are used.
> 
> Sorry, but I cannot parse this.  Is this meant to be a two-item list,
> one describing the command substitution mode (zero or 1 arguments) and
> the other describing the prompt command mode?  If so, perhaps replacing
> the " or" at the end of the first item with ".\n#\n" would make it readable.

I agree, that would make it more readable.

> 
>>  __git_ps1 ()
>>  {
>> +	local pcmode=yes
>> +	local ps1pc_start='\u@\h:\w '
>> +	local ps1pc_end='\$ '
>> +
>> +	case "$PROMPT_COMMAND" in
>> +		__git_ps1*)
>> +			if [ $# = 2 ]; then
>> +				ps1pc_start="$1"
>> +				ps1pc_end="$2"
>> +			fi
>> +			case "$PS1" in
>> +			*__git_ps1*)
>> +				echo '__git_ps1: overwriting PS1 due to PROMPT_COMMAND'
> 
> Is this supposed to be an error and/or warning message?  Why is it
> worth warning only when you are overwriting __git_ps1 style of PS1
> and not other user customization?

That's what this is doing, I wasn't able to make it the other way
around. I thought that I could detect when PS1 contained __git_ps1 that
it shouldn't overwrite the PS1, which worked, but when PROMPT_COMMAND
was set with __git_ps1 and PS1=$(__git_ps1) as well, it gave double output.
As PROMPT_COMMAND is relatively obscure I thought a warning when
overwriting the PS1 was good, but not unless there was a real conflict
(double call to __git_ps1).
The warning is only shown once.

I don't think it would be possible to detect any other kind of
customization without including specific code for it. Do you think it's
unnecessary to include a warning? (I think it would take people a long
time to figure out why their prompt goes whoopsy without getting a hint
that PROMPT_COMMAND messes up the PS1)

(OTOH, if you configure PROMPT_COMMAND, you're bound to know a little
bit about what you're doing...)

Perhaps it's better to just do it.


> 
>> +			;;
>> +			esac
>> +		;;
>> +		*)  pcmode=no ;; #no output
>> +	esac
> 
> Please align outer "case" "its arms)" and "esac" at the same column,
> like you did for the inner "case/esac".

ok

> 
> Auto-detetction based on PROMPT_COMMAND is a flaky approach.  In
> practice, nobody will call PROMPT_COMMAND with the __git_ps1 without
> any parameter (100% people want their prompt to end with some sort
> of whitespace so they want the "what comes after what is computed",
> aka $2), and even more importantly, nobody has been relying on use
> of 0 argument form of __git_ps1 in PROMPT_COMMAND.  So why not
> always require 2 args and take that as a cue to go into the pc mode?

That's a good idea, I'll change that.


>> +		if [ $pcmode = yes ]; then
>> +			PS1="$ps1pc_start($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
>> +		elif [ $pcmode = no ]; then
>> +			printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
>> +		fi
> 
> Are there $pcmode other than yes or no?  Why not just "else",
> instead of performing the test twice?

I forgot to remove that one, I had a default at some point ;-)

Cheers

Simon.

Re: [PATCH 1/2] Allow __git_ps1 to be used in PROMPT_COMMAND

[Junio C Hamano]

Simon Oosthoek <s.oosthoek@xs4all.nl> writes:

> How about "This has advantages for using color without running
> into prompt-wrapping issues. Only by assigning \[ and \] to PS1
> directly can bash know these and the color codes in between don't
> count in the length of the prompt."?
>
> I'll rewrite the patch later on...
>
> Isn't it confusing that the color codes don't figure in this patch at all?

Since the "Subject:" is "allow ... to be used in PROMPT_COMMAND", it
very much is relevant to mention that we need to use PROMPT_COMMAND
in order to correctly use \[ and \] to generated zero-width escape
sequences, which cannot be done by simply setting PS1 to command
substitution.

"color" is just an example; if it makes you uneasy to say "color",
you can say zero-width escape sequences.  It could be \007 (BEL) ;-)

> I don't think it would be possible to detect any other kind of
> customization without including specific code for it. Do you think it's
> unnecessary to include a warning? (I think it would take people a long
> time to figure out why their prompt goes whoopsy without getting a hint
> that PROMPT_COMMAND messes up the PS1)

I think you need your warning _only_ because you attempt to
auto-detect it, when you do not have to.

> (OTOH, if you configure PROMPT_COMMAND, you're bound to know a little
> bit about what you're doing...)

Yup.

[PATCH 1/2] Allow __git_ps1 to be used in PROMPT_COMMAND

[Simon Oosthoek]

Changes __git_ps1 to allow its use as PROMPT_COMMAND in bash
in addition to setting PS1 with __git_ps1 in a command substitution.
PROMPT_COMMAND has advantages for using color without running
into prompt-wrapping issues. Only by assigning \[ and \] to PS1
directly can bash know that these and the enclosed zero-width codes in
between don't count in the length of the prompt.

Signed-off-by: Simon Oosthoek <s.oosthoek@xs4all.nl>
---
 contrib/completion/git-prompt.sh |   51 +++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 29b1ec9..4fbc1e6 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -10,9 +10,14 @@
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
 #    2) Add the following line to your .bashrc/.zshrc:
 #        source ~/.git-prompt.sh
-#    3) Change your PS1 to also show the current branch:
-#         Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
-#         ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
+#    3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
+#        To customize the prompt, provide start/end arguments
+#        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
+#    3b) Alternatively change your PS1 to call __git_ps1 as
+#        command-substitution:
+#        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
+#        ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
+#        the optional argument will be used as format string
 #
 # The argument to __git_ps1 will be displayed only if you are currently
 # in a git repository.  The %s token will be the name of the current
@@ -194,11 +199,39 @@ __git_ps1_show_upstream ()
 
 
 # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
-# returns text to add to bash PS1 prompt (includes branch name)
+# when called from PS1 using command substitution
+# in this mode it prints text to add to bash PS1 prompt (includes branch name)
+#
+# __git_ps1 requires 2 arguments when called from PROMPT_COMMAND (pc)
+# in that case it _sets_ PS1. The arguments are parts of a PS1 string.
+# when both arguments are given, the first is prepended and the second appended
+# to the state string when assigned to PS1.
 __git_ps1 ()
 {
+	local pcmode=no
+	#defaults/examples:
+	local ps1pc_start='\u@\h:\w '
+	local ps1pc_end='\$ '
+	local printf_format='(%s)'
+
+	case "$#" in
+		2)	pcmode=yes
+			ps1pc_start="$1"
+			ps1pc_end="$2"
+		;;
+		0|1)	printf_format="${1:-$printf_format}"
+		;;
+		*)	return
+		;;
+	esac
+
 	local g="$(__gitdir)"
-	if [ -n "$g" ]; then
+	if [ -z "$g" ]; then
+		if [ $pcmode = yes ]; then
+			#In PC mode PS1 always needs to be set
+			PS1="$ps1pc_start$ps1pc_end"
+		fi
+	else
 		local r=""
 		local b=""
 		if [ -f "$g/rebase-merge/interactive" ]; then
@@ -284,6 +317,12 @@ __git_ps1 ()
 		fi
 
 		local f="$w$i$s$u"
-		printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
+		if [ $pcmode = yes ]; then
+			PS1="$ps1pc_start("
+			PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+			PS1="$PS1)$ps1pc_end"
+		else
+			printf -- "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p"
+		fi
 	fi
 }
-- 
1.7.9.5

Re: [PATCH 1/2] Allow __git_ps1 to be used in PROMPT_COMMAND

[Junio C Hamano]

Simon Oosthoek <s.oosthoek@xs4all.nl> writes:

>  __git_ps1 ()
>  {
> +	local pcmode=no
> +	#defaults/examples:
> +	local ps1pc_start='\u@\h:\w '
> +	local ps1pc_end='\$ '
> +	local printf_format='(%s)'
> + ...

This conversion is wrong, given that ...

> @@ -284,6 +317,12 @@ __git_ps1 ()
>  		fi
>  
>  		local f="$w$i$s$u"
> -		printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"

... the original has a SP in front.  I've tentatively queued a
fix-up patch on the topic branch before merging it to 'pu'.

> +		if [ $pcmode = yes ]; then
> +			PS1="$ps1pc_start("
> +			PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
> +			PS1="$PS1)$ps1pc_end"
> +		else
> +			printf -- "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p"
> +		fi
>  	fi
>  }

[PATCH 2/2] show color hints based on state of the git tree

[Simon Oosthoek]

By setting GIT_PS1_SHOW_COLORHINTS when using __git_ps1
as PROMPT_COMMAND, you will get color hints in addition to
a different character (*+% etc.) to indicate the state of
the tree.

Signed-off-by: Simon Oosthoek <s.oosthoek@xs4all.nl>
---
 contrib/completion/git-prompt.sh |   36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4fbc1e6..a693565 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -53,6 +53,12 @@
 # find one, or @{upstream} otherwise.  Once you have set
 # GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
 # setting the bash.showUpstream config variable.
+#
+# If you would like a colored hint about the current dirty state, set
+# GIT_PS1_SHOWCOLORHINTS to a nonempty value. When tracked files are
+# modified, the branch name turns red, when all modifications are staged
+# the branch name turns yellow and when all changes are checked in, the
+# color changes to green. The colors are currently hardcoded in the function.
 
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
@@ -206,6 +212,7 @@ __git_ps1_show_upstream ()
 # in that case it _sets_ PS1. The arguments are parts of a PS1 string.
 # when both arguments are given, the first is prepended and the second appended
 # to the state string when assigned to PS1.
+# In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true
 __git_ps1 ()
 {
 	local pcmode=no
@@ -319,9 +326,36 @@ __git_ps1 ()
 		local f="$w$i$s$u"
 		if [ $pcmode = yes ]; then
 			PS1="$ps1pc_start("
-			PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+			if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
+				local c_red='\e[31m'
+				local c_green='\e[32m'
+				local c_yellow='\e[33m'
+				local c_lblue='\e[1;34m'
+				local c_purple='\e[35m'
+				local c_cyan='\e[36m'
+				local c_clear='\e[0m'
+				local branchstring="$c${b##refs/heads/}"
+				local branch_color="$c_green"
+				local flags_color="$c_cyan"
+
+				if [ "$w" = "*" ]; then
+					branch_color="$c_red"
+				elif [ -n "$i" ]; then
+					branch_color="$c_yellow"
+				fi
+
+				# Setting PS1 directly with \[ and \] around colors
+				# is necessary to prevent wrapping issues!
+				PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
+				if [ -n "$f" ]; then
+					PS1="$PS1 \[$flags_color\]$f\[$c_clear\]"
+				fi
+			else
+				PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+			fi
 			PS1="$PS1)$ps1pc_end"
 		else
+			# NO color option unless in PROMPT_COMMAND mode
 			printf -- "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p"
 		fi
 	fi
-- 
1.7.9.5

[PATCH 1/2] Allow __git_ps1 to be used in PROMPT_COMMAND

[Simon Oosthoek]

Apologies if you receive this e-mail multiple times, my MTA was misconfigured...


Changes __git_ps1 to allow its use as PROMPT_COMMAND in bash
in addition to setting PS1 with __git_ps1 in a command substitution.
PROMPT_COMMAND has advantages for using color without running
into prompt-wrapping issues. Only by assigning \[ and \] to PS1
directly can bash know that these and the enclosed zero-width codes in
between don't count in the length of the prompt.

Signed-off-by: Simon Oosthoek <s.oosthoek@xs4all.nl>
---
 contrib/completion/git-prompt.sh |   51 +++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 29b1ec9..4fbc1e6 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -10,9 +10,14 @@
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
 #    2) Add the following line to your .bashrc/.zshrc:
 #        source ~/.git-prompt.sh
-#    3) Change your PS1 to also show the current branch:
-#         Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
-#         ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
+#    3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
+#        To customize the prompt, provide start/end arguments
+#        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
+#    3b) Alternatively change your PS1 to call __git_ps1 as
+#        command-substitution:
+#        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
+#        ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
+#        the optional argument will be used as format string
 #
 # The argument to __git_ps1 will be displayed only if you are currently
 # in a git repository.  The %s token will be the name of the current
@@ -194,11 +199,39 @@ __git_ps1_show_upstream ()
 
 
 # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
-# returns text to add to bash PS1 prompt (includes branch name)
+# when called from PS1 using command substitution
+# in this mode it prints text to add to bash PS1 prompt (includes branch name)
+#
+# __git_ps1 requires 2 arguments when called from PROMPT_COMMAND (pc)
+# in that case it _sets_ PS1. The arguments are parts of a PS1 string.
+# when both arguments are given, the first is prepended and the second appended
+# to the state string when assigned to PS1.
 __git_ps1 ()
 {
+	local pcmode=no
+	#defaults/examples:
+	local ps1pc_start='\u@\h:\w '
+	local ps1pc_end='\$ '
+	local printf_format='(%s)'
+
+	case "$#" in
+		2)	pcmode=yes
+			ps1pc_start="$1"
+			ps1pc_end="$2"
+		;;
+		0|1)	printf_format="${1:-$printf_format}"
+		;;
+		*)	return
+		;;
+	esac
+
 	local g="$(__gitdir)"
-	if [ -n "$g" ]; then
+	if [ -z "$g" ]; then
+		if [ $pcmode = yes ]; then
+			#In PC mode PS1 always needs to be set
+			PS1="$ps1pc_start$ps1pc_end"
+		fi
+	else
 		local r=""
 		local b=""
 		if [ -f "$g/rebase-merge/interactive" ]; then
@@ -284,6 +317,12 @@ __git_ps1 ()
 		fi
 
 		local f="$w$i$s$u"
-		printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
+		if [ $pcmode = yes ]; then
+			PS1="$ps1pc_start("
+			PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+			PS1="$PS1)$ps1pc_end"
+		else
+			printf -- "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p"
+		fi
 	fi
 }
-- 
1.7.9.5

[PATCH 2/2] show color hints based on state of the git tree

[Simon Oosthoek]

By setting GIT_PS1_SHOW_COLORHINTS when using __git_ps1
as PROMPT_COMMAND, you will get color hints in addition to
a different character (*+% etc.) to indicate the state of
the tree.

Signed-off-by: Simon Oosthoek <s.oosthoek@xs4all.nl>
---
 contrib/completion/git-prompt.sh |   36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4fbc1e6..a693565 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -53,6 +53,12 @@
 # find one, or @{upstream} otherwise.  Once you have set
 # GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
 # setting the bash.showUpstream config variable.
+#
+# If you would like a colored hint about the current dirty state, set
+# GIT_PS1_SHOWCOLORHINTS to a nonempty value. When tracked files are
+# modified, the branch name turns red, when all modifications are staged
+# the branch name turns yellow and when all changes are checked in, the
+# color changes to green. The colors are currently hardcoded in the function.
 
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
@@ -206,6 +212,7 @@ __git_ps1_show_upstream ()
 # in that case it _sets_ PS1. The arguments are parts of a PS1 string.
 # when both arguments are given, the first is prepended and the second appended
 # to the state string when assigned to PS1.
+# In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true
 __git_ps1 ()
 {
 	local pcmode=no
@@ -319,9 +326,36 @@ __git_ps1 ()
 		local f="$w$i$s$u"
 		if [ $pcmode = yes ]; then
 			PS1="$ps1pc_start("
-			PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+			if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
+				local c_red='\e[31m'
+				local c_green='\e[32m'
+				local c_yellow='\e[33m'
+				local c_lblue='\e[1;34m'
+				local c_purple='\e[35m'
+				local c_cyan='\e[36m'
+				local c_clear='\e[0m'
+				local branchstring="$c${b##refs/heads/}"
+				local branch_color="$c_green"
+				local flags_color="$c_cyan"
+
+				if [ "$w" = "*" ]; then
+					branch_color="$c_red"
+				elif [ -n "$i" ]; then
+					branch_color="$c_yellow"
+				fi
+
+				# Setting PS1 directly with \[ and \] around colors
+				# is necessary to prevent wrapping issues!
+				PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
+				if [ -n "$f" ]; then
+					PS1="$PS1 \[$flags_color\]$f\[$c_clear\]"
+				fi
+			else
+				PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+			fi
 			PS1="$PS1)$ps1pc_end"
 		else
+			# NO color option unless in PROMPT_COMMAND mode
 			printf -- "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p"
 		fi
 	fi
-- 
1.7.9.5

[PATCH 2/2] show color hints based on state of the git tree

[Simon Oosthoek]

By setting GIT_PS1_SHOW_COLORHINTS when using __git_ps1
as PROMPT_COMMAND, you will get color hints in addition to
a different character (*+% etc.)

Signed-off-by: Simon Oosthoek <soosthoek@nieuwland.nl>
---
 contrib/completion/git-prompt.sh |   42 +++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c50c94a..51285d7 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -53,6 +53,12 @@
 # find one, or @{upstream} otherwise.  Once you have set
 # GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
 # setting the bash.showUpstream config variable.
+#
+# If you would like a colored hint about the current dirty state, set
+# GIT_PS1_SHOWCOLORHINTS to a nonempty value. When tracked files are
+# modified, the branch name turns red, when all modifications are staged
+# the branch name turns yellow and when all changes are checked in, the
+# color changes to green. The colors are currently hardcoded in the function.
 
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
@@ -201,11 +207,12 @@ __git_ps1_show_upstream ()
 # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
 # when called from PS1 using command substitution
 # in this mode it returns text to add to bash PS1 prompt (includes branch name) or
-# __git_ps1 accepts 0 or 2 arguments when called from PROMPT_COMMAND
+# __git_ps1 accepts 0 or 2 arguments when called from PROMPT_COMMAND (pc)
 # in that case it _sets_ PS1. The arguments are parts of a PS1 string.
 # when both arguments are given, the first is prepended and the second appended
 # to the state string when assigned to PS1, otherwise default start/end strings
 # are used.
+# In pcmode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true
 __git_ps1 ()
 {
 	local pcmode=yes
@@ -320,8 +327,37 @@ __git_ps1 ()
 
 		local f="$w$i$s$u"
 		if [ $pcmode = yes ]; then
-			PS1="$ps1pc_start($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
-		elif [ $pcmode = no ]; then
+			PS1="$ps1pc_start("
+			if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
+				local c_red='\e[31m'
+				local c_green='\e[32m'
+				local c_yellow='\e[33m'
+				local c_lblue='\e[1;34m'
+				local c_purple='\e[35m'
+				local c_cyan='\e[36m'
+				local c_clear='\e[0m'
+				local branchstring="$c${b##refs/heads/}"
+				local branch_color="$c_green"
+				local flags_color="$c_cyan"
+
+				if [ "$w" = "*" ]; then
+					branch_color="$c_red"
+				elif [ -n "$i" ]; then
+					branch_color="$c_yellow"
+				fi
+
+				# Setting PS1 directly with \[ and \] around colors
+				# is necessary to prevent wrapping issues!
+				PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
+				if [ -n "$f" ]; then
+					PS1="$PS1 \[$flags_color\]$f\[$c_clear\]"
+				fi
+			else
+				PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+			fi
+			PS1="$PS1)$ps1pc_end"
+		else
+			# NO color option unless in PROMPT_COMMAND mode
 			printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
 		fi
 	fi
-- 
1.7.9.5

Re: [PATCH 2/2] show color hints based on state of the git tree

[Michael J Gruber]

Sorry for being late ($DAYJOB and such), but I just noticed this is on
next already:

Simon Oosthoek venit, vidit, dixit 05.10.2012 23:10:
> By setting GIT_PS1_SHOW_COLORHINTS when using __git_ps1
> as PROMPT_COMMAND, you will get color hints in addition to
> a different character (*+% etc.)
> 
> Signed-off-by: Simon Oosthoek <soosthoek@nieuwland.nl>
> ---
>  contrib/completion/git-prompt.sh |   42 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index c50c94a..51285d7 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -53,6 +53,12 @@
>  # find one, or @{upstream} otherwise.  Once you have set
>  # GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
>  # setting the bash.showUpstream config variable.
> +#
> +# If you would like a colored hint about the current dirty state, set
> +# GIT_PS1_SHOWCOLORHINTS to a nonempty value. When tracked files are
> +# modified, the branch name turns red, when all modifications are staged
> +# the branch name turns yellow and when all changes are checked in, the
> +# color changes to green. The colors are currently hardcoded in the function.
>  
>  # __gitdir accepts 0 or 1 arguments (i.e., location)
>  # returns location of .git repo
> @@ -201,11 +207,12 @@ __git_ps1_show_upstream ()
>  # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
>  # when called from PS1 using command substitution
>  # in this mode it returns text to add to bash PS1 prompt (includes branch name) or
> -# __git_ps1 accepts 0 or 2 arguments when called from PROMPT_COMMAND
> +# __git_ps1 accepts 0 or 2 arguments when called from PROMPT_COMMAND (pc)
>  # in that case it _sets_ PS1. The arguments are parts of a PS1 string.
>  # when both arguments are given, the first is prepended and the second appended
>  # to the state string when assigned to PS1, otherwise default start/end strings
>  # are used.
> +# In pcmode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true
>  __git_ps1 ()
>  {
>  	local pcmode=yes
> @@ -320,8 +327,37 @@ __git_ps1 ()
>  
>  		local f="$w$i$s$u"
>  		if [ $pcmode = yes ]; then
> -			PS1="$ps1pc_start($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
> -		elif [ $pcmode = no ]; then
> +			PS1="$ps1pc_start("
> +			if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then

You're missing the "S" here (HINTS).

> +				local c_red='\e[31m'
> +				local c_green='\e[32m'
> +				local c_yellow='\e[33m'
> +				local c_lblue='\e[1;34m'
> +				local c_purple='\e[35m'
> +				local c_cyan='\e[36m'
> +				local c_clear='\e[0m'
> +				local branchstring="$c${b##refs/heads/}"
> +				local branch_color="$c_green"
> +				local flags_color="$c_cyan"
> +
> +				if [ "$w" = "*" ]; then
> +					branch_color="$c_red"
> +				elif [ -n "$i" ]; then
> +					branch_color="$c_yellow"
> +				fi
> +
> +				# Setting PS1 directly with \[ and \] around colors
> +				# is necessary to prevent wrapping issues!
> +				PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
> +				if [ -n "$f" ]; then
> +					PS1="$PS1 \[$flags_color\]$f\[$c_clear\]"
> +				fi
> +			else
> +				PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
> +			fi
> +			PS1="$PS1)$ps1pc_end"
> +		else
> +			# NO color option unless in PROMPT_COMMAND mode
>  			printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
>  		fi
>  	fi
> 

I'm afraid I don't like this coloring style at all because it is
inconsistent with the color usage of "git status -sb". First of all, the
colors are different, and second, the fact *what* is colored is
different. I had suggested following "git status -sb" for a good reason.
It colors a branch green and a detached head red. It colors "change"
(A/D/M/R) as red/green depending on non-cached/cached, so that's how */+
should be. Your call for $/% (I'd leave them uncolored).[*][**]

I think it's very confusing to have completely different schemes (not
just themes) for two versions of the same information: concise status
information.

So, please try and follow "git status -sb".

Michael

[*] Really, there's nothin "red" about a branch when there are cached or
non-cached changes. They are changes wrt. to what's in HEAD resp. the
index, no matter what HEAD is.

[**] Also, coloring the status characters opens the way to even using
the same characters as "status -sb" (ADM).

Re: [PATCH 2/2] show color hints based on state of the git tree

[Simon Oosthoek]

On 10/15/2012 10:23 AM, Michael J Gruber wrote:
> Sorry for being late ($DAYJOB and such), but I just noticed this is on
> next already:
>
>> +			if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
>
> You're missing the "S" here (HINTS).

indeed, well spotted!
My test setup was apparently configured without the S and the commented 
uses all had HINTS...


>
>> +				local c_red='\e[31m'
>> +				local c_green='\e[32m'
>> +				local c_yellow='\e[33m'
>> +				local c_lblue='\e[1;34m'
>> +				local c_purple='\e[35m'
>> +				local c_cyan='\e[36m'
>> +				local c_clear='\e[0m'
>> +				local branchstring="$c${b##refs/heads/}"
>> +				local branch_color="$c_green"
>> +				local flags_color="$c_cyan"
>> +
>> +				if [ "$w" = "*" ]; then
>> +					branch_color="$c_red"
>> +				elif [ -n "$i" ]; then
>> +					branch_color="$c_yellow"
>> +				fi
>> +
>> +				# Setting PS1 directly with \[ and \] around colors
>> +				# is necessary to prevent wrapping issues!
>> +				PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
>> +				if [ -n "$f" ]; then
>> +					PS1="$PS1 \[$flags_color\]$f\[$c_clear\]"
>> +				fi
>> +			else
>> +				PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
>> +			fi
>> +			PS1="$PS1)$ps1pc_end"
>> +		else
>> +			# NO color option unless in PROMPT_COMMAND mode
>>   			printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
>>   		fi
>>   	fi
>>
>
> I'm afraid I don't like this coloring style at all because it is
> inconsistent with the color usage of "git status -sb". First of all, the
> colors are different, and second, the fact *what* is colored is
> different. I had suggested following "git status -sb" for a good reason.
> It colors a branch green and a detached head red. It colors "change"
> (A/D/M/R) as red/green depending on non-cached/cached, so that's how */+
> should be. Your call for $/% (I'd leave them uncolored).[*][**]

This way works for me. The coloring scheme itself is probably quite 
personal and can, and probably should, be modified by end-users.
(Whether the current code is suitable for user-modification is another 
matter)

I'm quite unfamiliar with the color coding of git (I hadn't enabled that 
option), I suppose consistency would be better, but then you'd have to 
add some code to figure out which color is used for what in git's output 
and convert that to the code setting the colors here.

As for the characters used, I think there's a good reason not to use the 
ones git uses in the prompt. The characters in git status output are put 
in front of the files they apply to, in the prompt you only get a 
summarized output. And perhaps that argument could be extended to the 
use of the colors as well, I prefer to know whether I have uncommitted 
changes and in that category I want to know whether I already staged 
them or not.

If I want to know which files are unstaged/uncommitted I do git status 
to see that.

>
> I think it's very confusing to have completely different schemes (not
> just themes) for two versions of the same information: concise status
> information.
>
> So, please try and follow "git status -sb".

I think there are different levels of conciseness. And I see "git status 
-sb" uses green for staged modified files, which would be confusing to me.

>
> Michael
>
> [*] Really, there's nothin "red" about a branch when there are cached or
> non-cached changes. They are changes wrt. to what's in HEAD resp. the
> index, no matter what HEAD is.

Do you mean that red (unstaged) and yellow (staged) are both "red" to you?

>
> [**] Also, coloring the status characters opens the way to even using
> the same characters as "status -sb" (ADM).
>

Perhaps, but that would be confusing to me ;-)

Cheers

Simon

Re: [PATCH 2/2] show color hints based on state of the git tree

[Michael J Gruber]

Simon Oosthoek venit, vidit, dixit 15.10.2012 11:01:
> On 10/15/2012 10:23 AM, Michael J Gruber wrote:
>> Sorry for being late ($DAYJOB and such), but I just noticed this is on
>> next already:
>>
>>> +			if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
>>
>> You're missing the "S" here (HINTS).
> 
> indeed, well spotted!
> My test setup was apparently configured without the S and the commented 
> uses all had HINTS...
> 
> 
>>
>>> +				local c_red='\e[31m'
>>> +				local c_green='\e[32m'
>>> +				local c_yellow='\e[33m'
>>> +				local c_lblue='\e[1;34m'
>>> +				local c_purple='\e[35m'
>>> +				local c_cyan='\e[36m'
>>> +				local c_clear='\e[0m'
>>> +				local branchstring="$c${b##refs/heads/}"
>>> +				local branch_color="$c_green"
>>> +				local flags_color="$c_cyan"
>>> +
>>> +				if [ "$w" = "*" ]; then
>>> +					branch_color="$c_red"
>>> +				elif [ -n "$i" ]; then
>>> +					branch_color="$c_yellow"
>>> +				fi
>>> +
>>> +				# Setting PS1 directly with \[ and \] around colors
>>> +				# is necessary to prevent wrapping issues!
>>> +				PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
>>> +				if [ -n "$f" ]; then
>>> +					PS1="$PS1 \[$flags_color\]$f\[$c_clear\]"
>>> +				fi
>>> +			else
>>> +				PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
>>> +			fi
>>> +			PS1="$PS1)$ps1pc_end"
>>> +		else
>>> +			# NO color option unless in PROMPT_COMMAND mode
>>>   			printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
>>>   		fi
>>>   	fi
>>>
>>
>> I'm afraid I don't like this coloring style at all because it is
>> inconsistent with the color usage of "git status -sb". First of all, the
>> colors are different, and second, the fact *what* is colored is
>> different. I had suggested following "git status -sb" for a good reason.
>> It colors a branch green and a detached head red. It colors "change"
>> (A/D/M/R) as red/green depending on non-cached/cached, so that's how */+
>> should be. Your call for $/% (I'd leave them uncolored).[*][**]
> 
> This way works for me. The coloring scheme itself is probably quite 
> personal and can, and probably should, be modified by end-users.
> (Whether the current code is suitable for user-modification is another 
> matter)

It really doesn't matter much what works for you, and it doesn't matter
what works for me either. The point is: What works for most users?

> I'm quite unfamiliar with the color coding of git (I hadn't enabled that 
> option), I suppose consistency would be better, but then you'd have to 
> add some code to figure out which color is used for what in git's output 
> and convert that to the code setting the colors here.

As a starter, you could use the default colors which git uses. Querying
git for the colors could be the next, optional step.

> As for the characters used, I think there's a good reason not to use the 
> ones git uses in the prompt. The characters in git status output are put 
> in front of the files they apply to, in the prompt you only get a 
> summarized output. And perhaps that argument could be extended to the 
> use of the colors as well, I prefer to know whether I have uncommitted 
> changes and in that category I want to know whether I already staged 
> them or not.

Well, sure. The prompt could show any of AMCRD if you have any of those
changes. * + are shortcuts saying you have any of those.

> If I want to know which files are unstaged/uncommitted I do git status 
> to see that.

Sure. I don't propose putting file names in the prompt ;)

>>
>> I think it's very confusing to have completely different schemes (not
>> just themes) for two versions of the same information: concise status
>> information.
>>
>> So, please try and follow "git status -sb".
> 
> I think there are different levels of conciseness. And I see "git status 
> -sb" uses green for staged modified files, which would be confusing to me.

...only because you don't know the color coding scheme. It's green
because those changes are saved somewhere (in the index) and would even
survice a branch switch.

>> Michael
>>
>> [*] Really, there's nothin "red" about a branch when there are cached or
>> non-cached changes. They are changes wrt. to what's in HEAD resp. the
>> index, no matter what HEAD is.
> 
> Do you mean that red (unstaged) and yellow (staged) are both "red" to you?

No, I said red for unstaged, green for staged for the characters, i.e. a
* would always be red if present, a + always green if present.

For the "branch name", it would be red for detached head and green for
checked out branch.

>> [**] Also, coloring the status characters opens the way to even using
>> the same characters as "status -sb" (ADM).
>>
> 
> Perhaps, but that would be confusing to me ;-)

If that is case, you can try and propose changing that scheme...

But there's really a good reason for it. Cached (staged) changes are
"safe" because they are saved in the index, and also they are "good to
go into the next commit", i.e. the commit traffic lights are green!

Michael

Re: [PATCH 2/2] show color hints based on state of the git tree

[Simon Oosthoek]

On 10/15/2012 11:13 AM, Michael J Gruber wrote:
>
> It really doesn't matter much what works for you, and it doesn't matter
> what works for me either. The point is: What works for most users?

Obviously, that was my point as well ;-)

>> I'm quite unfamiliar with the color coding of git (I hadn't enabled that
>> option), I suppose consistency would be better, but then you'd have to
>> add some code to figure out which color is used for what in git's output
>> and convert that to the code setting the colors here.
>
> As a starter, you could use the default colors which git uses. Querying
> git for the colors could be the next, optional step.

I suppose it would be a start.

so how should it be?

all committed: green branch, no characters
only staged changes: green branch, colored characters (yellow/red?)
only unstaged changes: red/yellow branch, colored characters (red)
staged and unstaged changes: red/yellow branch, multiple characters in 
red/yellow?)
detached head: red commit-sha1 (abbrev.), characters as above?

What is the most useful thing to show in case of a detached head?
-the commit hash of current head?
-"detached head"


>
>> As for the characters used, I think there's a good reason not to use the
>> ones git uses in the prompt. The characters in git status output are put
>> in front of the files they apply to, in the prompt you only get a
>> summarized output. And perhaps that argument could be extended to the
>> use of the colors as well, I prefer to know whether I have uncommitted
>> changes and in that category I want to know whether I already staged
>> them or not.
>
> Well, sure. The prompt could show any of AMCRD if you have any of those
> changes. * + are shortcuts saying you have any of those.

How do you suppose that should work?
Add another configuration parameter like GIT_PS1_SHOWSTATUSSBCHARS 
(please give a better suggestion ;-) or the other way around 
GIT_PS1_SHOWSTARPLUS (either of which could be the default)

>>> I think it's very confusing to have completely different schemes (not
>>> just themes) for two versions of the same information: concise status
>>> information.
>>>
>>> So, please try and follow "git status -sb".
>>
>> I think there are different levels of conciseness. And I see "git status
>> -sb" uses green for staged modified files, which would be confusing to me.
>
> ...only because you don't know the color coding scheme. It's green
> because those changes are saved somewhere (in the index) and would even
> survice a branch switch.

I suppose you could see that as green (I usually view a yellow traffic 
light as green as well ;-)

Isn't it best practice to commit the staged changes ASAP? (But let's not 
diverge in this thread to discuss best-practices, so consider that a 
rhetorical question for now ;-)

> No, I said red for unstaged, green for staged for the characters, i.e. a
> * would always be red if present, a + always green if present.
>
> For the "branch name", it would be red for detached head and green for
> checked out branch.

using green for staged changes could be acceptable for me, but I'm 
slightly worried that a colored character would be too slight a hint.

OTOH, a detached head would show up with or without color...

>
>> Perhaps, but that would be confusing to me ;-)
>
> If that is case, you can try and propose changing that scheme...
>
> But there's really a good reason for it. Cached (staged) changes are
> "safe" because they are saved in the index, and also they are "good to
> go into the next commit", i.e. the commit traffic lights are green!

I suppose I knew that in the back of my mind, but not as clearly as you 
spell it out here... Tnx!

/Simon

Re: [PATCH 2/2] show color hints based on state of the git tree

[Simon Oosthoek]

Hi Michael, sorry for the duplicate, forgot to reply-all...

On 10/15/2012 11:13 AM, Michael J Gruber wrote:

> ...only because you don't know the color coding scheme. It's green
> because those changes are saved somewhere (in the index) and would even
> survice a branch switch.
>

But git doesn't exactly let you do this:
I modified some things in git-prompt.sh trying to implement some of what 
we discussed. Then staged the file and tried git checkout HEAD^^ (or any 
branch)

error: Your local changes to the following files would be overwritten by 
checkout:
         contrib/completion/git-prompt.sh
Please, commit your changes or stash them before you can switch branches.
Aborting

So I don't think it's all that strange to mark the branch as not quite 
safe to change. The idea (or at least my idea) behind these hints is 
that it reminds me to do stuff that prevents these "Aborts". I think 
that that is a useful feature for any user of git.

In this light, would you accept yellow in the branch color to indicate 
uncommitted staged changes?

Cheers

Simon

Re: [PATCH 2/2] show color hints based on state of the git tree

[Michael J Gruber]

Simon Oosthoek venit, vidit, dixit 15.10.2012 15:20:
> Hi Michael, sorry for the duplicate, forgot to reply-all...
> 
> On 10/15/2012 11:13 AM, Michael J Gruber wrote:
> 
>> ...only because you don't know the color coding scheme. It's green
>> because those changes are saved somewhere (in the index) and would even
>> survice a branch switch.
>>
> 
> But git doesn't exactly let you do this:
> I modified some things in git-prompt.sh trying to implement some of what 
> we discussed. Then staged the file and tried git checkout HEAD^^ (or any 
> branch)
> 
> error: Your local changes to the following files would be overwritten by 
> checkout:
>          contrib/completion/git-prompt.sh
> Please, commit your changes or stash them before you can switch branches.
> Aborting
> 
> So I don't think it's all that strange to mark the branch as not quite 
> safe to change. The idea (or at least my idea) behind these hints is 
> that it reminds me to do stuff that prevents these "Aborts". I think 
> that that is a useful feature for any user of git.
> 
> In this light, would you accept yellow in the branch color to indicate 
> uncommitted staged changes?

No, simply because it's not up to me to accept something ;)

I still think that following "git status -sb" would be the least
confusing solution, and that means

- green for a checked out branch, where you know that any committed
changes will not be "lost"
- red for a detached head, where any committed changes will only be
accessible from the reflog, and only until pruned.

I'm open to change "git status -sb", but I think having the branch
coloring and the status character coloring be independent has quite some
merit. (Your yellow branch just duplicates the information "there are
staged changes".)

Also note that in your example above, you cannot switch branches because
your staged changes overlap with the changes in the branch that you want
to switch to (changes relative to your current head). Otherwise, you
could have switched. The only way to find out whether there's such a
conflict is to actually try it, and that is too heavy for a prompt command.

Michael

Re: [PATCH 2/2] show color hints based on state of the git tree

[Simon Oosthoek]

On 10/15/2012 11:39 AM, Junio C Hamano wrote:
> Thanks for bringing some sanity to the color of the bikeshed ;-)
>
> As I don't use colors very much, I didn't bother checking the color
> assignment in the patch in question, but everything you said in your
> response makes 100% sense, including the traffic light analogy.
>

Hi Junio, Michael

The point of the thread and the patch was to enable the possibility of 
colors in the prompt without messing it up.

The actual colors used are more or less how I'm used to it, but as you 
said they may not be suitable to everyone.

@Junio, is this patch something you want to include as it is now (with 
the extra S that Michael pointed out) or do you want to wait for a 
discussion about which colors to use for which state?

I guess it could be quite a messy discussion, as you already hint at 
bikeshed colors, it's quite personal and subjective.

As a start, I think it's worth considering what the color would actually 
mean.

To me:
A neutral color should mean it's safe to switch branches or merge/update
A non-neutral color would mean some action is required before switching 
branches.
There are various states that may or may not get in the way of 
switching, like stashed stuff or divergence from upstream. I think these 
could get a color hint, but I'm not sure what kind.

Cheers

Simon

Re: [PATCH 2/2] show color hints based on state of the git tree

[Junio C Hamano]

Simon Oosthoek <soosthoek@nieuwland.nl> writes:

> The point of the thread and the patch was to enable the possibility of
> colors in the prompt without messing it up.
>
> The actual colors used are more or less how I'm used to it, but as you
> said they may not be suitable to everyone.
>
> @Junio, is this patch something you want to include as it is now (with
> the extra S that Michael pointed out) or do you want to wait for a
> discussion about which colors to use for which state?

The latter.

> I guess it could be quite a messy discussion, as you already hint at
> bikeshed colors, it's quite personal and subjective.

The choice of colours may be subjective, but you can just omit that
part, if there is already an existing choice made by others in
earlier changes like in "status" output.  As long as the subjective
choices in two systems that show similar information are consistent,
exact choice of colours does not matter that much.

[PATCH 3/3] Change colors to be based on git status -sb in color mode

[Simon Oosthoek]

Hi

this patch is an additional patch to the previous series of two.  It also
corrects the missing S and some minor details. The main point of this
one is changing the used colors to be more close to the color output of "git
status -sb" Mainly, the branchname stays green until it loses a HEAD, in
detached head state it becomes red.
The flags get their own color, either red or green for unstaged/staged and the
remaining flags get a different color or none at all.

Cheers

Simon

The color in the prompt now follows the colors used by
git itself when showing colors. The branch name is only
colored red if the tree has a detached HEAD.

Signed-off-by: Simon Oosthoek <s.oosthoek@xs4all.nl>
---
 contrib/completion/git-prompt.sh |   52 ++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4fb998a..ff69bbc 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -55,11 +55,9 @@
 # setting the bash.showUpstream config variable.
 #
 # If you would like a colored hint about the current dirty state, set
-# GIT_PS1_SHOWCOLORHINTS to a nonempty value. When tracked files are
-# modified, the branch name turns red, when all modifications are staged
-# the branch name turns yellow and when all changes are checked in, the
-# color changes to green. The colors are currently hardcoded in the function.
-
+# GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on 
+# the colored output of "git status -sb".
+#
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
 __gitdir ()
@@ -325,35 +323,45 @@ __git_ps1 ()
 
 		local f="$w$i$s$u"
 		if [ $pcmode = yes ]; then
-			PS1="$ps1pc_start("
-			if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
+			if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
 				local c_red='\e[31m'
 				local c_green='\e[32m'
-				local c_yellow='\e[33m'
 				local c_lblue='\e[1;34m'
-				local c_purple='\e[35m'
-				local c_cyan='\e[36m'
 				local c_clear='\e[0m'
 				local branchstring="$c${b##refs/heads/}"
-				local branch_color="$c_green"
-				local flags_color="$c_cyan"
+				local branch_color="$c_clear"
+				local flags_color="$c_lblue"
 
-				if [ "$w" = "*" ]; then
-					branch_color="$c_red"
-				elif [ -n "$i" ]; then
-					branch_color="$c_yellow"
-				fi
+				case "$b" in 
+					\(*\))	branch_color="$c_red"
+					;;
+					*)	local branch_color="$c_green"
+					;;
+				esac
 
 				# Setting PS1 directly with \[ and \] around colors
 				# is necessary to prevent wrapping issues!
-				PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
-				if [ -n "$f" ]; then
-					PS1="$PS1 \[$flags_color\]$f\[$c_clear\]"
+				PS1="$ps1pc_start (\[$branch_color\]$branchstring\[$c_clear\]"
+
+				if [ -n "$w$i$s$u$r$p" ]; then
+					PS1="$PS1 "
+				fi
+				if [ "$w" = "*" ]; then
+					PS1="$PS1\[$c_red\]$w"
+				fi
+				if [ -n "$i" ]; then
+					PS1="$PS1\[$c_green\]$i"
+				fi
+				if [ -n "$s" ]; then
+					PS1="$PS1\[$flags_color\]$s"
+				fi
+				if [ -n "$u" ]; then
+					PS1="$PS1\[$c_red\]$u"
 				fi
+				PS1="$PS1\[$c_clear\]$r$p)$ps1pc_end"
 			else
-				PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+				PS1="$ps1pc_start ($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
 			fi
-			PS1="$PS1)$ps1pc_end"
 		else
 			# NO color option unless in PROMPT_COMMAND mode
 			printf -- "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p"
-- 
1.7.9.5

Re: [PATCH 3/3] Change colors to be based on git status -sb in color mode

[Junio C Hamano]

Simon Oosthoek <s.oosthoek@xs4all.nl> writes:

> this patch is an additional patch to the previous series of two.


> @@ -325,35 +323,45 @@ __git_ps1 ()
>  
>  		local f="$w$i$s$u"
>  		if [ $pcmode = yes ]; then
> +			if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
>  				local c_red='\e[31m'
>  				local c_green='\e[32m'
>  				local c_lblue='\e[1;34m'
>  				local c_clear='\e[0m'
>  				local branchstring="$c${b##refs/heads/}"
> +				local branch_color="$c_clear"
> +				local flags_color="$c_lblue"
>  
> +				case "$b" in 
> +					\(*\))	branch_color="$c_red"
> +					;;
> +					*)	local branch_color="$c_green"
> +					;;
> +				esac

Why "local" only on one side and not the other?

Also switching on "$b" and relying on how it happens to be formatted
inside parentheses when detached (which can very well change in the
future) looks like a fragile bug waiting to be triggered.

>  				# Setting PS1 directly with \[ and \] around colors
>  				# is necessary to prevent wrapping issues!
> +				PS1="$ps1pc_start (\[$branch_color\]$branchstring\[$c_clear\]"
> +
> +				if [ -n "$w$i$s$u$r$p" ]; then
> +					PS1="$PS1 "
> +				fi
> +				if [ "$w" = "*" ]; then
> +					PS1="$PS1\[$c_red\]$w"
> +				fi
> +				if [ -n "$i" ]; then
> +					PS1="$PS1\[$c_green\]$i"
> +				fi

You gave nice symbolic names to branch_color and flags_color, but
these two use hardcoded colors?

[PATCH 3/3] Change colors to be based on git status -sb in color mode

[Simon Oosthoek]

Hi

Fixed the mistakes of the last one, and I'm now using the symbolic names "ok_color" and "bad_color".
The test for headless state is now more direct, and I hope it is still correct.

/Simon


this patch is an additional patch to the previous series of two.  It also
corrects the missing S and some minor details. The main point of this
one is changing the used colors to be more close to the color output of "git
status -sb" Mainly, the branchname stays green until it loses a HEAD, in
detached head state it becomes red.
The flags get their own color, either red or green for unstaged/staged and the
remaining flags get a different color or none at all.

Signed-off-by: Simon Oosthoek <s.oosthoek@xs4all.nl>
---
 contrib/completion/git-prompt.sh |   52 +++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4fb998a..7dce319 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -55,11 +55,9 @@
 # setting the bash.showUpstream config variable.
 #
 # If you would like a colored hint about the current dirty state, set
-# GIT_PS1_SHOWCOLORHINTS to a nonempty value. When tracked files are
-# modified, the branch name turns red, when all modifications are staged
-# the branch name turns yellow and when all changes are checked in, the
-# color changes to green. The colors are currently hardcoded in the function.
-
+# GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on 
+# the colored output of "git status -sb".
+#
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
 __gitdir ()
@@ -325,35 +323,47 @@ __git_ps1 ()
 
 		local f="$w$i$s$u"
 		if [ $pcmode = yes ]; then
-			PS1="$ps1pc_start("
-			if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
+			if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
 				local c_red='\e[31m'
 				local c_green='\e[32m'
-				local c_yellow='\e[33m'
 				local c_lblue='\e[1;34m'
-				local c_purple='\e[35m'
-				local c_cyan='\e[36m'
 				local c_clear='\e[0m'
+				local bad_color=$c_red
+				local ok_color=$c_green
+				local branch_color="$c_clear"
+				local flags_color="$c_lblue"
 				local branchstring="$c${b##refs/heads/}"
-				local branch_color="$c_green"
-				local flags_color="$c_cyan"
 
-				if [ "$w" = "*" ]; then
-					branch_color="$c_red"
-				elif [ -n "$i" ]; then
-					branch_color="$c_yellow"
+				if git symbolic-ref HEAD 2>/dev/null 1>&2
+				then
+					branch_color="$ok_color"
+				else
+					branch_color="$bad_color"
 				fi
 
 				# Setting PS1 directly with \[ and \] around colors
 				# is necessary to prevent wrapping issues!
-				PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
-				if [ -n "$f" ]; then
-					PS1="$PS1 \[$flags_color\]$f\[$c_clear\]"
+				PS1="$ps1pc_start (\[$branch_color\]$branchstring\[$c_clear\]"
+
+				if [ -n "$w$i$s$u$r$p" ]; then
+					PS1="$PS1 "
+				fi
+				if [ "$w" = "*" ]; then
+					PS1="$PS1\[$bad_color\]$w"
+				fi
+				if [ -n "$i" ]; then
+					PS1="$PS1\[$ok_color\]$i"
+				fi
+				if [ -n "$s" ]; then
+					PS1="$PS1\[$flags_color\]$s"
+				fi
+				if [ -n "$u" ]; then
+					PS1="$PS1\[$bad_color\]$u"
 				fi
+				PS1="$PS1\[$c_clear\]$r$p)$ps1pc_end"
 			else
-				PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+				PS1="$ps1pc_start ($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
 			fi
-			PS1="$PS1)$ps1pc_end"
 		else
 			# NO color option unless in PROMPT_COMMAND mode
 			printf -- "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p"
-- 
1.7.9.5

Re: [PATCH 3/3] Change colors to be based on git status -sb in color mode

[Junio C Hamano]

Simon Oosthoek <s.oosthoek@xs4all.nl> writes:

> Hi
>
> Fixed the mistakes of the last one, and I'm now using the symbolic names "ok_color" and "bad_color".
> The test for headless state is now more direct, and I hope it is still correct.
>
> /Simon

Will apply with a bit more readable log message.

I think it would be a good idea to squash something like the
attached into this patch, though.

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

diff --git i/contrib/completion/git-prompt.sh w/contrib/completion/git-prompt.sh
index dced628..9f0b6bb 100644
--- i/contrib/completion/git-prompt.sh
+++ w/contrib/completion/git-prompt.sh
@@ -215,7 +215,7 @@ __git_ps1_show_upstream ()
 __git_ps1 ()
 {
 	local pcmode=no
-	#defaults/examples:
+	local detached=no
 	local ps1pc_start='\u@\h:\w '
 	local ps1pc_end='\$ '
 	local printf_format=' (%s)'
@@ -264,7 +264,7 @@ __git_ps1 ()
 			fi
 
 			b="$(git symbolic-ref HEAD 2>/dev/null)" || {
-
+				detached=yes
 				b="$(
 				case "${GIT_PS1_DESCRIBE_STYLE-}" in
 				(contains)
@@ -335,8 +335,7 @@ __git_ps1 ()
 				local flags_color="$c_lblue"
 				local branchstring="$c${b##refs/heads/}"
 
-				if git symbolic-ref HEAD 2>/dev/null 1>&2
-				then
+				if [ $detached = yes ]; then
 					branch_color="$ok_color"
 				else
 					branch_color="$bad_color"

Re: [PATCH 3/3] Change colors to be based on git status -sb in color mode

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
>
>> Hi
>>
>> Fixed the mistakes of the last one, and I'm now using the symbolic names "ok_color" and "bad_color".
>> The test for headless state is now more direct, and I hope it is still correct.
>>
>> /Simon
>
> Will apply with a bit more readable log message.
>
> I think it would be a good idea to squash something like the
> attached into this patch, though.

The reason for this suggestion is not to avoid running symbolic-ref
twice (which is a good side effect, though).  When you are in the
middle of bisection or multi-commit cherry-pick, for example, you
would conceptually be working on the original branch, but the HEAD
pointer may be detached.  Earlier parts of the existing code in the
function makes this distinction, and the same distinction should be
carried over to the new code that decides the color to paint things
in.

>  contrib/completion/git-prompt.sh | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git i/contrib/completion/git-prompt.sh w/contrib/completion/git-prompt.sh
> index dced628..9f0b6bb 100644
> --- i/contrib/completion/git-prompt.sh
> +++ w/contrib/completion/git-prompt.sh
> @@ -215,7 +215,7 @@ __git_ps1_show_upstream ()
>  __git_ps1 ()
>  {
>  	local pcmode=no
> -	#defaults/examples:
> +	local detached=no
>  	local ps1pc_start='\u@\h:\w '
>  	local ps1pc_end='\$ '
>  	local printf_format=' (%s)'
> @@ -264,7 +264,7 @@ __git_ps1 ()
>  			fi
>  
>  			b="$(git symbolic-ref HEAD 2>/dev/null)" || {
> -
> +				detached=yes
>  				b="$(
>  				case "${GIT_PS1_DESCRIBE_STYLE-}" in
>  				(contains)
> @@ -335,8 +335,7 @@ __git_ps1 ()
>  				local flags_color="$c_lblue"
>  				local branchstring="$c${b##refs/heads/}"
>  
> -				if git symbolic-ref HEAD 2>/dev/null 1>&2
> -				then
> +				if [ $detached = yes ]; then
>  					branch_color="$ok_color"
>  				else
>  					branch_color="$bad_color"

Re: [PATCH 3/3] Change colors to be based on git status -sb in color mode

[Simon Oosthoek]

On 16/10/12 23:30, Junio C Hamano wrote:
> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
> 
>> Hi
>>
>> Fixed the mistakes of the last one, and I'm now using the symbolic names "ok_color" and "bad_color".
>> The test for headless state is now more direct, and I hope it is still correct.
>>
>> /Simon
> 
> Will apply with a bit more readable log message.

Ok

> 
> I think it would be a good idea to squash something like the
> attached into this patch, though.

I tried the patch and it works when you switch the color assignments in
the if statement you added...

> @@ -264,7 +264,7 @@ __git_ps1 ()
>  			fi
>  
>  			b="$(git symbolic-ref HEAD 2>/dev/null)" || {
> -
> +				detached=yes
>  				b="$(
>  				case "${GIT_PS1_DESCRIBE_STYLE-}" in
>  				(contains)
> @@ -335,8 +335,7 @@ __git_ps1 ()
>  				local flags_color="$c_lblue"
>  				local branchstring="$c${b##refs/heads/}"
>  
> -				if git symbolic-ref HEAD 2>/dev/null 1>&2
> -				then
> +				if [ $detached = yes ]; then
-  					branch_color="$ok_color"
+  					branch_color="$bad_color"
>  				else
-  					branch_color="$bad_color"
+  					branch_color="$ok_color"

if detached = yes, then bad_color should be applied, else use ok_color.

Cheers

Simon

Re: [PATCH] Add __git_ps1_pc to use as PROMPT_COMMAND

[Simon Oosthoek]

Hi Junio

I hope you found my patches, if not, I'll try to send them out again. 
(Unfortunately my current mail setup is a mess and I need time to figure 
out what to fix...)

As for the zsh support, I found out a little bit more.
ZSH doesn't have a variable like PROMPT_COMMAND in bash. The precmd name 
is a function the user has to define herself to have it called before 
the prompt is shown. Functionally this is almost, but not quite, the 
same as PROMPT_COMMAND. The subtle difference is that with 
PROMPT_COMMAND you can use parameters to customize the prompt, so in 
bash you can say:

PROMPT_COMMAND="__git_ps1 '\u@\[\e[1;34m\]\h\[\e[0m\]:\w' '\\\$ '"

where in zsh, if you want the status in the prompt, you can either use 
$(__git_ps1 "(%s)") or something like it in setting the PS1 (and forget 
about the color hints!) or you can copy the __git_ps1 function and 
modify and rename it as precmd to set PS1 via that function. Obviously 
it is probably even more complicated, but I'd have to try it to be certain.

An alternative way might be to set special variables from __git_set_vars 
function which may be used in a custom precmd to set the prompt.

e.g. say __git_set_vars sets __GIT_VAR_STATE=dirty|stage|clean

in precmd you could do
case $__GIT_VAR_STATE in
(dirty) PS1="$PS1 files modified in __GIT_VAR_BRANCHNAME"
;;
(stage) PS1="$PS1 files staged in __GIT_VAR_BRANCHNAME"
;;
(clean) PS1="$PS1 __GIT_VAR_BRANCHNAME clean"
;;
esac

(more or less of course)..

In that way it would be possible to add the colors relatively easily 
based on the state of the tree in a custom precmd function of zsh.

/Simon