Fix the completion of git switch/git checkout: Treat --track and -t the same
#1584
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/submit |
|
Submitted as pull.1584.git.1694176123471.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "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.
> 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.
> 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.
> 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'.
> 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. |
|
On the Git mailing list, Todd Zullinger wrote (reply to this): Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>> 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 &&
Is this new leading ":" intended? It looks out of place
(though perhaps I just don't unerstand the context well
enough).
>> + 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'.
--
Todd |
|
User |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Todd Zullinger <tmz@pobox.com> writes:
> Junio C Hamano wrote:
>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
>> writes:
>>> 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 &&
>
> Is this new leading ":" intended? It looks out of place
> (though perhaps I just don't unerstand the context well
> enough).
Good eyes. It makes it an expensive no-op ;-) |
|
This branch is now known as |
|
This patch series was integrated into seen via git@03ec188. |
|
This patch series was integrated into seen via git@e9ef49d. |
|
This patch series was integrated into next via git@461bb28. |
When `git switch --track ` is to be completed, only remote refs are eligible because that is what the `--track` option targets. And when the short-hand `-t` is used instead, the same _should_ happen. Let's make it so. 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. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
force-pushed
the
complete-switch-t-properly
branch
from
9585c78
to
618ca51
Compare
|
On the Git mailing list, Johannes Schindelin wrote (reply to this): Hi Todd,
On Fri, 8 Sep 2023, Todd Zullinger wrote:
> Junio C Hamano wrote:
> > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >> 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 &&
>
> Is this new leading ":" intended? It looks out of place
> (though perhaps I just don't unerstand the context well
> enough).
Thanks for catching this. It is a debugging left-over, when I wanted to
make sure that the `-t` validation I added would run immediately.
I see that Junio helpfully dropped it before merging down to `next`, so I
will refrain from sending a v2.
Ciao,
Johannes
>
> >> + 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'.
>
> --
> Todd
> |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Thanks for catching this. It is a debugging left-over, when I wanted to
> make sure that the `-t` validation I added would run immediately.
>
> I see that Junio helpfully dropped it before merging down to `next`, so I
> will refrain from sending a v2.
Yup. Thanks, all. |
|
There was a status update in the "Cooking" section about the branch The completion script (in contrib/) has been taught to treat the "-t" option to "git checkout" and "git switch" just like the "--track" option, to complete remote-tracking branches. Will merge to 'master'. source: <pull.1584.git.1694176123471.gitgitgadget@gmail.com> |
|
There was a status update in the "Cooking" section about the branch The completion script (in contrib/) has been taught to treat the "-t" option to "git checkout" and "git switch" just like the "--track" option, to complete remote-tracking branches. Will merge to 'master'. source: <pull.1584.git.1694176123471.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@446376a. |
|
This patch series was integrated into seen via git@5538b8b. |
|
This patch series was integrated into seen via git@9404769. |
|
There was a status update in the "Cooking" section about the branch The completion script (in contrib/) has been taught to treat the "-t" option to "git checkout" and "git switch" just like the "--track" option, to complete remote-tracking branches. Will merge to 'master'. source: <pull.1584.git.1694176123471.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@f41c5a5. |
|
This patch series was integrated into master via git@f41c5a5. |
|
This patch series was integrated into next via git@f41c5a5. |
|
Closed via f41c5a5. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just something that was nagging me for one year too many.
Changes since v1, not sent because the Git maintainer had already fixed it and advanced the patch to
next:cc: Todd Zullinger tmz@pobox.com