[PATCH] completion: git: fix __git_commit_objects/__git_recent_commits

[PATCH] completion: git: fix __git_commit_objects/__git_recent_commits

[Daniel Hahler]

From: Daniel Hahler <git@thequod.de>

$pipestatus for `: foo` appears to be 0 always.
Explicitly declare `$commits` as associative array, and assign it
normally.

Without this, "git checkout" in a non-git directory would complete " ",
but not result in a note/error about not being in a git dir.
---
 Completion/Unix/Command/_git | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index ed23b39..da852ef 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -5649,11 +5649,11 @@ __git_heads_remote () {
 (( $+functions[__git_commit_objects] )) ||
 __git_commit_objects () {
   local gitdir expl start
-  declare -a commits
+  declare -A commits
 
   # Note: the after-the-colon part must be unique across the entire array;
   # see workers/34768
-  : ${(A)commits::=${(f)"$(_call_program commits git --no-pager log -20 --format='%h:\\\[%h\\\]\ %s')"}}
+  commits=(${(f)"$(_call_program commits git --no-pager log -20 --format='%h:\\\[%h\\\]\ %s')"})
   __git_command_successful $pipestatus || return 1
 
   _describe -V -t commits 'commit object name' commits || _guard '[[:xdigit:]](#c,40)' 'commit object name'
@@ -5662,11 +5662,12 @@ __git_commit_objects () {
 (( $+functions[__git_recent_commits] )) ||
 __git_recent_commits () {
   local gitdir expl start
-  declare -a descr tags heads commits
+  declare -a descr tags heads
+  declare -A commits
   local i j k
 
   # Careful: most %d will expand to the empty string.  Quote properly!
-  : "${(A)commits::=${(@f)"$(_call_program commits git --no-pager log -20 --format='%h%n%d%n%s')"}}"
+  commits=(${(@f)"$(_call_program commits git --no-pager log -20 --format='%h%n%d%n%s')"})
   __git_command_successful $pipestatus || return 1
 
   for i j k in "$commits[@]" ; do
-- 
2.4.0.dirty

Re: [PATCH] completion: git: fix __git_commit_objects/__git_recent_commits

[Daniel Hahler]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I will push this later with the "return $ret" mentioned by Bart in 35162.


On 17.05.2015 19:37, Daniel Hahler wrote:
> From: Daniel Hahler <git@thequod.de>
> 
> $pipestatus for `: foo` appears to be 0 always.
> Explicitly declare `$commits` as associative array, and assign it
> normally.
> 
> Without this, "git checkout" in a non-git directory would complete " ",
> but not result in a note/error about not being in a git dir.
> ---
>  Completion/Unix/Command/_git | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
> index ed23b39..da852ef 100644
> --- a/Completion/Unix/Command/_git
> +++ b/Completion/Unix/Command/_git
> @@ -5649,11 +5649,11 @@ __git_heads_remote () {
>  (( $+functions[__git_commit_objects] )) ||
>  __git_commit_objects () {
>    local gitdir expl start
> -  declare -a commits
> +  declare -A commits
>  
>    # Note: the after-the-colon part must be unique across the entire array;
>    # see workers/34768
> -  : ${(A)commits::=${(f)"$(_call_program commits git --no-pager log -20 --format='%h:\\\[%h\\\]\ %s')"}}
> +  commits=(${(f)"$(_call_program commits git --no-pager log -20 --format='%h:\\\[%h\\\]\ %s')"})
>    __git_command_successful $pipestatus || return 1
>  
>    _describe -V -t commits 'commit object name' commits || _guard '[[:xdigit:]](#c,40)' 'commit object name'
> @@ -5662,11 +5662,12 @@ __git_commit_objects () {
>  (( $+functions[__git_recent_commits] )) ||
>  __git_recent_commits () {
>    local gitdir expl start
> -  declare -a descr tags heads commits
> +  declare -a descr tags heads
> +  declare -A commits
>    local i j k
>  
>    # Careful: most %d will expand to the empty string.  Quote properly!
> -  : "${(A)commits::=${(@f)"$(_call_program commits git --no-pager log -20 --format='%h%n%d%n%s')"}}"
> +  commits=(${(@f)"$(_call_program commits git --no-pager log -20 --format='%h%n%d%n%s')"})
>    __git_command_successful $pipestatus || return 1
>  
>    for i j k in "$commits[@]" ; do
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iD8DBQFVWNZyfAK/hT/mPgARAjzOAKDy/2q75fIHMrD3b0wLXqjNyKBxawCePJR4
yHupBK2rR7GLl/7jzctuN9w=
=74Ni
-----END PGP SIGNATURE-----

Re: [PATCH] completion: git: fix __git_commit_objects/__git_recent_commits

[Daniel Shahaf]

Daniel Hahler wrote on Sun, May 17, 2015 at 19:37:53 +0200:
> From: Daniel Hahler <git@thequod.de>
> 
> $pipestatus for `: foo` appears to be 0 always.
> Explicitly declare `$commits` as associative array, and assign it
> normally.

The change from sequential to associative array breaks __git_recent_commits.
I assume you should revert the declaration change but keep the
initialization change.

Incidentally, _describe's documentation says colons should be escaped,
so I think we also need something like this:

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 918f6be..a281597 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -5672,7 +5672,7 @@ __git_recent_commits () {
   for i j k in "$commits[@]" ; do
     # Note: the after-the-colon part must be unique across the entire array;
     # see workers/34768
-    descr+=("$i:[$i] $k")
+    descr+=("$i:[$i] ${k//:/\\:}")
     j=${${j# \(}%\)} # strip leading ' (' and trailing ')'
     for j in ${(s:, :)j}; do
       if [[ $j == 'tag: '* ]] ; then

But I haven't tested this, and in any case I won't commit this until you
finish pushing your outstanding changes (to avoid conflicts).

Daniel

> Without this, "git checkout" in a non-git directory would complete " ",
> but not result in a note/error about not being in a git dir.

Re: [PATCH] completion: git: fix __git_commit_objects/__git_recent_commits

[Daniel Hahler]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 18.05.2015 01:46, Daniel Shahaf wrote:
> Daniel Hahler wrote on Sun, May 17, 2015 at 19:37:53 +0200:
>> From: Daniel Hahler <git@thequod.de>
>>
>> $pipestatus for `: foo` appears to be 0 always.
>> Explicitly declare `$commits` as associative array, and assign it
>> normally.
> 
> The change from sequential to associative array breaks __git_recent_commits.
> I assume you should revert the declaration change but keep the
> initialization change.

Thanks for catching that, pushed as suggested.

> Incidentally, _describe's documentation says colons should be escaped,
> so I think we also need something like this:
> 
> diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
> index 918f6be..a281597 100644
> --- a/Completion/Unix/Command/_git
> +++ b/Completion/Unix/Command/_git
> @@ -5672,7 +5672,7 @@ __git_recent_commits () {
>    for i j k in "$commits[@]" ; do
>      # Note: the after-the-colon part must be unique across the entire array;
>      # see workers/34768
> -    descr+=("$i:[$i] $k")
> +    descr+=("$i:[$i] ${k//:/\\:}")
>      j=${${j# \(}%\)} # strip leading ' (' and trailing ')'
>      for j in ${(s:, :)j}; do
>        if [[ $j == 'tag: '* ]] ; then
> 
> But I haven't tested this, and in any case I won't commit this until you
> finish pushing your outstanding changes (to avoid conflicts).

Should be good to get committed now - in case it works.


Regards,
Daniel.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iD8DBQFVWrIAfAK/hT/mPgARAnriAKDi3sptg2KRSZVxpNqjtrLsO9skUQCg0ToS
NU8Sc42Gu0K+Xqy5FOkH6Ow=
=dtto
-----END PGP SIGNATURE-----

Re: [PATCH] completion: git: fix __git_commit_objects/__git_recent_commits

[Bart Schaefer]

On May 19,  5:46am, Daniel Hahler wrote:
}
} >      # Note: the after-the-colon part must be unique across the entire array;
} >      # see workers/34768
} > -    descr+=("$i:[$i] $k")
} > +    descr+=("$i:[$i] ${k//:/\\:}")
} 
} Should be good to get committed now - in case it works.

It's not harmful, but it doesn't seem to be necessary either -- the only
colons that need escaping are any that may appear in $i not those in $k.