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

[Jonathan Nieder <jrnieder@gmail.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).

> --- 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?

> @@ -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>

?

> @@ -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?

> @@ -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.

> @@ -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.

> @@ -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.

> @@ -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.

> @@ -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?