Re: [PATCH v4 2/2] Use the new functions to get the current cword.

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 2, 2010, 07:45 UTC
Message-ID: <20101202074504.GA1771@burratino>
In-Reply-To: <1291236582-28603-3-git-send-email-peter@avirtualhome.com>

Hi Peter,

Peter van der Does wrote:

> Change the completion functions to use the newly introduced functions to
> get the current and/or previous cword and to reassemble the COMP_CWORDS,
> making sure the options are correctly split.

Some comments. Please don't reroll until discussion has quieted down (though thoughts and incremental patches would always be welcome, of course).

Show 9 quoted lines

> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -554,7 +554,8 @@ __gitcomp_1 ()
>  # generates completion reply with compgen
>  __gitcomp ()
>  {
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	local cur
> +	_get_comp_words_by_ref -n "=" cur

To save the reader some time: this excludes '=' from word-breaking characters, so $cur will include an = when appropriate. IIUC that is precisely the behavior that bash 4 changed.

Perhaps that is worth explaining in the commit message in the next round?

Show 7 quoted lines

> @@ -615,7 +616,8 @@ __git_tags ()
>  __git_refs ()
>  {
>  	local i is_hash=y dir="$(__gitdir "${1-}")" track="${2-}"
> -	local cur="${COMP_WORDS[COMP_CWORD]}" format refs
> +	local cur format refs
> +	_get_comp_words_by_ref cur

This does not exclude '=' from word-breaking characters. Would that break completion of

	git update-ref refs/topics/foo=bar HEAD
	git checkout refs/topics/foo=<tab><tab>

Show 7 quoted lines

> @@ -729,7 +731,8 @@ __git_compute_merge_strategies ()
>  
>  __git_complete_file ()
>  {
> -	local pfx ls ref cur="${COMP_WORDS[COMP_CWORD]}"
> +	local pfx ls ref cur
> +	_get_comp_words_by_ref -n ":" cur

This treats '=' as a word-breaking character but not ':'. Is that the right behavior?

Show 7 quoted lines

> @@ -777,7 +780,8 @@ __git_complete_file ()
>  
>  __git_complete_revlist ()
>  {
> -	local pfx cur="${COMP_WORDS[COMP_CWORD]}"
> +	local pfx cur
> +	_get_comp_words_by_ref cur

'=' and ':' are word-breakers.

Show 9 quoted lines

> @@ -797,11 +801,13 @@ __git_complete_revlist ()
>  
>  __git_complete_remote_or_refspec ()
>  {
> -	local cmd="${COMP_WORDS[1]}"
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	local cur words cword
> +	_get_comp_words_by_ref -n ":" cur words cword
> +	local cmd="${words[1]}"

'=' is a word-breaker, ':' not.

Show 6 quoted lines

> @@ -869,13 +875,15 @@ __git_complete_remote_or_refspec ()
>  
>  __git_complete_strategy ()
>  {
> +	local cur prev
> +	_get_comp_words_by_ref -n "=" cur prev

'=' is not a wordbreaker, so --strategy= can be completed correctly.

Show 10 quoted lines

> @@ -1048,10 +1056,11 @@ __git_aliased_command ()
>  # __git_find_on_cmdline requires 1 argument
>  __git_find_on_cmdline ()
>  {
> -	local word subcommand c=1
> +	local word subcommand c=1 words cword
>  
> -	while [ $c -lt $COMP_CWORD ]; do
> -		word="${COMP_WORDS[c]}"
> +	_get_comp_words_by_ref words cword

':' and '=' are word-breakers when completing subcommand names.

Show 8 quoted lines

> @@ -1064,9 +1073,10 @@ __git_find_on_cmdline ()
>  
>  __git_has_doubledash ()
>  {
> -	local c=1
> -	while [ $c -lt $COMP_CWORD ]; do
> -		if [ "--" = "${COMP_WORDS[c]}" ]; then
> +	local c=1, words cword

Extra comma.

> +	_get_comp_words_by_ref words cword

':' and '=' are word-breakers when looking for "--".

[etc]

So in general, it seems that : and = are treated as word-breakers after this change much more often than git itself would treat them as such. Is that intentional? What rule is used to choose -n arguments?