[PATCH] prompt: fix show upstream with svn and zsh

[PATCH] prompt: fix show upstream with svn and zsh

[Thomas Gummerer]

Currently the __git_ps1 git prompt gives the following error with a
repository converted by git-svn, when used with zsh:

	   __git_ps1_show_upstream:19: bad pattern: svn_remote[

This was introduced by 6d158cba (bash completion: Support "divergence
from upstream" messages in __git_ps1), when the script was for bash
only.  Make it compatible with zsh.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Tested with bash 4.2.45 and zsh 5.0.2.

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

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index eaf5c36..e537300 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -124,7 +124,7 @@ __git_ps1_show_upstream ()
 			fi
 			;;
 		svn-remote.*.url)
-			svn_remote[ $((${#svn_remote[@]} + 1)) ]="$value"
+			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
 			svn_url_pattern+="\\|$value"
 			upstream=svn+git # default upstream is SVN if available, else git
 			;;
@@ -146,8 +146,8 @@ __git_ps1_show_upstream ()
 	svn*)
 		# get the upstream from the "git-svn-id: ..." in a commit message
 		# (git-svn uses essentially the same procedure internally)
-		local svn_upstream=($(git log --first-parent -1 \
-					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null))
+		set -a svn_upstream "$(git log --first-parent -1 \
+					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null)"
 		if [[ 0 -ne ${#svn_upstream[@]} ]]; then
 			svn_upstream=${svn_upstream[ ${#svn_upstream[@]} - 2 ]}
 			svn_upstream=${svn_upstream%@*}
-- 
1.8.3.rc2.359.g2fb82f5

Re: [PATCH] prompt: fix show upstream with svn and zsh

[SZEDER Gábor]

Hi,


On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
> Currently the __git_ps1 git prompt gives the following error with a
> repository converted by git-svn, when used with zsh:
> 
> 	   __git_ps1_show_upstream:19: bad pattern: svn_remote[
> 
> This was introduced by 6d158cba (bash completion: Support "divergence
> from upstream" messages in __git_ps1), when the script was for bash
> only.  Make it compatible with zsh.

What is the actual cause of this problem/incompatibility and how/why do
these changes fix it?

> -			svn_remote[ $((${#svn_remote[@]} + 1)) ]="$value"
> +			svn_remote[$((${#svn_remote[@]} + 1))]="$value"

I mean, did zsh really complained because of the space after the '[' ?!

> @@ -146,8 +146,8 @@ __git_ps1_show_upstream ()
>  	svn*)
>  		# get the upstream from the "git-svn-id: ..." in a commit message
>  		# (git-svn uses essentially the same procedure internally)
> -		local svn_upstream=($(git log --first-parent -1 \
> -					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null))
> +		set -a svn_upstream "$(git log --first-parent -1 \
> +					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null)"
>  		if [[ 0 -ne ${#svn_upstream[@]} ]]; then
>  			svn_upstream=${svn_upstream[ ${#svn_upstream[@]} - 2 ]}

If so, then what about this one?


Best,
Gábor

Re: [PATCH] prompt: fix show upstream with svn and zsh

[Felipe Contreras]

On Tue, May 21, 2013 at 5:41 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:

> On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
>> Currently the __git_ps1 git prompt gives the following error with a
>> repository converted by git-svn, when used with zsh:
>>
>>          __git_ps1_show_upstream:19: bad pattern: svn_remote[
>>
>> This was introduced by 6d158cba (bash completion: Support "divergence
>> from upstream" messages in __git_ps1), when the script was for bash
>> only.  Make it compatible with zsh.
>
> What is the actual cause of this problem/incompatibility and how/why do
> these changes fix it?

I think the commit message makes that very clear.

-- 
Felipe Contreras

Re: [PATCH] prompt: fix show upstream with svn and zsh

[SZEDER Gábor]

On Tue, May 21, 2013 at 06:04:35PM -0500, Felipe Contreras wrote:
> On Tue, May 21, 2013 at 5:41 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> 
> > On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
> >> Currently the __git_ps1 git prompt gives the following error with a
> >> repository converted by git-svn, when used with zsh:
> >>
> >>          __git_ps1_show_upstream:19: bad pattern: svn_remote[
> >>
> >> This was introduced by 6d158cba (bash completion: Support "divergence
> >> from upstream" messages in __git_ps1), when the script was for bash
> >> only.  Make it compatible with zsh.
> >
> > What is the actual cause of this problem/incompatibility and how/why do
> > these changes fix it?
> 
> I think the commit message makes that very clear.

If that were the case I wouldn't have asked in the first place.

Hth,
Gábor

Re: [PATCH] prompt: fix show upstream with svn and zsh

[Felipe Contreras]

On Tue, May 21, 2013 at 6:36 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> On Tue, May 21, 2013 at 06:04:35PM -0500, Felipe Contreras wrote:
>> On Tue, May 21, 2013 at 5:41 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>>
>> > On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
>> >> Currently the __git_ps1 git prompt gives the following error with a
>> >> repository converted by git-svn, when used with zsh:
>> >>
>> >>          __git_ps1_show_upstream:19: bad pattern: svn_remote[
>> >>
>> >> This was introduced by 6d158cba (bash completion: Support "divergence
>> >> from upstream" messages in __git_ps1), when the script was for bash
>> >> only.  Make it compatible with zsh.
>> >
>> > What is the actual cause of this problem/incompatibility and how/why do
>> > these changes fix it?
>>
>> I think the commit message makes that very clear.
>
> If that were the case I wouldn't have asked in the first place.

You are not the authority on what *I think*, or if you meant s/If that
were the case/If the message was clear/, still; you are not the
authority on what is or is not true. Only on what is your opinion.

-- 
Felipe Contreras

Re: [PATCH] prompt: fix show upstream with svn and zsh

[SZEDER Gábor]

On Tue, May 21, 2013 at 07:03:09PM -0500, Felipe Contreras wrote:
> On Tue, May 21, 2013 at 6:36 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> > On Tue, May 21, 2013 at 06:04:35PM -0500, Felipe Contreras wrote:
> >> On Tue, May 21, 2013 at 5:41 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> >>
> >> > On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
> >> >> Currently the __git_ps1 git prompt gives the following error with a
> >> >> repository converted by git-svn, when used with zsh:
> >> >>
> >> >>          __git_ps1_show_upstream:19: bad pattern: svn_remote[
> >> >>
> >> >> This was introduced by 6d158cba (bash completion: Support "divergence
> >> >> from upstream" messages in __git_ps1), when the script was for bash
> >> >> only.  Make it compatible with zsh.
> >> >
> >> > What is the actual cause of this problem/incompatibility and how/why do
> >> > these changes fix it?
> >>
> >> I think the commit message makes that very clear.
> >
> > If that were the case I wouldn't have asked in the first place.
> 
> You are not the authority on what *I think*, or if you meant s/If that
> were the case/If the message was clear/, still; you are not the
> authority on what is or is not true. Only on what is your opinion.

I would have preferred a more constructive reply, perhaps with answers
to the questions I asked earlier...

Bye,
Gábor

Re: [PATCH] prompt: fix show upstream with svn and zsh

[Felipe Contreras]

On Tue, May 21, 2013 at 7:20 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> On Tue, May 21, 2013 at 07:03:09PM -0500, Felipe Contreras wrote:
>> On Tue, May 21, 2013 at 6:36 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>> > On Tue, May 21, 2013 at 06:04:35PM -0500, Felipe Contreras wrote:
>> >> On Tue, May 21, 2013 at 5:41 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>> >>
>> >> > On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
>> >> >> Currently the __git_ps1 git prompt gives the following error with a
>> >> >> repository converted by git-svn, when used with zsh:
>> >> >>
>> >> >>          __git_ps1_show_upstream:19: bad pattern: svn_remote[
>> >> >>
>> >> >> This was introduced by 6d158cba (bash completion: Support "divergence
>> >> >> from upstream" messages in __git_ps1), when the script was for bash
>> >> >> only.  Make it compatible with zsh.
>> >> >
>> >> > What is the actual cause of this problem/incompatibility and how/why do
>> >> > these changes fix it?
>> >>
>> >> I think the commit message makes that very clear.
>> >
>> > If that were the case I wouldn't have asked in the first place.
>>
>> You are not the authority on what *I think*, or if you meant s/If that
>> were the case/If the message was clear/, still; you are not the
>> authority on what is or is not true. Only on what is your opinion.
>
> I would have preferred a more constructive reply, perhaps with answers
> to the questions I asked earlier...

That's the job of the patch submitter, all I did was express my option
about the commit message. You have no say as to what is my option.

You are the one not being constructive by claiming that all opinions
contrary to yours are wrong.

-- 
Felipe Contreras

Re: [PATCH] prompt: fix show upstream with svn and zsh

[SZEDER Gábor]

On Tue, May 21, 2013 at 07:29:38PM -0500, Felipe Contreras wrote:
> On Tue, May 21, 2013 at 7:20 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> > On Tue, May 21, 2013 at 07:03:09PM -0500, Felipe Contreras wrote:
> >> On Tue, May 21, 2013 at 6:36 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> >> > On Tue, May 21, 2013 at 06:04:35PM -0500, Felipe Contreras wrote:
> >> >> On Tue, May 21, 2013 at 5:41 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> >> >>
> >> >> > On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
> >> >> >> Currently the __git_ps1 git prompt gives the following error with a
> >> >> >> repository converted by git-svn, when used with zsh:
> >> >> >>
> >> >> >>          __git_ps1_show_upstream:19: bad pattern: svn_remote[
> >> >> >>
> >> >> >> This was introduced by 6d158cba (bash completion: Support "divergence
> >> >> >> from upstream" messages in __git_ps1), when the script was for bash
> >> >> >> only.  Make it compatible with zsh.
> >> >> >
> >> >> > What is the actual cause of this problem/incompatibility and how/why do
> >> >> > these changes fix it?
> >> >>
> >> >> I think the commit message makes that very clear.
> >> >
> >> > If that were the case I wouldn't have asked in the first place.
> >>
> >> You are not the authority on what *I think*, or if you meant s/If that
> >> were the case/If the message was clear/, still; you are not the
> >> authority on what is or is not true. Only on what is your opinion.
> >
> > I would have preferred a more constructive reply, perhaps with answers
> > to the questions I asked earlier...
> 
> That's the job of the patch submitter, all I did was express my option
> about the commit message. You have no say as to what is my option.
> 
> You are the one not being constructive by claiming that all opinions
> contrary to yours are wrong.

Seriously!?  Alright, I won't waste any more time on this, sorry.

Gábor

Re: [PATCH] prompt: fix show upstream with svn and zsh

[Thomas Gummerer]

SZEDER Gábor <szeder@ira.uka.de> writes:

> Hi,
>
>
> On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
>> Currently the __git_ps1 git prompt gives the following error with a
>> repository converted by git-svn, when used with zsh:
>>
>> 	   __git_ps1_show_upstream:19: bad pattern: svn_remote[
>>
>> This was introduced by 6d158cba (bash completion: Support "divergence
>> from upstream" messages in __git_ps1), when the script was for bash
>> only.  Make it compatible with zsh.
>
> What is the actual cause of this problem/incompatibility and how/why do
> these changes fix it?
>
>> -			svn_remote[ $((${#svn_remote[@]} + 1)) ]="$value"
>> +			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
>
> I mean, did zsh really complained because of the space after the '[' ?!

Yes, removing the spaces after the '[' fixes the problem.  I'm not very
proficient in shell scripting, so I can't tell if there is another
cause.

>> @@ -146,8 +146,8 @@ __git_ps1_show_upstream ()
>>  	svn*)
>>  		# get the upstream from the "git-svn-id: ..." in a commit message
>>  		# (git-svn uses essentially the same procedure internally)
>> -		local svn_upstream=($(git log --first-parent -1 \
>> -					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null))
>> +		set -a svn_upstream "$(git log --first-parent -1 \
>> +					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null)"
>>  		if [[ 0 -ne ${#svn_upstream[@]} ]]; then
>>  			svn_upstream=${svn_upstream[ ${#svn_upstream[@]} - 2 ]}
>
> If so, then what about this one?

You're right, this line gives an error too, the code just wasn't
following that path before.  I'll fix it in the re-roll.  Other than
those two I couldn't spot any other occurrence of this pattern.

> Best,
> Gábor

Thanks,
Thomas

Re: [PATCH] prompt: fix show upstream with svn and zsh

[SZEDER Gábor]

Hi,

On Wed, May 22, 2013 at 09:32:44AM +0200, Thomas Gummerer wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> > On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
> >> -			svn_remote[ $((${#svn_remote[@]} + 1)) ]="$value"
> >> +			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
> >
> > I mean, did zsh really complained because of the space after the '[' ?!
> 
> Yes, removing the spaces after the '[' fixes the problem.  I'm not very
> proficient in shell scripting, so I can't tell if there is another
> cause.

Thanks.  I first suspected that this was just an independent style
cleanup, that's why I asked for clarification.


Gábor

Re: [PATCH] prompt: fix show upstream with svn and zsh

[Felipe Contreras]

On Tue, May 21, 2013 at 3:54 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> Currently the __git_ps1 git prompt gives the following error with a
> repository converted by git-svn, when used with zsh:
>
>            __git_ps1_show_upstream:19: bad pattern: svn_remote[
>
> This was introduced by 6d158cba (bash completion: Support "divergence
> from upstream" messages in __git_ps1), when the script was for bash
> only.  Make it compatible with zsh.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>

This patch is fine by me. I would like to see an example of how to
trigger the issue with a standalone command in the commit message, but
it's not necessary. It would also make sense to address the comment
from Szeder that does raise questions about other places in the code
where 'array[ $foo ]' is used, maybe there's a caveat we are not
considering, or maybe your use-case did not execute that code.

And finally, I don't recall seen 'set -a' used elsewhere in the code.
If memory serves well, we have replaced 'local -a foo=value' with
'local -a foo\nfoo=value' before to fix zsh issues, I think that would
be safer.

But this patch is needed regardless, that, or the patch that broke
things should be reverted for v1.8.3.

Thomas, if you don't have time, let me know and I can take a look.

Cheers.

-- 
Felipe Contreras

Re: [PATCH] prompt: fix show upstream with svn and zsh

[Thomas Gummerer]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Tue, May 21, 2013 at 3:54 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
>> Currently the __git_ps1 git prompt gives the following error with a
>> repository converted by git-svn, when used with zsh:
>>
>>            __git_ps1_show_upstream:19: bad pattern: svn_remote[
>>
>> This was introduced by 6d158cba (bash completion: Support "divergence
>> from upstream" messages in __git_ps1), when the script was for bash
>> only.  Make it compatible with zsh.
>>
>> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
>
> This patch is fine by me. I would like to see an example of how to
> trigger the issue with a standalone command in the commit message, but
> it's not necessary. It would also make sense to address the comment
> from Szeder that does raise questions about other places in the code
> where 'array[ $foo ]' is used, maybe there's a caveat we are not
> considering, or maybe your use-case did not execute that code.

Yes, the code was not executed, it will be fixed in the re-roll.

> And finally, I don't recall seen 'set -a' used elsewhere in the code.
> If memory serves well, we have replaced 'local -a foo=value' with
> 'local -a foo\nfoo=value' before to fix zsh issues, I think that would
> be safer.

Yes, thanks for the suggestion, that works.

> But this patch is needed regardless, that, or the patch that broke
> things should be reverted for v1.8.3.

I don't think anything should or can be reverted, as this code was
introduced in 2010, but nobody triggered it until now.

> Thomas, if you don't have time, let me know and I can take a look.
>
> Cheers.
>
> --
> Felipe Contreras

Thanks,
Thomas

[PATCH v2] prompt: fix show upstream with svn and zsh

[Thomas Gummerer]

Currently the __git_ps1 git prompt gives the following error with a
repository converted by git-svn, when used with zsh:

   __git_ps1_show_upstream:19: bad pattern: svn_remote[
   __git_ps1_show_upstream:45: bad substitution

To reproduce the problem, the __git_ps1_show_upstream function can be
executed in a repository converted with git-svn.  Both those errors are
triggered by spaces after the '['.

Zsh also doesn't support initializing an array with `local var=(...)`.
This triggers the following error:

   __git_ps1_show_upstream:41: bad pattern: svn_upstream=(commit

Use
   local -a
   var=(...)
instead to make is compatible.

This was introduced by 6d158cba (bash completion: Support "divergence
from upstream" messages in __git_ps1), when the script was for bash
only.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 contrib/completion/git-prompt.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index eaf5c36..b6b1534 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -124,7 +124,7 @@ __git_ps1_show_upstream ()
 			fi
 			;;
 		svn-remote.*.url)
-			svn_remote[ $((${#svn_remote[@]} + 1)) ]="$value"
+			svn_remote[$((${#svn_remote[@]} + 1))]="$value"
 			svn_url_pattern+="\\|$value"
 			upstream=svn+git # default upstream is SVN if available, else git
 			;;
@@ -146,10 +146,11 @@ __git_ps1_show_upstream ()
 	svn*)
 		# get the upstream from the "git-svn-id: ..." in a commit message
 		# (git-svn uses essentially the same procedure internally)
-		local svn_upstream=($(git log --first-parent -1 \
+		local -a svn_upstream
+		svn_upstream=($(git log --first-parent -1 \
 					--grep="^git-svn-id: \(${svn_url_pattern#??}\)" 2>/dev/null))
 		if [[ 0 -ne ${#svn_upstream[@]} ]]; then
-			svn_upstream=${svn_upstream[ ${#svn_upstream[@]} - 2 ]}
+			svn_upstream=${svn_upstream[${#svn_upstream[@]} - 2]}
 			svn_upstream=${svn_upstream%@*}
 			local n_stop="${#svn_remote[@]}"
 			for ((n=1; n <= n_stop; n++)); do
-- 
1.8.3.rc2.359.g2fb82f5

Re: [PATCH v2] prompt: fix show upstream with svn and zsh

[Felipe Contreras]

On Wed, May 22, 2013 at 2:40 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> Currently the __git_ps1 git prompt gives the following error with a
> repository converted by git-svn, when used with zsh:
>
>    __git_ps1_show_upstream:19: bad pattern: svn_remote[
>    __git_ps1_show_upstream:45: bad substitution
>
> To reproduce the problem, the __git_ps1_show_upstream function can be
> executed in a repository converted with git-svn.  Both those errors are
> triggered by spaces after the '['.
>
> Zsh also doesn't support initializing an array with `local var=(...)`.
> This triggers the following error:
>
>    __git_ps1_show_upstream:41: bad pattern: svn_upstream=(commit
>
> Use
>    local -a
>    var=(...)
> instead to make is compatible.
>
> This was introduced by 6d158cba (bash completion: Support "divergence
> from upstream" messages in __git_ps1), when the script was for bash
> only.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>

I think there's no need to be so verbose in the commit message, but
hey, that's just an opinion.

Acknowledged-by: Felipe Contreras <felipe.contreras@gmail.com>

-- 
Felipe Contreras