Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan

From: Junio C Hamano <gitster@pobox.com>
To: Jacob Keller <jacob.keller@gmail.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>,
	Git mailing list <git@vger.kernel.org>,
	Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
Date: Tue, 28 Apr 2020 09:24:52 -0700	[thread overview]
Message-ID: <xmqqo8rb4lyj.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <CA+P7+xpEMb-A1cOkOxdWf0pM=5o8Cyn9=5HLZPtFNMcLUaypsg@mail.gmail.com> (Jacob Keller's message of "Mon, 27 Apr 2020 19:20:44 -0700")

Jacob Keller <jacob.keller@gmail.com> writes:

> The proposed SQUASH you have in gitster/jk/complete-git-switch is
> correct. The commit message body is correct, but the title could be
> reworded to
>
> "completion: stop completing refs for git switch --orphan"
>
> I can send a v2 if that would be helpful, and I've got it fixed up
> locally if other review increases the need for a new spin.

Thnaks.  In the meantime, what I have is attached (the test title,
in addition to the commit title, has been updated).

The same logic would apply to "git checkout -b <TAB>" (or "git
switch -c <TAB>") as this change: these are meant to create a new
branch so you do not want to offer an existing branch name.

I have a mixed feelings about that reasoning, though.  I understand
that not offering any existing branch name would avoid offering a
branch name that would cause an error message from the command,
which at a first glance feels like a nice help to the user, but I am
not sure if it is really helping.

When you are on jk/complete-switch branch to work on this topic, you
may want to keep the current state of the branch and use a "v2"
branch to play around (running "rebase -i" etc.), for which the way
I would hope to work would be:

	git checkout -b jk/comp<TAB>

that would complete to an existing branch

	git checkout -b jk/complete-switch

and then I can just type "-v2<Enter>" (or "<BackSpace>-v2<Enter>" to
remove the inter-word space completion adds?)  after that.  After
all, "git checkout -b jk/complete-switch" in that scenario would
error out by saying that you already use that name, which is a good
enough protection.

And that same "is this really helping?" reasoning applies equally to
the "--orphan" option.

I dunno.

-- >8 --
From: Jacob Keller <jacob.keller@gmail.com>
Date: Fri, 24 Apr 2020 19:20:38 -0700
Subject: [PATCH] completion: stop completing refs for git switch --orphan

git switch with the --orphan option is used to create a new branch that
is not connected to any history and is instead based on the empty tree.

It does not make sense for completion to return anything in this case,
because there is nothing to complete. Check for --orphan, and if it's
found, immediately return from _git_switch() without completing
anything.

Add a test case which documents this expected behavior.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/completion/git-completion.bash | 11 ++++++++++-
 t/t9902-completion.sh                  |  6 ++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c21786f2fd..08d3406cf3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2223,9 +2223,18 @@ _git_switch ()
 		__gitcomp_builtin switch
 		;;
 	*)
+		local track_opt="--track" only_local_ref=n
+
+		# --orphan is used to create a branch disconnected from the
+		# current history, based on the empty tree. Since the only
+		# option required is the branch name, it doesn't make sense to
+		# complete anything here.
+		if [ -n "$(__git_find_on_cmdline "--orphan")" ]; then
+			return
+		fi
+
 		# check if --track, --no-track, or --no-guess was specified
 		# if so, disable DWIM mode
-		local track_opt="--track" only_local_ref=n
 		if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
 		   [ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
 			track_opt=''
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a134a87910..7e4dd8e722 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1351,6 +1351,12 @@ test_expect_failure 'git switch - with -C and --no-track, complete all reference
 	EOF
 '
 
+test_expect_success 'git switch --orphan completes no references' '
+	test_completion "git switch --orphan " <<-\EOF
+
+	EOF
+'
+
 test_expect_success 'teardown after ref completion' '
 	git branch -d matching-branch &&
 	git tag -d matching-tag &&
-- 
2.26.2-266-ge870325ee8

