Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Simon Oosthoek]

* Piotr Krukowiecki <piotr.krukowiecki@gmail.com> [2012-11-28 11:03:29 +0100]:

> Hi,
> 
> when I set PROMPT_COMMAND to __git_ps1 I get a space at the beginning:
> 

Is your setting?:
PROMPT_COMMAND=__git_ps1

I believe you need to give 2 parameters in order to use it in PROMPT_COMMAND mode.

In my .bashrc I have:
if [ -f ~/.gitprompt.sh ]
then
        . ~/.gitprompt.sh
        GIT_PS1_SHOWDIRTYSTATE=true
        GIT_PS1_SHOWCOLORHINTS=true
        GIT_PS1_SHOWUNTRACKEDFILES=true
        PROMPT_COMMAND="__git_ps1 '\u@\[\e[1;34m\]\h\[\e[0m\]:\w' '\\\$ '"
fi


>  (master)pkruk@foobar ~/dir$
> ^ space
> 
> Is there a reason for this? It looks like a waste of space. If I'm not in
> git repository I don't have the space:
> 
> pkruk@foobar ~/other$
> 
> I noticed the space is explicitly specified in printf_format in
> git-prompt.sh. Is it needed? If I remove it, everything seems to work fine
> (no leading space)...
> 
> --- /usr/local/src/git/git/contrib/completion/git-prompt.sh 2012-11-28
> 10:27:05.728939201 +0100
> +++ /home/pkruk/.git-prompt.sh 2012-11-28 10:52:56.852629745 +0100
> @@ -218,7 +218,7 @@ __git_ps1 ()
>   local detached=no
>   local ps1pc_start='\u@\h:\w '
>   local ps1pc_end='\$ '
> - local printf_format=' (%s)'
> + local printf_format='(%s)'
> 
>   case "$#" in
>   2) pcmode=yes


These last 2 lines say: if 2 arguments are given, use pcmode. Otherwise you get command-subtitution mode, which gives weird effects when being called from PROMPT_COMMAND.

Cheers

Simon

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Piotr Krukowiecki]

Re-sending mail.

On Wed, Nov 28, 2012 at 7:02 PM, Piotr Krukowiecki
<piotr.krukowiecki@gmail.com> wrote:
>
> On Wed, Nov 28, 2012 at 2:20 PM, Simon Oosthoek <s.oosthoek@xs4all.nl> wrote:
>>
>> * Piotr Krukowiecki <piotr.krukowiecki@gmail.com> [2012-11-28 11:03:29 +0100]:
>>
>> > Hi,
>> >
>> > when I set PROMPT_COMMAND to __git_ps1 I get a space at the beginning:
>> >
>>
>> Is your setting?:
>> PROMPT_COMMAND=__git_ps1
>
>
> That's right
>
>
>> I believe you need to give 2 parameters in order to use it in PROMPT_COMMAND mode.
>
>
> Are you sure? git-prompt.sh says:
>
> #    3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
> #        To customize the prompt, provide start/end arguments
> #        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>
> I interpret this as: if you don't want to customize the prompt, the simple "PROMPT_COMMAND=__git_ps1" is enough.
>
>>
>> >  (master)pkruk@foobar ~/dir$
>> > ^ space
>> >
>> > Is there a reason for this? It looks like a waste of space. If I'm not in
>> > git repository I don't have the space:
>> >
>> > pkruk@foobar ~/other$
>> >
>> > I noticed the space is explicitly specified in printf_format in
>> > git-prompt.sh. Is it needed? If I remove it, everything seems to work fine
>> > (no leading space)...
>> >
>> > --- /usr/local/src/git/git/contrib/completion/git-prompt.sh 2012-11-28
>> > 10:27:05.728939201 +0100
>> > +++ /home/pkruk/.git-prompt.sh 2012-11-28 10:52:56.852629745 +0100
>> > @@ -218,7 +218,7 @@ __git_ps1 ()
>> >   local detached=no
>> >   local ps1pc_start='\u@\h:\w '
>> >   local ps1pc_end='\$ '
>> > - local printf_format=' (%s)'
>> > + local printf_format='(%s)'
>> >
>> >   case "$#" in
>> >   2) pcmode=yes
>>
>>
>> These last 2 lines say: if 2 arguments are given, use pcmode. Otherwise you get command-subtitution mode, which gives weird effects when being called from PROMPT_COMMAND.
>>
>
> Still, it seems to work with above "patch"..
>




