Re: [PATCH] _git: Fix __git_recent_branches for case when commit has empty message

[Daniel Shahaf <d.s@daniel.shahaf.name>]

WGH wrote on Fri, 31 Jan 2020 19:57 +0300:
> Pressing "Tab" after entering "git checkout " will cause the completion
> script to crash with the following error:
> 
>     __git_recent_branches:21: bad set of key/value pairs for associative array

Thanks, I can reproduce this.

> This error happens when an associative array is assigned odd number of
> elements. Unless the variable being split is inside double quotes, empty
> elements are removed (which is what causes the problem).
> 
> This commit fixes it.

Not on my machine.

First, the unidiff was improperly serialized: the spaces at the start
of each context line were converted to non-breaking spaces (U+00A0).
I applied the patch manually, and completion did work, but stderr was
spammed with two warnings:

+__git_recent_branches:21>   local $'z=\C-@'
__git_recent_branches:21: command not found:  
+__git_recent_branches:22> _call_program all-descriptions 'git --no-pager for-each-ref --format='\''%(refname)%00%(subject)'\' refs/heads/master --                                                                
__git_recent_branches:22: unknown file attribute:

I'm not sure why that happened.  I assume "unknown file attribute"
means parentheses were taken for glob qualifiers, but I don't see why
the patch would have that effect.  I was testing in an empty
repository, produced by «cd "$(mktemp -d)" && git init && git commit
--allow-empty{,-message} && git checkout -b foo».  Does anyone see what caused
the warnings?

As to the bug, I found a workaround:

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index ba1852699..7fccc11b0 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -6663,13 +6663,14 @@ __git_recent_branches() {
   # 4. Obtain log messages for all of them in one shot.
   # TODO: we'd really like --sort=none here...  but git doesn't support such a thing.
   # The \n removal is because for-each-ref prints a \n after each entry.
+  # The x is added in case %(subject)'s expansion is the empty string.
-  descriptions=( ${(0)"$(_call_program all-descriptions "git --no-pager for-each-ref --format='%(refname)%00%(subject)%00'" refs/heads/${(q)^branches} "--")"//$'\n'} )
+  descriptions=( ${(0)"$(_call_program all-descriptions "git --no-pager for-each-ref --format='%(refname)%00x%(subject)%00'" refs/heads/${(q)^branches} "--")"//$'\n'} )
 
   # 5. Synthesize the data structure _describe wants.
   local -a branches_colon_descriptions
   local branch
   for branch in ${branches} ; do
-    branches_colon_descriptions+="${branch//:/\:}:${descriptions[refs/heads/${(b)branch}]}"
+    branches_colon_descriptions+="${branch//:/\:}:${descriptions[refs/heads/${(b)branch}]#x}"
   done
 
   _describe -V -t recent-branches "recent branches" branches_colon_descriptions

> Since --format=%(refname)%00%(subject)%00 appends null byte to every line,
> using this format would cause an extra empty element appear, breaking the
> assigment again. In order to fix this, I removed the trailing null byte
> from the format string, and replaced newlines separating entries with
> null byte (they were previously removed). Since neither refname nor
> subject appears to be capable of containing newlines, this change should
> be safe.

Agreed.

Thanks for the bug report and the patch.  I guess I'll commit my
version, since it fixes the bug, but I do want to understand why your
patch didn't work for me.

Cheers,

Daniel