bash completion with colour hints

> --- 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...

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

> +		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
>  }

> 
>> --- 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.
> 

>> +		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 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

> 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.
> 

>> 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.

>
> 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:

>
> - 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?

>
>  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?

> +# __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
> +}

> 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.
>

> 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.

> 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.
> 
> 

>> 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?

> 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????
> 

> 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).

> 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 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?

>
> 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.

> 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...
> 

> 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

>  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.

>  __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'

> +			;;
> +			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

> 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 ;-)

>>  
>>  # __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?

>> +		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?

> 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?

> 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)

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

> @@ -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
>  }

> 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
> 

> 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).

>
>> +				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.

> 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)

> 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.

>>
>> 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 ;-)

>> 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.

>>> 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.

> 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.

>
>> 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!

> 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?

> 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.
>

> 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?

> @@ -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

>  				# 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

> 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

> 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"

> 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.

> @@ -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