--
Piotr Krukowiecki

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Simon Oosthoek]

On 28/11/12 19:02, Piotr Krukowiecki wrote:
>     Is your setting?:
>     PROMPT_COMMAND=__git_ps1
> 
> 
> That's right
>  
> 
>     I believe you need to give 2 parameters in order to use it in
>     PROMPT_COMMAND mode.
> 
> 
> Are you sure? git-prompt.sh says:
> 
> #    3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
> #        To customize the prompt, provide start/end arguments
> #        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
> 
> I interpret this as: if you don't want to customize the prompt, the
> simple "PROMPT_COMMAND=__git_ps1" is enough.
>  

I'm sure, although you're right this documentation is wrong...
I guess it must have slipped by when the final changes were made to the
code. (The requirement to have 2 arguments for PROMPT_COMMAND mode were
one of the last changes before being accepted into the release process.)

I've been too busy with other stuff to take another look at this in the
meantime.

perhaps the point should read like this:
#    3a) In ~/.bashrc set PROMPT_COMMAND
#        To customize the prompt, provide start/end arguments
#        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'

Which would not be confusing at all, I think...

> 
> 
>     These last 2 lines say: if 2 arguments are given, use pcmode.
>     Otherwise you get command-subtitution mode, which gives weird
>     effects when being called from PROMPT_COMMAND.
> 
> 
> Still, it seems to work with above "patch"..

It only works in your specific case, since you're expecting the branch
info before the rest of your prompt. The output comes from the part of
the code that is meant for substitution mode ;-)

Cheers

Simon

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Junio C Hamano]

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

> perhaps the point should read like this:
> #    3a) In ~/.bashrc set PROMPT_COMMAND
> #        To customize the prompt, provide start/end arguments
> #        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>
> Which would not be confusing at all, I think...

It says "to customize", so a user who just wants the default (which
does not exist but the comment does not say so) would be left
without instruction, no?

    In $HOME/.bashrc, PROMPT_COMMAND can be set to
    '__git_ps1 <pre> <post>', where <pre> and <post>
    are strings you would put in $PS1 before and after
    the status string generated by git-prompt machinery.
    e.g.
        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'

or something?

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Simon Oosthoek]

On 28/11/12 21:47, Junio C Hamano wrote:
> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
> 
>> perhaps the point should read like this:
>> #    3a) In ~/.bashrc set PROMPT_COMMAND
>> #        To customize the prompt, provide start/end arguments
>> #        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>>
>> Which would not be confusing at all, I think...
> 
> It says "to customize", so a user who just wants the default (which
> does not exist but the comment does not say so) would be left
> without instruction, no?
> 
>     In $HOME/.bashrc, PROMPT_COMMAND can be set to
>     '__git_ps1 <pre> <post>', where <pre> and <post>
>     are strings you would put in $PS1 before and after
>     the status string generated by git-prompt machinery.
>     e.g.
>         PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
> 
> or something?
> 

Looks better than my suggestion :-)

thanks

/Simon

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Simon Oosthoek]

Hi Junio

This patch for the documentation doesn't seem to be in rc2 of 1.8.1...

tonight I was explaining this feature to a small group of people and I
pulled the git tree to get the latest code.

The current tagged 1.8.1-rc2 doesn't yet have your improvement and after
trying to explain it, I think it should be improved even more ;-)

I'm sorry to say I don't have the time right now to make a proper patch,
but I'll try to expand below...

On 28/11/12 21:47, Junio C Hamano wrote:
> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
> 
>> perhaps the point should read like this:
>> #    3a) In ~/.bashrc set PROMPT_COMMAND
>> #        To customize the prompt, provide start/end arguments
>> #        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>>
>> Which would not be confusing at all, I think...
> 
> It says "to customize", so a user who just wants the default (which
> does not exist but the comment does not say so) would be left
> without instruction, no?
> 
>     In $HOME/.bashrc, PROMPT_COMMAND can be set to
>     '__git_ps1 <pre> <post>', where <pre> and <post>
>     are strings you would put in $PS1 before and after
>     the status string generated by git-prompt machinery.
>     e.g.
>         PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
> 
> or something?
> 

The gist should be that:
__git_ps1 can function in two modes; The "traditional" way with command
substitution. This mode is activated when the function is provided 0 or
1 arguments. The new way, activated by providing exactly 2 arguments to
__git_ps1 is intended for use in the PROMPT_COMMAND way of bash to call
a function before writing the prompt. This command can then be (and is
in fact) used to set/overwrite the PS1 variable.

