Re: [PATCH] completion(switch/checkout): treat --track and -t the same

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 8, 2023, 15:51 UTC
Message-ID: <xmqq1qf8vf1e.fsf@gitster.g>
In-Reply-To: <pull.1584.git.1694176123471.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When `git switch --track ` is to be completed, only remote refs are
> eligible because that is what the `--track` option targets.

OK.  Presumably that is the same for "checkout --track".

> And when the short-hand `-t` is used instead, the same _should_ happen.

That sounds sensible. The code change for _git_checkout() and _git_switch() is surprisingly simple ;-) "-t" was not handled any specially at all and fell through to "refs" complation.

> Let's make it so.

Sounds good.

Show 5 quoted lines

> Note that the bug exists both in the completions of `switch` and
> `completion`, even if it manifests in slightly different ways: While
> the completion of `git switch -t ` will not even look at remote refs,
> the completion of `git checkout -t ` will look at both remote _and_
> local refs. Both should look only at remote refs.

Correct.

Show 22 quoted lines

> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 133ec92bfae..745dc901fbe 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1607,7 +1607,7 @@ _git_checkout ()
>  
>  		if [ -n "$(__git_find_on_cmdline "-b -B -d --detach --orphan")" ]; then
>  			__git_complete_refs --mode="refs"
> -		elif [ -n "$(__git_find_on_cmdline "--track")" ]; then
> +		elif [ -n "$(__git_find_on_cmdline "-t --track")" ]; then
>  			__git_complete_refs --mode="remote-heads"
>  		else
>  			__git_complete_refs $dwim_opt --mode="refs"
> @@ -2514,7 +2514,7 @@ _git_switch ()
>  
>  		if [ -n "$(__git_find_on_cmdline "-c -C -d --detach")" ]; then
>  			__git_complete_refs --mode="refs"
> -		elif [ -n "$(__git_find_on_cmdline "--track")" ]; then
> +		elif [ -n "$(__git_find_on_cmdline "-t --track")" ]; then
>  			__git_complete_refs --mode="remote-heads"
>  		else
>  			__git_complete_refs $dwim_opt --mode="heads"

The fallback behaviours are different, which was adequately described in the proposed log message. As "switch" does not want to auto-detach upon receiving a ref that is not a local branch, while "checkout" does, the difference is justifiable.

Show 18 quoted lines

> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 8835e16e811..df8bc44c285 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '
>  '
>  
>  test_expect_success 'git switch - with --track, complete only remote branches' '
> -	test_completion "git switch --track " <<-\EOF
> +:	test_completion "git switch --track " <<-\EOF &&
> +	other/branch-in-other Z
> +	other/main-in-other Z
> +	EOF
> +	test_completion "git switch -t " <<-\EOF
>  	other/branch-in-other Z
>  	other/main-in-other Z
>  	EOF
>  '

So, this demonstrates that '-t' behaves the same way as '--track'.

Show 10 quoted lines

>  test_expect_success 'git checkout - with --track, complete only remote branches' '
> -	test_completion "git checkout --track " <<-\EOF
> +	test_completion "git checkout --track " <<-\EOF &&
> +	other/branch-in-other Z
> +	other/main-in-other Z
> +	EOF
> +	test_completion "git checkout -t " <<-\EOF
>  	other/branch-in-other Z
>  	other/main-in-other Z
>  	EOF

This is, too. If we had a test for "-t" without the code fix, we would have seen local branches in its output, but now we can see the candidates are limited to the remote ones.

Good.

Will queue.  Thanks.