{"thread":{"id":"60209","subject":"[PATCH] completion(switch/checkout): treat --track and -t the same","startedAt":"2023-09-08T12:28:50Z","lastAt":"2023-09-11T06:09:41Z","messageCount":6,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Todd Zullinger","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"481552","messageId":"pull.1584.git.1694176123471.gitgitgadget@gmail.com","threadId":"60209","inReplyTo":null,"subject":"[PATCH] completion(switch/checkout): treat --track and -t the same","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-08T12:28:43Z","receivedAt":"2023-09-08T12:28:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen `git switch --track ` is to be completed, only remote refs are\neligible because that is what the `--track` option targets.\n\nAnd when the short-hand `-t` is used instead, the same _should_ happen.\nLet's make it so.\n\nNote that the bug exists both in the completions of `switch` and\n`completion`, even if it manifests in slightly different ways: While\nthe completion of `git switch -t ` will not even look at remote refs,\nthe completion of `git checkout -t ` will look at both remote _and_\nlocal refs. Both should look only at remote refs.\n\nSigned-off-by: Johannes Schindelin \n---\n Fix the completion of git switch/git checkout: Treat --track and -t the\n same\n \n Just something that was nagging me for one year too many.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1584%2Fdscho%2Fcomplete-switch-t-properly-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1584/dscho/complete-switch-t-properly-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1584\n\n contrib/completion/git-completion.bash | 4 ++--\n t/t9902-completion.sh | 12 ++++++++++--\n 2 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 133ec92bfae..745dc901fbe 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1607,7 +1607,7 @@ _git_checkout ()\n \n \t\tif [ -n \"$(__git_find_on_cmdline \"-b -B -d --detach --orphan\")\" ]; then\n \t\t\t__git_complete_refs --mode=\"refs\"\n-\t\telif [ -n \"$(__git_find_on_cmdline \"--track\")\" ]; then\n+\t\telif [ -n \"$(__git_find_on_cmdline \"-t --track\")\" ]; then\n \t\t\t__git_complete_refs --mode=\"remote-heads\"\n \t\telse\n \t\t\t__git_complete_refs $dwim_opt --mode=\"refs\"\n@@ -2514,7 +2514,7 @@ _git_switch ()\n \n \t\tif [ -n \"$(__git_find_on_cmdline \"-c -C -d --detach\")\" ]; then\n \t\t\t__git_complete_refs --mode=\"refs\"\n-\t\telif [ -n \"$(__git_find_on_cmdline \"--track\")\" ]; then\n+\t\telif [ -n \"$(__git_find_on_cmdline \"-t --track\")\" ]; then\n \t\t\t__git_complete_refs --mode=\"remote-heads\"\n \t\telse\n \t\t\t__git_complete_refs $dwim_opt --mode=\"heads\"\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 8835e16e811..df8bc44c285 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '\n '\n \n test_expect_success 'git switch - with --track, complete only remote branches' '\n-\ttest_completion \"git switch --track \" <<-\\EOF\n+:\ttest_completion \"git switch --track \" <<-\\EOF &&\n+\tother/branch-in-other Z\n+\tother/main-in-other Z\n+\tEOF\n+\ttest_completion \"git switch -t \" <<-\\EOF\n \tother/branch-in-other Z\n \tother/main-in-other Z\n \tEOF\n '\n \n test_expect_success 'git checkout - with --track, complete only remote branches' '\n-\ttest_completion \"git checkout --track \" <<-\\EOF\n+\ttest_completion \"git checkout --track \" <<-\\EOF &&\n+\tother/branch-in-other Z\n+\tother/main-in-other Z\n+\tEOF\n+\ttest_completion \"git checkout -t \" <<-\\EOF\n \tother/branch-in-other Z\n \tother/main-in-other Z\n \tEOF\n\nbase-commit: 94e83dcf5b5faaa22e32729305f8fd7090bfdfed\n-- \ngitgitgadget\n"},{"id":"481556","messageId":"xmqq1qf8vf1e.fsf@gitster.g","threadId":"60209","inReplyTo":"pull.1584.git.1694176123471.gitgitgadget@gmail.com","subject":"Re: [PATCH] completion(switch/checkout): treat --track and -t the same","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-08T15:51:57Z","receivedAt":"2023-09-08T15:52:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> When `git switch --track ` is to be completed, only remote refs are\n> eligible because that is what the `--track` option targets.\n\nOK. Presumably that is the same for \"checkout --track\".\n\n> And when the short-hand `-t` is used instead, the same _should_ happen.\n\nThat sounds sensible. The code change for _git_checkout() and\n_git_switch() is surprisingly simple ;-) \"-t\" was not handled any\nspecially at all and fell through to \"refs\" complation.\n\n> Let's make it so.\n\nSounds good.\n\n> Note that the bug exists both in the completions of `switch` and\n> `completion`, even if it manifests in slightly different ways: While\n> the completion of `git switch -t ` will not even look at remote refs,\n> the completion of `git checkout -t ` will look at both remote _and_\n> local refs. Both should look only at remote refs.\n\nCorrect.\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 133ec92bfae..745dc901fbe 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1607,7 +1607,7 @@ _git_checkout ()\n> \n> \t\tif [ -n \"$(__git_find_on_cmdline \"-b -B -d --detach --orphan\")\" ]; then\n> \t\t\t__git_complete_refs --mode=\"refs\"\n> -\t\telif [ -n \"$(__git_find_on_cmdline \"--track\")\" ]; then\n> +\t\telif [ -n \"$(__git_find_on_cmdline \"-t --track\")\" ]; then\n> \t\t\t__git_complete_refs --mode=\"remote-heads\"\n> \t\telse\n> \t\t\t__git_complete_refs $dwim_opt --mode=\"refs\"\n> @@ -2514,7 +2514,7 @@ _git_switch ()\n> \n> \t\tif [ -n \"$(__git_find_on_cmdline \"-c -C -d --detach\")\" ]; then\n> \t\t\t__git_complete_refs --mode=\"refs\"\n> -\t\telif [ -n \"$(__git_find_on_cmdline \"--track\")\" ]; then\n> +\t\telif [ -n \"$(__git_find_on_cmdline \"-t --track\")\" ]; then\n> \t\t\t__git_complete_refs --mode=\"remote-heads\"\n> \t\telse\n> \t\t\t__git_complete_refs $dwim_opt --mode=\"heads\"\n\nThe fallback behaviours are different, which was adequately\ndescribed in the proposed log message. As \"switch\" does not want to\nauto-detach upon receiving a ref that is not a local branch, while\n\"checkout\" does, the difference is justifiable.\n\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 8835e16e811..df8bc44c285 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '\n> '\n> \n> test_expect_success 'git switch - with --track, complete only remote branches' '\n> -\ttest_completion \"git switch --track \" <<-\\EOF\n> +:\ttest_completion \"git switch --track \" <<-\\EOF &&\n> +\tother/branch-in-other Z\n> +\tother/main-in-other Z\n> +\tEOF\n> +\ttest_completion \"git switch -t \" <<-\\EOF\n> \tother/branch-in-other Z\n> \tother/main-in-other Z\n> \tEOF\n> '\n\nSo, this demonstrates that '-t' behaves the same way as '--track'.\n\n> test_expect_success 'git checkout - with --track, complete only remote branches' '\n> -\ttest_completion \"git checkout --track \" <<-\\EOF\n> +\ttest_completion \"git checkout --track \" <<-\\EOF &&\n> +\tother/branch-in-other Z\n> +\tother/main-in-other Z\n> +\tEOF\n> +\ttest_completion \"git checkout -t \" <<-\\EOF\n> \tother/branch-in-other Z\n> \tother/main-in-other Z\n> \tEOF\n\nThis is, too. If we had a test for \"-t\" without the code fix, we\nwould have seen local branches in its output, but now we can see the\ncandidates are limited to the remote ones.\n\nGood.\n\nWill queue. Thanks.\n"},{"id":"481558","messageId":"ZPtISYVufU0MazO_@pobox.com","threadId":"60209","inReplyTo":"xmqq1qf8vf1e.fsf@gitster.g","subject":"Re: [PATCH] completion(switch/checkout): treat --track and -t the same","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-09-08T16:14:01Z","receivedAt":"2023-09-08T16:14:08Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Junio C Hamano wrote:\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n>> index 8835e16e811..df8bc44c285 100755\n>> --- a/t/t9902-completion.sh\n>> +++ b/t/t9902-completion.sh\n>> @@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '\n>> '\n>> \n>> test_expect_success 'git switch - with --track, complete only remote branches' '\n>> -\ttest_completion \"git switch --track \" <<-\\EOF\n>> +:\ttest_completion \"git switch --track \" <<-\\EOF &&\n\nIs this new leading \":\" intended? It looks out of place\n(though perhaps I just don't unerstand the context well\nenough).\n\n>> +\tother/branch-in-other Z\n>> +\tother/main-in-other Z\n>> +\tEOF\n>> +\ttest_completion \"git switch -t \" <<-\\EOF\n>> \tother/branch-in-other Z\n>> \tother/main-in-other Z\n>> \tEOF\n>> '\n> \n> So, this demonstrates that '-t' behaves the same way as '--track'.\n\n-- \nTodd\n"},{"id":"481560","messageId":"xmqqil8ktywj.fsf@gitster.g","threadId":"60209","inReplyTo":"ZPtISYVufU0MazO_@pobox.com","subject":"Re: [PATCH] completion(switch/checkout): treat --track and -t the same","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-08T16:25:48Z","receivedAt":"2023-09-08T16:26:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger writes:\n\n> Junio C Hamano wrote:\n>> \"Johannes Schindelin via GitGitGadget\" \n>> writes:\n>>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n>>> index 8835e16e811..df8bc44c285 100755\n>>> --- a/t/t9902-completion.sh\n>>> +++ b/t/t9902-completion.sh\n>>> @@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '\n>>> '\n>>> \n>>> test_expect_success 'git switch - with --track, complete only remote branches' '\n>>> -\ttest_completion \"git switch --track \" <<-\\EOF\n>>> +:\ttest_completion \"git switch --track \" <<-\\EOF &&\n>\n> Is this new leading \":\" intended? It looks out of place\n> (though perhaps I just don't unerstand the context well\n> enough).\n\nGood eyes. It makes it an expensive no-op ;-)\n"},{"id":"481643","messageId":"4f7d166c-040e-eb7c-ef27-81f9748d44a0@gmx.de","threadId":"60209","inReplyTo":"ZPtISYVufU0MazO_@pobox.com","subject":"Re: [PATCH] completion(switch/checkout): treat --track and -t the same","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-09-10T08:22:27Z","receivedAt":"2023-09-10T08:22:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Todd,\n\nOn Fri, 8 Sep 2023, Todd Zullinger wrote:\n\n> Junio C Hamano wrote:\n> > \"Johannes Schindelin via GitGitGadget\" \n> > writes:\n> >> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> >> index 8835e16e811..df8bc44c285 100755\n> >> --- a/t/t9902-completion.sh\n> >> +++ b/t/t9902-completion.sh\n> >> @@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '\n> >> '\n> >>\n> >> test_expect_success 'git switch - with --track, complete only remote branches' '\n> >> -\ttest_completion \"git switch --track \" <<-\\EOF\n> >> +:\ttest_completion \"git switch --track \" <<-\\EOF &&\n>\n> Is this new leading \":\" intended? It looks out of place\n> (though perhaps I just don't unerstand the context well\n> enough).\n\nThanks for catching this. It is a debugging left-over, when I wanted to\nmake sure that the `-t` validation I added would run immediately.\n\nI see that Junio helpfully dropped it before merging down to `next`, so I\nwill refrain from sending a v2.\n\nCiao,\nJohannes\n\n>\n> >> +\tother/branch-in-other Z\n> >> +\tother/main-in-other Z\n> >> +\tEOF\n> >> +\ttest_completion \"git switch -t \" <<-\\EOF\n> >> \tother/branch-in-other Z\n> >> \tother/main-in-other Z\n> >> \tEOF\n> >> '\n> >\n> > So, this demonstrates that '-t' behaves the same way as '--track'.\n>\n> --\n> Todd\n>\n"},{"id":"481665","messageId":"xmqq34zljl5v.fsf@gitster.g","threadId":"60209","inReplyTo":"4f7d166c-040e-eb7c-ef27-81f9748d44a0@gmx.de","subject":"Re: [PATCH] completion(switch/checkout): treat --track and -t the same","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-11T06:09:32Z","receivedAt":"2023-09-11T06:09:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Thanks for catching this. It is a debugging left-over, when I wanted to\n> make sure that the `-t` validation I added would run immediately.\n>\n> I see that Junio helpfully dropped it before merging down to `next`, so I\n> will refrain from sending a v2.\n\nYup. Thanks, all.\n"}]}