Only in the PROMPT_COMMAND mode can you get a prompt with colors for the
various states of the git tree showing in the prompt.

An example for inclusion in the ~/.bashrc file is:
-----snip-----
PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
GIT_PS1_SHOWDIRTYSTATE=true
GIT_PS1_SHOWCOLORHINTS=true
--------------

It turns out that it can be quite confusing to get your head around this
split personality of __git_ps1...

I hope you can somehow get this into the next release with some
improvement to the docs!

Cheers

Simon

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Junio C Hamano]

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

> This patch for the documentation doesn't seem to be in rc2 of 1.8.1...

There wasn't any patch, and after sending "something like this" I
forgot about the topic, as usual.

> The current tagged 1.8.1-rc2 doesn't yet have your improvement and after
> trying to explain it, I think it should be improved even more ;-)

I am not sure what "my improvement" you are refering to.  Is it only
about how the usage appears in the comment?

Perhaps like this?  I think the straight-forward one should be
listed as the primary usage, and the confusing one should be an
alternate for advanced users, so 3a/3b have been swapped.

 contrib/completion/git-prompt.sh | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git i/contrib/completion/git-prompt.sh w/contrib/completion/git-prompt.sh
index 00fc099..899eb09 100644
--- i/contrib/completion/git-prompt.sh
+++ w/contrib/completion/git-prompt.sh
@@ -10,14 +10,20 @@
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
 #    2) Add the following line to your .bashrc/.zshrc:
 #        source ~/.git-prompt.sh
-#    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
+#    3a) 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 optional argument will be used as format string.
+#    3b) Alternatively, if you are using bash, __git_ps1 can be
+#        used for PROMPT_COMMAND with two parameters, <pre> and
+#        <post>, which are strings you would put in $PS1 before
+#        and after the status string generated by the git-prompt
+#        machinery.  e.g.
+#           PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
+#        will show username, at-sign, host, colon, cwd, then
+#        various status string, followed by dollar and SP, as
+#        your prompt.
 #
 # 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

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Junio C Hamano]

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

> Perhaps like this?

OK, this time with a log message.

-- >8 --
Subject: [PATCH] git-prompt.sh: update PROMPT_COMMAND documentation

The description of __git_ps1 function operating in two-arg mode was
not very clear.  It said "set PROMPT_COMMAND=__git_ps1" which is not
the right usage for this mode, followed by "To customize the prompt,
do this", giving a false impression that those who do not want to
customize it can get away with no-arg form, which was incorrect.

Make it clear that this mode always takes two arguments, pre and
post, with an example.

The straight-forward one should be listed as the primary usage, and
the confusing one should be an alternate for advanced users.  Swap
the order of these two.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/completion/git-prompt.sh | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index a8b53ba..9b074e1 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -10,14 +10,20 @@
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
 #    2) Add the following line to your .bashrc/.zshrc:
 #        source ~/.git-prompt.sh
-#    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
+#    3a) 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 optional argument will be used as format string.
+#    3b) Alternatively, if you are using bash, __git_ps1 can be
+#        used for PROMPT_COMMAND with two parameters, <pre> and
+#        <post>, which are strings you would put in $PS1 before
+#        and after the status string generated by the git-prompt
+#        machinery.  e.g.
+#           PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
+#        will show username, at-sign, host, colon, cwd, then
+#        various status string, followed by dollar and SP, as
+#        your prompt.
 #
 # 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
-- 
1.8.1.rc1.128.gd8d1528

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Simon Oosthoek]

Hi Junio

This removes most of the ambiguities :-)
Ack from me!

I still have some minor nits, but I'll leave that for another time when I'm less busy.

BTW, I haven't tried this yet, but if you pass 2 arguments to __git_ps1 when called from command-substition mode, I suppose it will think it's in PC mode and overwrite the PS1!

At some point, I'd like to see this code split off into "pc" and "cs" functions which call a common function to get the git status. But that's a major rewrite and it may involve more overhead, since each function should process the output of the common function in a different way.

Cheers

Simon

