git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Fix git-completion.bash for use in zsh
@ 2011-09-01 13:47 Alex Merry
  2012-03-20 11:53 ` Alex Merry
  0 siblings, 1 reply; 4+ messages in thread
From: Alex Merry @ 2011-09-01 13:47 UTC (permalink / raw)
  To: Shawn O. Pearce, git

Certain versions (or option combinations) of zsh appear to treat
things like
local some_var=()
as a function declaration.  This makes errors appear when using it in
combination with the GIT_PS1_SHOWUPSTREAM option.

Signed-off-by: Alex Merry <dev@randomguy3.me.uk>
---
 contrib/completion/git-completion.bash |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5a83090..89de45d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -106,8 +106,9 @@ __gitdir ()
 __git_ps1_show_upstream ()
 {
        local key value
-       local svn_remote=() svn_url_pattern count n
+       local svn_remote svn_url_pattern count n
        local upstream=git legacy="" verbose=""
+       svn_remote=()

        # get some config options from git-config
        while read key value; do
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix git-completion.bash for use in zsh
  2011-09-01 13:47 [PATCH] Fix git-completion.bash for use in zsh Alex Merry
@ 2012-03-20 11:53 ` Alex Merry
  2012-03-20 16:23   ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: Alex Merry @ 2012-03-20 11:53 UTC (permalink / raw)
  To: gitster, git

This is just a reminder message, since I haven't heard anything back on 
this, and it hasn't made its way into the repo.

I realise Felipe Contreras has been pushing a different approach to 
making it work properly in zsh, but this should be a fairly innocuous 
fix in the meantime.

Alex

On 01/09/11 14:47, Alex Merry wrote:
> Certain versions (or option combinations) of zsh appear to treat
> things like
> local some_var=()
> as a function declaration.  This makes errors appear when using it in
> combination with the GIT_PS1_SHOWUPSTREAM option.
>
> Signed-off-by: Alex Merry<dev@randomguy3.me.uk>
> ---
>   contrib/completion/git-completion.bash |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 5a83090..89de45d 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -106,8 +106,9 @@ __gitdir ()
>   __git_ps1_show_upstream ()
>   {
>          local key value
> -       local svn_remote=() svn_url_pattern count n
> +       local svn_remote svn_url_pattern count n
>          local upstream=git legacy="" verbose=""
> +       svn_remote=()
>
>          # get some config options from git-config
>          while read key value; do

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix git-completion.bash for use in zsh
  2012-03-20 11:53 ` Alex Merry
@ 2012-03-20 16:23   ` Junio C Hamano
  2012-03-20 18:20     ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2012-03-20 16:23 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Alex Merry

Alex Merry <kde@randomguy3.me.uk> writes:

> This is just a reminder message, since I haven't heard anything back
> on this, and it hasn't made its way into the repo.
> I realise Felipe Contreras has been pushing a different approach to
> making it work properly in zsh, but this should be a fairly innocuous
> fix in the meantime.

Thanks.

"git grep 'local .*=('" tells me that there is only one such construct;
there is another hit but it is not an empty array "=()" but that assigns
"=($(some command output))" so presumably zsh does not mistake it for a
declaration.

Felipe, any comments?
>
> Alex
>
> On 01/09/11 14:47, Alex Merry wrote:
>> Certain versions (or option combinations) of zsh appear to treat
>> things like
>> local some_var=()
>> as a function declaration.  This makes errors appear when using it in
>> combination with the GIT_PS1_SHOWUPSTREAM option.
>>
>> Signed-off-by: Alex Merry<dev@randomguy3.me.uk>
>> ---
>>   contrib/completion/git-completion.bash |    3 ++-
>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 5a83090..89de45d 100755
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -106,8 +106,9 @@ __gitdir ()
>>   __git_ps1_show_upstream ()
>>   {
>>          local key value
>> -       local svn_remote=() svn_url_pattern count n
>> +       local svn_remote svn_url_pattern count n
>>          local upstream=git legacy="" verbose=""
>> +       svn_remote=()
>>
>>          # get some config options from git-config
>>          while read key value; do

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix git-completion.bash for use in zsh
  2012-03-20 16:23   ` Junio C Hamano
@ 2012-03-20 18:20     ` Junio C Hamano
  0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2012-03-20 18:20 UTC (permalink / raw)
  To: Alex Merry; +Cc: Felipe Contreras, git

> Alex Merry <kde@randomguy3.me.uk> writes:
> ...
>> On 01/09/11 14:47, Alex Merry wrote:
>>> Certain versions (or option combinations) of zsh appear to treat
>>> things like
>>> local some_var=()
>>> as a function declaration.  This makes errors appear when using it in
>>> combination with the GIT_PS1_SHOWUPSTREAM option.

The change in this patch itself may be innocuous, but we would point at
the log message of the commit this patch would create to the next person
who would write "local var=()" in contrib/completion/git-completion.bash
file and tell him to fix his patch.

And for that purpose, the above log message is way too vague.  It is
unclear how the problem can be reproduced ("Certain versions"??? "option
combinations"???  Even if you do not exhaustively enumerate the condition,
you at least should be able to say which *one* version you observed the
problem with, and with what option combinations).  It does not even say
what symptom to expect ("errors appear"???  What error message would one
get?).  We would need a more useful explanation to help people who make
further changes to the code.

Also, the original patch was whitespace damaged IIRC.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-03-20 18:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-01 13:47 [PATCH] Fix git-completion.bash for use in zsh Alex Merry
2012-03-20 11:53 ` Alex Merry
2012-03-20 16:23   ` Junio C Hamano
2012-03-20 18:20     ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).