Re: [completion] Request: Include remote heads as push targets

["SZEDER Gábor" <szeder@ira.uka.de>]

Hi,


On Thu, Oct 21, 2010 at 09:08:42PM -0400, Peter van der Does wrote:
> On Thu, 21 Oct 2010 14:10:45 -0500
> Jonathan Nieder <jrnieder@gmail.com> wrote:
> 
> > Marc Branchaud wrote:
> > 
> > > Hmmm, perhaps this is really a bug.
> > 
> > Compare:
> > http://thread.gmane.org/gmane.comp.version-control.git/159448

Yeah, it seems that the two issues are related.  I can confirm what
Marc saw, and below is a PoC patch to fix it.

> In the case of Marc's problem, it would be helpful to see what the
> result is in Bash 3.

On an oldish server with bash 3.2 it works as expected, i.e. I get
only the matching branches and tags from the remote repo.

(Sidenote: hm, offering _tags_ to _push to_?!  That doesn't seem quite
right at first sight.)


> > Gábor, would it be possible to summarize the problem with a simple
> > test case that could be used to get help on this from the (upstream
> > or distro-specific) bash maintainers?

Git's bash completion is not the first to suffer from changes in bash
4, and fortunately the bash-completion developers already provide a
solution for such issues.  Details below.

> As for Gábor find:
> The problem resides in Bash 4.

I agree.

> Bash 4 has a new set of characters that
> are defined as break up characters
> Thanks to Brain Gernhard: 
> From the Bash 4.0 changelog:
> i.  The programmable completion code now uses the same set of
> characters as readline when breaking the command line into a list of
> words.
> 
> As far as I can tell, from the Bash 4.0 source, these are the
> characters: " \t\n\"'@><=;|&(:" 

Um, well, I suspect that there are other subtle differences between
bash 4 and 3 besides the change of word-breaking characters that
trigger this breakage.  In fact, the oldish server mentioned above
with bash 3.2 has the exact same characters in $COMP_WORDBREAKS, and
neither Marc's nor my issue occur there.

> In the completion script checks are performed if an option is given.
> The test includes the equal sign but the array with words does not the
> equal sign. Example to clarify:
> 
> local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
> case "$cur" in
>   --whitespace=*)
>       __gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}"
>       return
>       ;;
> 
> If you execute:
> $ git am --whitespace=<tab><tab>
> 
> The variable cur holds the equal sign and so the __gitcomp function is
> never executed.

That's exactly what I observed.  This ${COMP_WORDS[COMP_CWORD]}
construct apparently is not the right way to find the word to complete
anymore, assuming you want your completion script to work with bash 4
and 3 as well.  Unfortunately, we use this construct all over the
place.

Now, the bash completion project has some functions that could be used
to circumvent these issues.  In particular, look at the description of
the _get_comp_words_by_ref() function here, especially at the -n
option:

http://git.debian.org/?p=bash-completion/bash-completion.git;a=blob;f=bash_completion;h=589c2e5afe283d2e6d7628b683ae6714ab70d3d9;hb=HEAD#l371

That would allow us to remove characters from $COMP_WORDBREAKS on a
per-function basis, without influencing unrelated completion functions
within or outside of git completion, and in a way that works with bash
4 and 3 as well.

Here is a proof of concept patch to use that function instead of
${COMP_WORDS[COMP_CWORD]} in two places.  The second hunk fixes the
completion of pretty aliases for 'git log --pretty='.  The first hunk
seems to fix Marc's issue with the completion of remotes after 'git
push origin HEAD:', but I haven't thought this one through (there's a
lot going on with scanning the previous words on the command line and
such, so it might actually break something else).  Both fixes seem to
work under bash 4 and 3.2.

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f83f019..5608e9b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -551,7 +551,8 @@ __git_complete_revlist ()
 __git_complete_remote_or_refspec ()
 {
 	local cmd="${COMP_WORDS[1]}"
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n ':' cur
 	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
 	while [ $c -lt $COMP_CWORD ]; do
 		i="${COMP_WORDS[c]}"
@@ -1360,7 +1361,8 @@ _git_log ()
 {
 	__git_has_doubledash && return
 
-	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local cur
+	_get_comp_words_by_ref -n '=' cur
 	local g="$(git rev-parse --git-dir 2>/dev/null)"
 	local merge=""
 	if [ -f "$g/MERGE_HEAD" ]; then

This patch assumes that you use fairly recent bash-completion, because
_get_comp_words_by_ref() was first included in bash-completion v1.2,
which was released just this summer.

However, git completion is currently a standalone completion script,
i.e. to use it you need only bash, git-completion.bash, and nothing
else.  If we start to use _get_comp_words_by_ref() directly, as in the
PoC patch above, then git completion will inherently depend on
bash-completion, too.  This could be considered as a regression.

Alternatively, we could just copy the necessary functions from
bash-completion to git-completion.bash (with the name changed, of
course, e.g. to __git_get_comp_words_by_ref()), keeping git completion
standalone but still getting the benefits of this function, and
getting these bash 4 vs. 3 issues fixed.

Thoughts?


Best,
Gábor