threads / patch / 33889

patchprompt: fix show upstream with svn and zsh

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

## tl;dr

14 messages between May 21, 2013 and May 28, 2013. Diffs are folded; open one to read it.

replies: 13people: 3as markdown or json

Thomas Gummerer· May 21, 2013, 20:54 UTC · lore

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(-)
Show changes to contrib/completion/git-prompt.sh +3 −3
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
SZEDER Gábor· May 21, 2013, 22:41 UTC · re: Thomas Gummerer · lore

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

Hi,
On Tue, May 21, 2013 at 10:54:27PM +0200, Thomas Gummerer wrote:
Show 8 quoted lines
> 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 '[' ?!
Show 10 quoted lines
> @@ -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

Felipe Contreras· May 21, 2013, 23:04 UTC · re: SZEDER Gábor · lore

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

On Tue, May 21, 2013 at 5:41 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 12 quoted lines
> 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
SZEDER Gábor· May 21, 2013, 23:36 UTC · re: Felipe Contreras · lore

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

On Tue, May 21, 2013 at 06:04:35PM -0500, Felipe Contreras wrote:
Show 16 quoted lines
> 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

Felipe Contreras· May 22, 2013, 00:03 UTC · re: SZEDER Gábor · lore

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

On Tue, May 21, 2013 at 6:36 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 19 quoted lines
> 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
SZEDER Gábor· May 22, 2013, 00:20 UTC · re: Felipe Contreras · lore

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

On Tue, May 21, 2013 at 07:03:09PM -0500, Felipe Contreras wrote:
Show 24 quoted lines
> 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

Felipe Contreras· May 22, 2013, 00:29 UTC · re: SZEDER Gábor · lore

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

On Tue, May 21, 2013 at 7:20 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 28 quoted lines
> 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
SZEDER Gábor· May 22, 2013, 00:44 UTC · re: Felipe Contreras · lore

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

On Tue, May 21, 2013 at 07:29:38PM -0500, Felipe Contreras wrote:
Show 35 quoted lines
> 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
Thomas Gummerer· May 22, 2013, 07:32 UTC · re: SZEDER Gábor · lore

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

SZEDER Gábor <szeder@ira.uka.de> writes:
Show 20 quoted lines
> 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.

Show 12 quoted lines
>> @@ -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

SZEDER Gábor· May 28, 2013, 10:30 UTC · re: Thomas Gummerer · lore

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

Hi,
On Wed, May 22, 2013 at 09:32:44AM +0200, Thomas Gummerer wrote:
Show 10 quoted lines
> 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
Felipe Contreras· May 22, 2013, 03:13 UTC · re: Thomas Gummerer · lore

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

On Tue, May 21, 2013 at 3:54 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
Show 10 quoted lines
> 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
Thomas Gummerer· May 22, 2013, 07:36 UTC · re: Felipe Contreras · lore

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

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 18 quoted lines
> 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.

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

Thanks, Thomas

Thomas Gummerer· May 22, 2013, 07:40 UTC · re: Thomas Gummerer · lore

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

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(-)
Show changes to contrib/completion/git-prompt.sh +4 −3
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
Felipe Contreras· May 22, 2013, 07:56 UTC · re: Thomas Gummerer · lore

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

On Wed, May 22, 2013 at 2:40 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
Show 25 quoted lines
> 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

← back to recent threads