threads / patch / 39033

patchfix global bash variable on __gitcompappend

Subject: [PATCH] fix global bash variable on __gitcompappend

## tl;dr

6 messages between Apr 8, 2015 and Apr 9, 2015. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Márcio Almada· Apr 8, 2015, 05:45 UTC · lore

Hi,

This is how to reproduce the bug:

``` git checkout [tab-tab] echo $x # outputs the name of the last branch on completion list ```

Or more directly:

``` __gitcompappend "something" echo $x # outputs 'something' ```

Might not be a big deal, but it's annoying to know that `$x` is lurking.

Márcio Almada (1):
  fix global bash variable leak on __gitcompappend

 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.9.3

Márcio Almada· Apr 8, 2015, 05:45 UTC · re: Márcio Almada · lore

[PATCH] fix global bash variable leak on __gitcompappend

---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to contrib/completion/git-completion.bash +1 −1

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 661a829..1620546 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -186,7 +186,7 @@ fi
 
 __gitcompappend ()
 {
-	local i=${#COMPREPLY[@]}
+	local x i=${#COMPREPLY[@]}
 	for x in $1; do
 		if [[ "$x" == "$3"* ]]; then
 			COMPREPLY[i++]="$2$x$4"

-- 
1.9.3

Junio C Hamano· Apr 9, 2015, 03:56 UTC · re: Márcio Almada · lore

Re: [PATCH] fix global bash variable leak on __gitcompappend

"Márcio Almada" <marcio.web2@gmail.com> writes:

> ---
>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Nicely found and corrected. Please sign-off your patch and Cc area experts if you can find them (I'll do that this time for you).

Thanks.

Show 14 quoted lines

>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 661a829..1620546 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -186,7 +186,7 @@ fi
>  
>  __gitcompappend ()
>  {
> -	local i=${#COMPREPLY[@]}
> +	local x i=${#COMPREPLY[@]}
>  	for x in $1; do
>  		if [[ "$x" == "$3"* ]]; then
>  			COMPREPLY[i++]="$2$x$4"

SZEDER Gábor· Apr 9, 2015, 08:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] fix global bash variable leak on __gitcompappend

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

Show 26 quoted lines

> "Márcio Almada" <marcio.web2@gmail.com> writes:
>
>> ---
>>  contrib/completion/git-completion.bash | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Nicely found and corrected.  Please sign-off your patch and Cc area
> experts if you can find them (I'll do that this time for you).
>
> Thanks.
>
>>
>> diff --git a/contrib/completion/git-completion.bash  
>> b/contrib/completion/git-completion.bash
>> index 661a829..1620546 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -186,7 +186,7 @@ fi
>>
>>  __gitcompappend ()
>>  {
>> -	local i=${#COMPREPLY[@]}
>> +	local x i=${#COMPREPLY[@]}
>>  	for x in $1; do
>>  		if [[ "$x" == "$3"* ]]; then
>>  			COMPREPLY[i++]="$2$x$4"

Looks good and obviously correct, imho even at -rc2 time.

Not sure whether it's maint-worthy, but just in case: it was introduced in v1.8.3-rc0~59^2~2.

Marcio Almada· Apr 9, 2015, 13:52 UTC · re: Junio C Hamano · lore

Re: [PATCH] fix global bash variable leak on __gitcompappend

2015-04-09 0:56 GMT-03:00 Junio C Hamano <gitster@pobox.com>:

Show 11 quoted lines

> "Márcio Almada" <marcio.web2@gmail.com> writes:
>
>> ---
>>  contrib/completion/git-completion.bash | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Nicely found and corrected.  Please sign-off your patch and Cc area
> experts if you can find them (I'll do that this time for you).
>
> Thanks.
>

Ok, I'll remember this next time. Thanks for your incredible work here.

Show 14 quoted lines

>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 661a829..1620546 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -186,7 +186,7 @@ fi
>>
>>  __gitcompappend ()
>>  {
>> -     local i=${#COMPREPLY[@]}
>> +     local x i=${#COMPREPLY[@]}
>>       for x in $1; do
>>               if [[ "$x" == "$3"* ]]; then
>>                       COMPREPLY[i++]="$2$x$4"

Eric Sunshine· Apr 9, 2015, 19:26 UTC · re: Marcio Almada · lore

Re: [PATCH] fix global bash variable leak on __gitcompappend

On Thu, Apr 9, 2015 at 9:52 AM, Marcio Almada <marcio.web2@gmail.com> wrote:

Show 9 quoted lines

> 2015-04-09 0:56 GMT-03:00 Junio C Hamano <gitster@pobox.com>:
>> "Márcio Almada" <marcio.web2@gmail.com> writes:
>>>  contrib/completion/git-completion.bash | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Nicely found and corrected.  Please sign-off your patch and Cc area
>> experts if you can find them (I'll do that this time for you).
>
> Ok, I'll remember this next time. Thanks for your incredible work here.

Junio's implication was that you should reply to his message with:

    Signed-off-by: Your Name <your@email>

Or re-send the patch with your sign-off added.