* Junio C Hamano <gitster@pobox.com> [2012-12-11 16:03:36 -0800]:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Perhaps like this?
> 
> OK, this time with a log message.
> 
> -- >8 --
> Subject: [PATCH] git-prompt.sh: update PROMPT_COMMAND documentation
> 
> The description of __git_ps1 function operating in two-arg mode was
> not very clear.  It said "set PROMPT_COMMAND=__git_ps1" which is not
> the right usage for this mode, followed by "To customize the prompt,
> do this", giving a false impression that those who do not want to
> customize it can get away with no-arg form, which was incorrect.
> 
> Make it clear that this mode always takes two arguments, pre and
> post, with an example.
> 
> The straight-forward one should be listed as the primary usage, and
> the confusing one should be an alternate for advanced users.  Swap
> the order of these two.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  contrib/completion/git-prompt.sh | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index a8b53ba..9b074e1 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -10,14 +10,20 @@
>  #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
>  #    2) Add the following line to your .bashrc/.zshrc:
>  #        source ~/.git-prompt.sh
> -#    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
> +#    3a) 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 optional argument will be used as format string.
> +#    3b) Alternatively, if you are using bash, __git_ps1 can be
> +#        used for PROMPT_COMMAND with two parameters, <pre> and
> +#        <post>, which are strings you would put in $PS1 before
> +#        and after the status string generated by the git-prompt
> +#        machinery.  e.g.
> +#           PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
> +#        will show username, at-sign, host, colon, cwd, then
> +#        various status string, followed by dollar and SP, as
> +#        your prompt.
>  #
>  # 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
> -- 
> 1.8.1.rc1.128.gd8d1528
> 

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Junio C Hamano]

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

> This removes most of the ambiguities :-)
> Ack from me!

OK, as this is a low-impact finishing touch for a new feature, I'll
fast-track this to 'master' before the final release.

Thanks.

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Simon Oosthoek]

On 12/12/12 18:50, Junio C Hamano wrote:
> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
> 
>> This removes most of the ambiguities :-)
>> Ack from me!
> 
> OK, as this is a low-impact finishing touch for a new feature, I'll
> fast-track this to 'master' before the final release.
> 

Ok, wonderful!
BTW, I tried the thing I mentioned and it was safe to do:
PS1='blabla$(__git_ps1 x y)blabla'
will not eat your prompt, although I'd recommend putting something
useful instead of blabla ;-)

Cheers

Simon

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Junio C Hamano]

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

> On 12/12/12 18:50, Junio C Hamano wrote:
>> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
>> 
>>> This removes most of the ambiguities :-)
>>> Ack from me!
>> 
>> OK, as this is a low-impact finishing touch for a new feature, I'll
>> fast-track this to 'master' before the final release.
>> 
>
> Ok, wonderful!
> BTW, I tried the thing I mentioned and it was safe to do:
> PS1='blabla$(__git_ps1 x y)blabla'
> will not eat your prompt, although I'd recommend putting something
> useful instead of blabla ;-)

Actually, I deeply regret merging this to 'master'.  The original
"as a command substitution in PS1" mode, you could add anything
around the status string, so I could do:

	PS1=': \h \W$(__git_ps1 "/%s"); '

to get something like:

	: hostname dirname/<STATUS>; <CURSOR HERE>

In the new PROMPT_COMMAND mode, there is always parentheses around
the status string (and an SP before the parenthesees) that the user
cannot get rid of.

This is not a usability regression per-se (if you do not like the
extra parentheses, you do not have to use the colored mode), but is
something that will make me never use the mode.

Can we make it take an optional third parameter so that we could say

	PROMPT_COMMAND='__git_ps1 ": \h \W" "; " "/%s"'

to do the same as what the command substitution mode would have
given for

	PS1=': \h \W$(__git_ps1 "/%s"); '

perhaps?

Totally untested, but perhaps along this line.

 contrib/completion/git-prompt.sh | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 9b074e1..b2579f4 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -236,9 +236,10 @@ __git_ps1 ()
 	local printf_format=' (%s)'
 
 	case "$#" in
-		2)	pcmode=yes
+		2|3)	pcmode=yes
 			ps1pc_start="$1"
 			ps1pc_end="$2"
+			printf_format="${3:-$printf_format}"
 		;;
 		0|1)	printf_format="${1:-$printf_format}"
 		;;
@@ -339,6 +340,7 @@ __git_ps1 ()
 
 		local f="$w$i$s$u"
 		if [ $pcmode = yes ]; then
+			local ps1=
 			if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
 				local c_red='\e[31m'
 				local c_green='\e[32m'
@@ -356,29 +358,31 @@ __git_ps1 ()
 					branch_color="$bad_color"
 				fi
 
