# [PATCH] completion(switch/checkout): treat --track and -t the same 6 messages from 2023-09-08 to 2023-09-11. Participants: Johannes Schindelin via GitGitGadget, Junio C Hamano, Todd Zullinger, Johannes Schindelin. Thread: https://gitlist.dev/t/60209 ## Johannes Schindelin via GitGitGadget, 2023-09-08 12:28 Subject: [PATCH] completion(switch/checkout): treat --track and -t the same Message-ID: URL: https://gitlist.dev/e/pull.1584.git.1694176123471.gitgitgadget%40gmail.com ``` From: Johannes Schindelin 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 --- Fix the completion of git switch/git checkout: Treat --track and -t the same Just something that was nagging me for one year too many. Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1584%2Fdscho%2Fcomplete-switch-t-properly-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1584/dscho/complete-switch-t-properly-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1584 contrib/completion/git-completion.bash | 4 ++-- t/t9902-completion.sh | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) 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" 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 ' 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 base-commit: 94e83dcf5b5faaa22e32729305f8fd7090bfdfed -- gitgitgadget ``` ## Junio C Hamano, 2023-09-08 15:51 Subject: Re: [PATCH] completion(switch/checkout): treat --track and -t the same Message-ID: URL: https://gitlist.dev/e/xmqq1qf8vf1e.fsf%40gitster.g In-Reply-To: ``` "Johannes Schindelin via GitGitGadget" writes: > From: Johannes Schindelin > > 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. ``` ## Todd Zullinger, 2023-09-08 16:14 Subject: Re: [PATCH] completion(switch/checkout): treat --track and -t the same Message-ID: URL: https://gitlist.dev/e/ZPtISYVufU0MazO_%40pobox.com In-Reply-To: ``` Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" > 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 ``` ## Junio C Hamano, 2023-09-08 16:25 Subject: Re: [PATCH] completion(switch/checkout): treat --track and -t the same Message-ID: URL: https://gitlist.dev/e/xmqqil8ktywj.fsf%40gitster.g In-Reply-To: ``` Todd Zullinger writes: > Junio C Hamano wrote: >> "Johannes Schindelin via GitGitGadget" >> 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 ;-) ``` ## Johannes Schindelin, 2023-09-10 08:22 Subject: Re: [PATCH] completion(switch/checkout): treat --track and -t the same Message-ID: <4f7d166c-040e-eb7c-ef27-81f9748d44a0@gmx.de> URL: https://gitlist.dev/e/4f7d166c-040e-eb7c-ef27-81f9748d44a0%40gmx.de In-Reply-To: ``` Hi Todd, On Fri, 8 Sep 2023, Todd Zullinger wrote: > Junio C Hamano wrote: > > "Johannes Schindelin via GitGitGadget" > > 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 > ``` ## Junio C Hamano, 2023-09-11 06:09 Subject: Re: [PATCH] completion(switch/checkout): treat --track and -t the same Message-ID: URL: https://gitlist.dev/e/xmqq34zljl5v.fsf%40gitster.g In-Reply-To: <4f7d166c-040e-eb7c-ef27-81f9748d44a0@gmx.de> ``` Johannes Schindelin 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. ```