bug in _git: (was Re: _git:6147: math recursion limit exceeded )

bug in _git: (was Re: _git:6147: math recursion limit exceeded )

[Michal Maruska]

It turned out, that
_git_third_party_commands+=$name$desc
should be
_git_third_party_commands+=($name $desc)


On Mon, Feb 25, 2013 at 10:31 AM, Michal Maruska <mmaruska@gmail.com> wrote:
> Hello,
> I have problem with  completion for: git  <TAB>.
>
> I have some  "third party" git commands with provided zsh completion.
>
> They are named _git-segment  _git-segment-name  etc.
>
> I see this relevant code in _git:
>
> line 6139:
> declare -gUa _git_third_party_commands
>
> I would have expected  -A associative array.
>
> then:
>
> local name=${${file:t}#_git-}
> if (( $+_git_third_party_commands[$name] )); then
> ....
>
> so my question is why  -- $name being   "segment-name" --,
> this  [$name] leads to recursion? How to avoid (& why is that needed)
> xxxx-name being interpreted as arithmetic (which obviously does lead
> to recursion on $name)?
>
>
> Thank you

Re: bug in _git: (was Re: _git:6147: math recursion limit exceeded )

[Mikael Magnusson]

On 25 February 2013 11:02, Michal Maruska <mmaruska@gmail.com> wrote:
> It turned out, that
> _git_third_party_commands+=$name$desc
> should be
> _git_third_party_commands+=($name $desc)

Probably something like this patch should be applied, I don't have
time to test these changes though (and only the one you found is
actually a bug, foo+=bar is okay for a regular array).

http://cgit.mika.l3ib.org/cgit/zsh-cvs/patch/?id=dd66e37e821eef41177af432b08fb76544ff345b

>From 398a9deb2fa4e32354d406f3b1719615c5461143 Mon Sep 17 00:00:00 2001
From: Mikael Magnusson <mikachu@gmail.com>
Date: Mon, 25 Feb 2013 11:51:27 +0100
Subject: PATCH: _git: Use () for all array append operations.

One of these, _git_third_party_commands+=$name$desc, was definitely a bug.
---
 Completion/Unix/Command/_git | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 2b6a369..d019df9 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -297,9 +297,9 @@ _git-branch () {
     dependent_deletion_args=(
       '-r[delete only remote-tracking branches]')
     if (( words[(I)-r] )); then
-      dependent_deletion_args+='*:
:__git_ignore_line_inside_arguments __git_remote_branch_names'
+      dependent_deletion_args+=( '*:
:__git_ignore_line_inside_arguments __git_remote_branch_names' )
     else
-      dependent_deletion_args+='*:
:__git_ignore_line_inside_arguments __git_branch_names'
+      dependent_deletion_args+=( '*:
:__git_ignore_line_inside_arguments __git_branch_names' )
     fi
   fi

@@ -2081,9 +2081,9 @@ _git-config () {
         declare -a match mbegin mend
         for opt in
${${${${(M)git_options_static:#(#i)${IPREFIX}[^.:]##:*}#(#i)${IPREFIX}}/#(#b)([^:]##:)([^\\:]#(\\?[^\\:]#)#:[^\\:]#(\\?[^\\:]#)#:->bool)/$match[1]whether
or not to $match[2]}/#(#b)([^:]##:([^\\:]#(\\?[^\\:]#)#))*/$match[1]};
do
           if (( ${git_options_static[(I)${opt%%:*}.*]} )); then
-            sections_and_options+=$opt
+            sections_and_options+=( $opt )
           else
-            options+=$opt
+            options+=( $opt )
           fi
         done

@@ -3677,9 +3677,9 @@ _git-merge-index () {
   else
     declare -a arguments

-    (( CURRENT == 2 )) && arguments+='-o[skip failed merges]'
-    (( CURRENT == 2 || CURRENT == 3 )) && arguments+='(-o)-q[do not
complain about failed merges]'
-    (( 2 <= CURRENT && CURRENT <= 4 )) && arguments+='*:merge
program:_files -g "*(*)"'
+    (( CURRENT == 2 )) && arguments+=( '-o[skip failed merges]' )
+    (( CURRENT == 2 || CURRENT == 3 )) && arguments+=( '(-o)-q[do not
complain about failed merges]' )
+    (( 2 <= CURRENT && CURRENT <= 4 )) && arguments+=( '*:merge
program:_files -g "*(*)"' )

     _arguments -S $arguments
   fi
@@ -4706,7 +4706,7 @@ _git_commands () {
   local -a third_party_commands
   local command
   for command in $_git_third_party_commands; do
-    (( $+commands[git-${command%%:*}] )) && third_party_commands+=$command
+    (( $+commands[git-${command%%:*}] )) && third_party_commands+=( $command )
   done

   local -a aliases unique_aliases
@@ -4724,7 +4724,7 @@ _git_commands () {
        plumbing_interrogator_commands[(I)$name:*] ||
        plumbing_sync_commands[(I)$name:*] ||
        plumbing_sync_helper_commands[(I)$name:*] ||
-       plumbing_internal_helper_commands[(I)$name:*] )) ||
unique_aliases+=$alias
+       plumbing_internal_helper_commands[(I)$name:*] )) ||
unique_aliases+=( $alias )
   done

   integer ret=1
@@ -5089,7 +5089,7 @@ __git_heads () {
   gitdir=$(_call_program gitdir git rev-parse --git-dir 2>/dev/null)
   if __git_command_successful $pipestatus; then
     for f in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
-      [[ -f $gitdir/$f ]] && heads+=$f
+      [[ -f $gitdir/$f ]] && heads+=( $f )
     done
   fi

@@ -5316,7 +5316,7 @@ __git_files_relative () {
       fi
     done

-    files+=${(l@${#prefix[i,-1]//[^\/]}*3@@../@)}${file[i,-1]}
+    files+=( ${(l@${#prefix[i,-1]//[^\/]}*3@@../@)}${file[i,-1]} )
   done

   print ${(pj:\0:)files}
@@ -6158,7 +6158,7 @@ for file in ${^fpath}/_git-*~(*~|*.zwc)(.N); do
     (( i++ ))
   done < $file

-  _git_third_party_commands+=$name$desc
+  _git_third_party_commands+=( $name $desc )
 done

 _git
-- 
1.7.10.GIT



-- 
Mikael Magnusson

Re: bug in _git: (was Re: _git:6147: math recursion limit exceeded )

[Frank Terbeck]

Hi there,

Michal Maruska wrote:
> I have some  "third party" git commands with provided zsh completion.
>
> They are named _git-segment  _git-segment-name  etc.
>
> I see this relevant code in _git:
>
> line 6139:
> declare -gUa _git_third_party_commands
>
> I would have expected  -A associative array.
>
> then:
>
> local name=${${file:t}#_git-}
> if (( $+_git_third_party_commands[$name] )); then
> ....

No, the original code was not written with assoc. arrays in mind.

The problem however is that arithmetic expansion is in implicitly in
place with subscripts of ordinary arrays, which is not done with assoc.
arrays - which might have led you to the conclusion, that those kind of
arrays were supposed to be in use there.

In reality, the array is formed for direct consumption by _describe.


Michal Maruska wrote:
> It turned out, that
> _git_third_party_commands+=$name$desc
> should be
> _git_third_party_commands+=($name $desc)

No, not really.

Turns out the way the look-up is done was changed in one of Nikolai's
rewrites. The way it was done before avoids the arithmetic expression
and thus fixes the problem. Here's a patch:


diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 2b6a369..cf9b498 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -6142,7 +6142,7 @@ _git_third_party_commands=()
 local file
 for file in ${^fpath}/_git-*~(*~|*.zwc)(.N); do
   local name=${${file:t}#_git-}
-  if (( $+_git_third_party_commands[$name] )); then
+  if [[ -n ${(M)_git_third_party:#${name}*} ]]; then
     continue
   fi