-				# Setting PS1 directly with \[ and \] around colors
+				# Setting ps1 directly with \[ and \] around colors
 				# is necessary to prevent wrapping issues!
-				PS1="$ps1pc_start (\[$branch_color\]$branchstring\[$c_clear\]"
+				ps1="\[$branch_color\]$branchstring\[$c_clear\]"
 
 				if [ -n "$w$i$s$u$r$p" ]; then
-					PS1="$PS1 "
+					ps1="$ps1 "					
 				fi
 				if [ "$w" = "*" ]; then
-					PS1="$PS1\[$bad_color\]$w"
+					ps1="$ps1\[$bad_color\]$w"
 				fi
 				if [ -n "$i" ]; then
-					PS1="$PS1\[$ok_color\]$i"
+					ps1="$ps1\[$ok_color\]$i"
 				fi
 				if [ -n "$s" ]; then
-					PS1="$PS1\[$flags_color\]$s"
+					ps1="$ps1\[$flags_color\]$s"
 				fi
 				if [ -n "$u" ]; then
-					PS1="$PS1\[$bad_color\]$u"
+					ps1="$ps1\[$bad_color\]$u"
 				fi
-				PS1="$PS1\[$c_clear\]$r$p)$ps1pc_end"
+				ps1="$ps1\[$c_clear\]$r$p"
 			else
-				PS1="$ps1pc_start ($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
+				ps1="$c${b##refs/heads/}${f:+ $f}$r$p"
 			fi
+			ps1=$(printf -- "$printf_format" "$ps1")
+			PS1="$ps1pc_start$ps1$ps1pc_end"
 		else
 			# NO color option unless in PROMPT_COMMAND mode
 			printf -- "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p"

Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format

[Simon Oosthoek]

* Junio C Hamano <gitster@pobox.com> [2012-12-25 23:47:53 -0800]:
 
> Can we make it take an optional third parameter so that we could say
> 
> 	PROMPT_COMMAND='__git_ps1 ": \h \W" "; " "/%s"'
> 
> to do the same as what the command substitution mode would have
> given for
> 
> 	PS1=': \h \W$(__git_ps1 "/%s"); '
> 
> perhaps?
> 
> Totally untested, but perhaps along this line.
> 

I tried your patch and (to my surprise, after the first reading) it worked.

I've further modified git-prompt.sh to include more usage text and I changed
the name of ps1 to gitstring, as it might be confused with PS1 upon casual
reading.

I'll be sending a format-patch patchmail ASAP...


>  contrib/completion/git-prompt.sh | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index 9b074e1..b2579f4 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -236,9 +236,10 @@ __git_ps1 ()
>  	local printf_format=' (%s)'
>  
>  	case "$#" in
> -		2)	pcmode=yes
> +		2|3)	pcmode=yes
>  			ps1pc_start="$1"
>  			ps1pc_end="$2"
> +			printf_format="${3:-$printf_format}"
>  		;;
>  		0|1)	printf_format="${1:-$printf_format}"
>  		;;
> @@ -339,6 +340,7 @@ __git_ps1 ()
>  
>  		local f="$w$i$s$u"
>  		if [ $pcmode = yes ]; then
> +			local ps1=
>  			if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
>  				local c_red='\e[31m'
>  				local c_green='\e[32m'
> @@ -356,29 +358,31 @@ __git_ps1 ()
>  					branch_color="$bad_color"
>  				fi
>  
> -				# Setting PS1 directly with \[ and \] around colors
> +				# Setting ps1 directly with \[ and \] around colors
>  				# is necessary to prevent wrapping issues!
> -				PS1="$ps1pc_start (\[$branch_color\]$branchstring\[$c_clear\]"
> +				ps1="\[$branch_color\]$branchstring\[$c_clear\]"
>  
>  				if [ -n "$w$i$s$u$r$p" ]; then
> -					PS1="$PS1 "
> +					ps1="$ps1 "					
>  				fi
>  				if [ "$w" = "*" ]; then
> -					PS1="$PS1\[$bad_color\]$w"
> +					ps1="$ps1\[$bad_color\]$w"
>  				fi
>  				if [ -n "$i" ]; then
> -					PS1="$PS1\[$ok_color\]$i"
> +					ps1="$ps1\[$ok_color\]$i"
>  				fi
>  				if [ -n "$s" ]; then
> -					PS1="$PS1\[$flags_color\]$s"
> +					ps1="$ps1\[$flags_color\]$s"
>  				fi
>  				if [ -n "$u" ]; then
> -					PS1="$PS1\[$bad_color\]$u"
> +					ps1="$ps1\[$bad_color\]$u"
>  				fi
> -				PS1="$PS1\[$c_clear\]$r$p)$ps1pc_end"
> +				ps1="$ps1\[$c_clear\]$r$p"
>  			else
> -				PS1="$ps1pc_start ($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
> +				ps1="$c${b##refs/heads/}${f:+ $f}$r$p"
>  			fi
> +			ps1=$(printf -- "$printf_format" "$ps1")
> +			PS1="$ps1pc_start$ps1$ps1pc_end"
>  		else
>  			# NO color option unless in PROMPT_COMMAND mode
>  			printf -- "$printf_format" "$c${b##refs/heads/}${f:+ $f}$r$p"

