Although I don't think anyone sane would ever do that, but I happened to create one on in my local repo, and, curiously, zsh git completion broke. 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 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. 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. --- Completion/Unix/Command/_git | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git index ba1852699..97ab26512 100644 --- a/Completion/Unix/Command/_git +++ b/Completion/Unix/Command/_git @@ -6662,8 +6662,8 @@ __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. - descriptions=( ${(0)"$(_call_program all-descriptions "git --no-pager for-each-ref --format='%(refname)%00%(subject)%00'" refs/heads/${(q)^branches} "--")"//$'\n'} ) + local z=$'\0' + descriptions=( "${(0)"$(_call_program all-descriptions "git --no-pager for-each-ref --format='%(refname)%00%(subject)'" refs/heads/${(q)^branches} "--")"//$'\n'/$z}" ) # 5. Synthesize the data structure _describe wants. local -a branches_colon_descriptions -- 2.24.1
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
On 2/1/20 3:09 AM, Daniel Shahaf wrote: > 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). Oh, damn, despite all my efforts, Thunderbird still messed the patch up. Sigh. (gotta finally find some sane mail client) Anyway, copypasting it from the mailing list archive results in something that applies cleanly: https://www.zsh.org/mla/workers/2020/msg00190.html > 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: That isn't supposed to happen. This should merely assign a string contained a single null byte to variable z. Maybe you didn't clean up all those NBSP? > > +__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: Likewise, I have no clue what it is.
WGH wrote on Sun, 02 Feb 2020 02:50 +0300: > On 2/1/20 3:09 AM, Daniel Shahaf wrote: > > 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). > > Oh, damn, despite all my efforts, Thunderbird still messed the patch up. Sigh. (gotta finally find some sane mail client) > Thanks for your efforts. For Thunderbird there's https://wiki.openstack.org/wiki/MailingListEtiquette#Thunderbird , but I don't think it affects you since your email wasn't format=flowed. Usually, I send patches through one of git-send-email(1), Mutt, and Claws. None of them munge whitespace (some of them had to be configured appropriately first). > Anyway, copypasting it from the mailing list archive results in something that applies cleanly: https://www.zsh.org/mla/workers/2020/msg00190.html If you look at the HTML source of that page you'll find it has the non-breaking spaces still. Firefox, at least, translates NBSP to ordinary space upon copying, so that might explain why copy-pasting that way worked. The problem is that copying text this way would also obliterate any non-breaking spaces that were added intentionally. > > 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: > > That isn't supposed to happen. This should merely assign a string contained a single null byte to variable z. Maybe you didn't clean up all those NBSP? Yes, sounds plausible. If the first column was a non-breaking space, that would cause the observed error message. > > > > +__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: > > Likewise, I have no clue what it is. That'd be the same issue. The word «descriptions=(…)» was taken as an ordinary positional argument followed by glob qualifiers to the « » (NBSP) command. The character «=» is not special when not in a pre-command assignment; it's just an ordinary filename character. I've applied your patch to master with a grammar tweak; it'll be in 5.8. Thanks again! Daniel