patch, 3 partscompletion: update am, commit, and log

Since commit 518ef8f (completion: Replace config --list with --get-regexp, 2009-09-11) an alias config value containing a newline would break the completion of aliases. Instead of setting the IFS to the newline, use git-config's null termination to separate aliases with the null character.

An example .gitconfig causing the breakage.

[alias]
	whowhat = "log -1 --pretty='format:%an <%ae>\n%s'"
	wont-complete = ...

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

This is the best I can come up with. I'm not sure about using the d option of read though.

 contrib/completion/git-completion.bash |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

git am learned --scissors, git commit learned --dry-run and git log learned --decorate=long|short recently.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 contrib/completion/git-completion.bash |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

I sent this a while back, no response. If no response again I'll drop.

 contrib/completion/git-completion.bash |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Stephen Boyd <bebarino@gmail.com> writes:

No objection from me, but I am not a completion expert ;-)

Stephen Boyd <bebarino@gmail.com> writes:

No objection from me, but I am not a completion expert ;-)  

Stephen Boyd schrieb:

Is it necessary to change the body of the loop? Your version spawns two processes on each iteration, while the original spawned no processes.

You can avoid the pipeline (i.e. yet another process) using a "here-string":

	local i aliases=$(git --git-dir="$(__gitdir)" config -z \
				--get-regexp "alias\..*" 2>/dev/null)
	while IFS= read -rd '' i; do
		i="${i#alias.}"
		echo "${i/ */}"	# could be: echo "${i%% *}"
  	done <<< "$aliases"

but I don't know how well bash handles variable values with embedded NULs.

-- Hannes

Johannes Sixt wrote:

I can't get the above snippet to work, but maybe I'm doing something wrong.

I had a problem with the newline between the key and the value. bash gives me the whole line in the loop, but when I try to trim it $i is treated as two values. I couldn't figure out any other way to do it, besides piping $i to cut or tr.

Maybe a better solution would be to add --keys-only or something to git-config?

Stephen Boyd <bebarino@gmail.com> wrote:

Yes, we should try to avoid spawning a process here, it fires on each completion attempt if I recall.

Or a format string with a language based escape like for-each-ref supports? Might make it easier to use git config in scripts if we can write things like:

  git config --format='$data{%(key)}=%(value);' --perl

Shawn O. Pearce wrote:

Ok. It looks like it fires on every completion with no command (i.e. git <TAB><TAB>). I don't particularly like my solution anyways, and this has existed for a long time (56fc25f maybe?) with no one noticing. A fix for it is probably not too pressing.

I found another problem, [ARGS] and COMMAND show up in git <TAB><TAB>. Looks like it's due to the wrapping of git's usage string.

This sounds reasonable, but also like more work ;-)

Stephen Boyd schrieb:

OTOH, you can simply revert the hunk from 518ef8f (completion: Replace config --list with --get-regexp, 2009-09-11) that applies here, or even only reinstate the case statement in the loop body without changing anything else.

-- Hannes

Johannes Sixt wrote:

I'm not opposed to the idea, but then it wouldn't work with something like

[alias]
    whowhat = "log --format="%an\nalias.broken=foo'"

although that actually happening is probably rare, it can still happen.

Stephen Boyd schrieb:

I'd ignore this case, but *you* write the patch, *you* get to draw the line ;)

-- Hannes

SZEDER Gábor <szeder@ira.uka.de> writes:

A similar example in "git init" completion (--shared vs --shared=) already exists, no?

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

I'm not sure why this is. It may just be historical accident, completing --shared vs. --shared= is ambiguous so you would need to type out the = anyway before trying to complete a value.

IIRC --shared= came about after --shared and its possible we just added the new option and didn't notice the duplicate.

SZEDER Gábor wrote:

I guess this is because nobody has added these. Feel free to send a follow up patch.