/Simon

[PATCH] make __git_ps1 accept a third parameter in pcmode

[Simon Oosthoek]

The optional third parameter when __git_ps1 is used in
PROMPT_COMMAND mode as format string for printf to further
customize the way the git status string is embedded in the
user's PS1 prompt.

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

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 9b074e1..2922bb3 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -24,6 +24,8 @@
 #        will show username, at-sign, host, colon, cwd, then
 #        various status string, followed by dollar and SP, as
 #        your prompt.
+#        Optionally, you can supply a third argument with a printf
+#        format string to finetune the output of the branch status
 #
 # 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
@@ -222,10 +224,12 @@ __git_ps1_show_upstream ()
 # 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)
+# __git_ps1 requires 2 or 3 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
+# when two arguments are given, the first is prepended and the second appended
 # to the state string when assigned to PS1.
+# The optional third parameter will be used as printf format string to further
+# customize the output of the git-status string.
 # In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true
 __git_ps1 ()
 {
@@ -236,9 +240,10 @@ __git_ps1 ()
 	local printf_format=' (%s)'
 
 	case "$#" in
-		2)	pcmode=yes
+		2|3)	pcmode=yes
 			ps1pc_start="$1"
 			ps1pc_end="$2"
+			printf_format="${3:-$printf_format}"
 		;;
 		0|1)	printf_format="${1:-$printf_format}"
 		;;
@@ -339,6 +344,7 @@ __git_ps1 ()
 
 		local f="$w$i$s$u"
 		if [ $pcmode = yes ]; then
+			local gitstring=
 			if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
 				local c_red='\e[31m'
 				local c_green='\e[32m'
@@ -356,29 +362,31 @@ __git_ps1 ()
 					branch_color="$bad_color"
 				fi
 
-				# Setting PS1 directly with \[ and \] around colors
+				# Setting gitstring directly with \[ and \] around colors
 				# is necessary to prevent wrapping issues!
-				PS1="$ps1pc_start (\[$branch_color\]$branchstring\[$c_clear\]"
+				gitstring="\[$branch_color\]$branchstring\[$c_clear\]"
 
 				if [ -n "$w$i$s$u$r$p" ]; then
-					PS1="$PS1 "
+					gitstring="$gitstring "					
 				fi
 				if [ "$w" = "*" ]; then
-					PS1="$PS1\[$bad_color\]$w"
+					gitstring="$gitstring\[$bad_color\]$w"
 				fi
 				if [ -n "$i" ]; then
-					PS1="$PS1\[$ok_color\]$i"
+					gitstring="$gitstring\[$ok_color\]$i"
 				fi
 				if [ -n "$s" ]; then
-					PS1="$PS1\[$flags_color\]$s"
+					gitstring="$gitstring\[$flags_color\]$s"
 				fi
 				if [ -n "$u" ]; then
-					PS1="$PS1\[$bad_color\]$u"
+					gitstring="$gitstring\[$bad_color\]$u"
 				fi
-				PS1="$PS1\[$c_clear\]$r$p)$ps1pc_end"
+				gitstring="$gitstring\[$c_clear\]$r$p"
 			else
-				PS1="$ps1pc_start ($c${b##refs/heads/}${f:+ $f}$r$p)$ps1pc_end"
+				gitstring="$c${b##refs/heads/}${f:+ $f}$r$p"
 			fi
+			gitstring=$(printf -- "$printf_format" "$gitstring")
+			PS1="$ps1pc_start$gitstring$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

PS, I was surprised that this worked, because I was under the impression that the \[ \] special characters needed to be put in PS1 directly. This is apparently not exactly the case, but I'm now more confused than before ;-)

