Skip to contentSkip to conversation

completion(switch/checkout): treat --track and -t the same

PATCH In review Johannes Schindelin via GitGitGadget 4 participants 6 messages Sep 8, 2023 – Sep 11, 2023

lore
Share & tools
lore

Johannes Schindelin via GitGitGadget

author
#0 lore raw

PATCH 1/1 completion(switch/checkout): treat --track and -t the same

From: Johannes Schindelin <[email protected]>

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 <[email protected]>

---
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(-)
contrib/completion/git-completion.bash +2 -2
index 133ec92bfae..745dc901fbe 100644
@@ -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"
t/t9902-completion.sh +10 -2
index 8835e16e811..df8bc44c285 100755
@@ -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

Signature · 1 line
gitgitgadget
#1 lore raw

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 4 lines · writes:

From: Johannes Schindelin <[email protected]>

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

Quoted 1 line

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.

Quoted 1 line

Let's make it so.

Sounds good.

Quoted 5 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.

Quoted 14 lines
contrib/completion/git-completion.bash +1 -1
index 133ec92bfae..745dc901fbe 100644
@@ -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"

contrib/completion/git-completion.bash _git_switch () · line 2520

Quoted 8 lines
contrib/completion/git-completion.bash +1 -1
@@ -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.

Quoted 8 lines
t/t9902-completion.sh +0 -0
index 8835e16e811..df8bc44c285 100755
@@ -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' '

t/t9902-completion.sh test_expect_success 'git checkout - with -d, complete only references' ' · line 1633

Quoted 10 lines
t/t9902-completion.sh +5 -1
-	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'.

t/t9902-completion.sh test_expect_success 'git checkout - with -d, complete only references' ' · line 1643

Quoted 10 lines
t/t9902-completion.sh +5 -1
 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.

#2 lore raw
Quoted 9 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 7 lines · writes:
t/t9902-completion.sh +0 -0
index 8835e16e811..df8bc44c285 100755
@@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '
 '
Quoted 3 lines

t/t9902-completion.sh test_expect_success 'git checkout - with -d, complete only references' ' · line 1625

Quoted 3 lines
t/t9902-completion.sh +1 -1
 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).

Quoted 10 lines

t/t9902-completion.sh test_expect_success 'git checkout - with -d, complete only references' ' · line 1633

Quoted 8 lines
t/t9902-completion.sh +4 -0
+	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'.

Signature · 1 line
Todd
#3 lore raw
Quoted 10 lines · Todd Zullinger
Quoted 9 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 7 lines · writes:
t/t9902-completion.sh +0 -0
index 8835e16e811..df8bc44c285 100755
@@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '
 '
Quoted 7 lines
Quoted 3 lines

t/t9902-completion.sh test_expect_success 'git checkout - with -d, complete only references' ' · line 1625

Quoted 3 lines
t/t9902-completion.sh +1 -1
 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 ;-)

#4 lore raw
In reply to Todd Zullinger

Hi Todd,

Quoted 10 lines · Todd Zullinger
Quoted 9 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 7 lines · writes:
t/t9902-completion.sh +0 -0
index 8835e16e811..df8bc44c285 100755
@@ -1622,14 +1622,22 @@ test_expect_success 'git checkout - with -d, complete only references' '
 '
Quoted 7 lines
Quoted 3 lines

t/t9902-completion.sh test_expect_success 'git checkout - with -d, complete only references' ' · line 1625

Quoted 3 lines
t/t9902-completion.sh +1 -1
 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

Quoted 15 lines
Quoted 11 lines

t/t9902-completion.sh test_expect_success 'git checkout - with -d, complete only references' ' · line 1633

Quoted 9 lines
t/t9902-completion.sh +4 -0
+	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

#5 lore raw
Quoted 5 lines · Johannes Schindelin

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.