Re: [PATCH] make __git_ps1 accept a third parameter in pcmode

[Junio C Hamano]

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

> The optional third parameter when __git_ps1 is used in
> PROMPT_COMMAND mode as format string for printf to further
> customize the way the git status string is embedded in the
> user's PS1 prompt.
>
> Signed-off-by: Simon Oosthoek <s.oosthoek@xs4all.nl>
> ---

Thanks.

If we do not care about the existing users (and in this case,
because PROMPT_COMMAND mode is in no released version, we could
declare there is no existing user), another and simpler approach is
to just drop " (" and ")" altogether and have the user give these as
part of the pre/post strings.

Or we could go the other way and drop "pre/post" strings, making
them part of the printf_format string.  Perhaps that might be a
better interface in the longer term.  Then people can use the same
"<pre>%s<post>" format string and do either of these:

	PS1=$(__git_ps1 "<pre>%s<post>")
	PROMPT_COMMAND='PS1=$(__git_ps1 "<pre>%s<post>")'

without __git_ps1 having a special "prompt command" mode, no?

I have a feeling that I am missing something major, though...

>  				if [ "$w" = "*" ]; then
> -					PS1="$PS1\[$bad_color\]$w"
> +					gitstring="$gitstring\[$bad_color\]$w"
>  				fi

Every time I looked at this line, I wondered why '*' state is
"bad".  Does a user go into any "bad" state by having a dirty
working tree?  Same for untracked ($u) and detached.  These are all
perfectly normal part of a workflow, so while choice of red may be
fine to attract attention, calling it "bad" sounds misguided.

Re: [PATCH] make __git_ps1 accept a third parameter in pcmode

[Simon Oosthoek]

* Junio C Hamano <gitster@pobox.com> [2012-12-26 11:45:48 -0800]:

> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
> 
> > The optional third parameter when __git_ps1 is used in
> > PROMPT_COMMAND mode as format string for printf to further
> > customize the way the git status string is embedded in the
> > user's PS1 prompt.
> >
> > Signed-off-by: Simon Oosthoek <s.oosthoek@xs4all.nl>
> > ---
> 
> Thanks.
> 
> If we do not care about the existing users (and in this case,
> because PROMPT_COMMAND mode is in no released version, we could
> declare there is no existing user), another and simpler approach is
> to just drop " (" and ")" altogether and have the user give these as
> part of the pre/post strings.
> 

The problem with doing it in pre-post is when inside non-git directories. You want to avoid any gitstring output, including the brackets, when not inside a repository.

Doing it all in the third parameter is perhaps a better approach, but then it becomes mandatory instead of optional.

> Or we could go the other way and drop "pre/post" strings, making
> them part of the printf_format string.  Perhaps that might be a
> better interface in the longer term.  Then people can use the same
> "<pre>%s<post>" format string and do either of these:
> 
> 	PS1=$(__git_ps1 "<pre>%s<post>")
> 	PROMPT_COMMAND='PS1=$(__git_ps1 "<pre>%s<post>")'
> 
> without __git_ps1 having a special "prompt command" mode, no?

But how to determine which mode to use?
In pcmode, you must set the PS1, in command-subsitute mode, you must print a formatted gitstring.

> 
> I have a feeling that I am missing something major, though...

I think the fundamentally different way of setting the PS1 between the two modes is very confusing. Which is why I originally made a different function (with duplicated code) for both modes.


> 
> >  				if [ "$w" = "*" ]; then
> > -					PS1="$PS1\[$bad_color\]$w"
> > +					gitstring="$gitstring\[$bad_color\]$w"
> >  				fi
> 
> Every time I looked at this line, I wondered why '*' state is
> "bad".  Does a user go into any "bad" state by having a dirty
> working tree?  Same for untracked ($u) and detached.  These are all
> perfectly normal part of a workflow, so while choice of red may be
> fine to attract attention, calling it "bad" sounds misguided.


Well, I'm most often a really casual user of git and to make this function work the way I want to, I found out by trial-and-error that this was a way to test whether it's time to colour the string red or green ;-)

I'm very open to better ways to determine the colour modes. Anyway, the colours are now more or less the same as what git itself uses when printing the status with colour hints in git status.

Cheers

Simon

Re: [PATCH] make __git_ps1 accept a third parameter in pcmode

[Junio C Hamano]

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

> The problem with doing it in pre-post is when inside non-git
> directories. You want to avoid any gitstring output, including the
> brackets, when not inside a repository.

Ah, Ok, that is probably what I missed.

>> Or we could go the other way and drop "pre/post" strings, making
>> them part of the printf_format string.  Perhaps that might be a
>> better interface in the longer term.  Then people can use the same
>> "<pre>%s<post>" format string and do either of these:
>> 
>> 	PS1=$(__git_ps1 "<pre>%s<post>")
>> 	PROMPT_COMMAND='PS1=$(__git_ps1 "<pre>%s<post>")'
>> 
>> without __git_ps1 having a special "prompt command" mode, no?
>
> But how to determine which mode to use?
> In pcmode, you must set the PS1, in command-subsitute mode, you must print a formatted gitstring.

The point of the above two was that __git_ps1 does not have to set
PS1 as long as the insn says user to use PROMPT_COMMAND that sets
PS1 himself, exactly as illustrated above.  In other words, replace
the last PS1=...  in the "prompt command" mode with an echo or
something and make the user responsible for assigning it to PS1 in
his PROMPT_COMMAND.

Or put it in another way, I was hoping that we can do without adding
the prompt command mode---if there is no two modes, there is no need
to switch between them.

But as I said, there probably is a reason why that approach does not
work, that is why I said...

>> I have a feeling that I am missing something major, though...

Re: [PATCH] make __git_ps1 accept a third parameter in pcmode

[Junio C Hamano]

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

>> Every time I looked at this line, I wondered why '*' state is
>> "bad".  Does a user go into any "bad" state by having a dirty
>> working tree?  Same for untracked ($u) and detached.  These are all
>> perfectly normal part of a workflow, so while choice of red may be
>> fine to attract attention, calling it "bad" sounds misguided.
>
> Well, I'm most often a really casual user of git and to make this
> function work the way I want to, I found out by trial-and-error
> that this was a way to test whether it's time to colour the string
> red or green ;-)
>
> I'm very open to better ways to determine the colour
> modes. Anyway, the colours are now more or less the same as what
> git itself uses when printing the status with colour hints in git
> status.

Oh, I am not opposed to the choice of colors; something that wants
an attention from the user may be better in red.

I was merely commenting on the choice of the variable name, the user
of word "bad" as if the conditions that use this color were "bad" in
some way.

Re: [PATCH] make __git_ps1 accept a third parameter in pcmode

[Junio C Hamano]

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

> But as I said, there probably is a reason why that approach does not
> work, that is why I said...
>
>>> I have a feeling that I am missing something major, though...

In any case, this was more like "if we were doing this from scratch"
conversation.

I think PROMPT_COMMAND mode that takes <pre> and <post> string has
been advertised throughout the pre-release freeze, which is long
enough in Git timescale to avoid backward incompatibility, so we are
already stuck with the function in two modes even if what I said in
the previous message (i.e. "why not just one mode") worked.

I am considering to fast-track the "optional third parameter" patch
to the 1.8.1 final, so that we can have the same degree of
customizability in both modes.

Thanks.

Re: [PATCH] make __git_ps1 accept a third parameter in pcmode

[Simon Oosthoek]

* Junio C Hamano <gitster@pobox.com> [2012-12-26 12:32:20 -0800]:
> The point of the above two was that __git_ps1 does not have to set
> PS1 as long as the insn says user to use PROMPT_COMMAND that sets
> PS1 himself, exactly as illustrated above.  In other words, replace
> the last PS1=...  in the "prompt command" mode with an echo or
> something and make the user responsible for assigning it to PS1 in
> his PROMPT_COMMAND.
> 
> Or put it in another way, I was hoping that we can do without adding
> the prompt command mode---if there is no two modes, there is no need
> to switch between them.
> 
> But as I said, there probably is a reason why that approach does not
> work, that is why I said...
> 

The only reason to my knowledge is that bash's handling of zero-length strings, like terminal colour commands, is producing a PS1 that outputs less visible characters than bash thinks and thus bash makes mistakes when wrapping the commandline. The way to prevent that is to use \[ and \] around those and that doesn't seem to work from a string produced from command-substitution. (BTW, the colours come through just fine, just the \[ and \] don't)

Another approach could be to split up the functionality and have a few support functions to set various variables (corresponding with the gitstring features, like *%+ characters and colour hints). These variables could then be used by a custom PROMPT_COMMAND function or a command substitution function to produce a gitstring. I suppose that would mean a complete rewrite or very close to it ;-)

/Simon