[RFC/PATCH] checkout: allow dwim for branch creation for "git checkout $branch --"

* [RFC/PATCH] checkout: allow dwim for branch creation for "git checkout $branch --"
@ 2013-09-25 12:49 Matthieu Moy
  2013-09-25 13:22 ` Duy Nguyen
  0 siblings, 1 reply; 7+ messages in thread
From: Matthieu Moy @ 2013-09-25 12:49 UTC (permalink / raw)
  To: git; +Cc: pclouds, jc, Matthieu Moy

The "--" notation disambiguates files and branches, but as a side-effect
of the previous implementation, also disabled the branch auto-creation
when $branch does not exist.

A possible scenario is then:

git checkout $branch
=> fails if $branch is both a ref and a file, and suggests --

git checkout $branch --
=> refuses to create the $branch

This patch allows the second form to create $branch, and since the -- is
provided, it does not look for file named $branch.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/checkout.c       | 32 ++++++++++++++++++++++++--------
 t/t2024-checkout-dwim.sh | 22 ++++++++++++++++++++++
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0f57397..f1f5915 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -863,6 +863,14 @@ static const char *unique_tracking_name(const char *name, unsigned char *sha1)
 	return NULL;
 }
 
+static int error_invalid_ref(const char *arg, int has_dash_dash, int argcount)
+{
+	if (has_dash_dash)
+		die(_("invalid reference: %s"), arg);
+	else
+		return argcount;
+}
+
 static int parse_branchname_arg(int argc, const char **argv,
 				int dwim_new_local_branch_ok,
 				struct branch_info *new,
@@ -916,20 +924,28 @@ static int parse_branchname_arg(int argc, const char **argv,
 	if (!strcmp(arg, "-"))
 		arg = "@{-1}";
 
-	if (get_sha1_mb(arg, rev)) {
-		if (has_dash_dash)          /* case (1) */
-			die(_("invalid reference: %s"), arg);
-		if (dwim_new_local_branch_ok &&
-		    !check_filename(NULL, arg) &&
-		    argc == 1) {
+	if (get_sha1_mb(arg, rev)) { /* case (1)? */
+		int try_dwim = dwim_new_local_branch_ok;
+
+		if (check_filename(NULL, arg) && !has_dash_dash)
+			try_dwim = 0;
+		/*
+		 * Accept "git checkout foo" and "git checkout foo --"
+		 * as candidates for dwim.
+		 */
+		if (!(argc == 1 && !has_dash_dash) &&
+		    !(argc == 2 && has_dash_dash))
+			try_dwim = 0;
+
+		if (try_dwim) {
 			const char *remote = unique_tracking_name(arg, rev);
 			if (!remote)
-				return argcount;
+				return error_invalid_ref(arg, has_dash_dash, argcount);
 			*new_branch = arg;
 			arg = remote;
 			/* DWIMmed to create local branch */
 		} else {
-			return argcount;
+			return error_invalid_ref(arg, has_dash_dash, argcount);
 		}
 	}
 
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 094b92e..d9afdb2 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -164,4 +164,26 @@ test_expect_success 'checkout of branch from a single remote succeeds #4' '
 	test_branch_upstream eggs repo_d eggs
 '
 
+test_expect_success 'checkout of branch with a file having the same name fails' '
+	git checkout -B master &&
+	test_might_fail git branch -D spam &&
+
+	>spam &&
+	test_must_fail git checkout spam &&
+	test_must_fail git checkout spam &&
+	test_must_fail git rev-parse --verify refs/heads/spam &&
+	test_branch master
+'
+
+test_expect_success 'checkout <branch> -- succeeds, even if a file with the same name exists' '
+	git checkout -B master &&
+	test_might_fail git branch -D spam &&
+
+	>spam &&
+	git checkout spam -- &&
+	test_branch spam &&
+	test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD &&
+	test_branch_upstream spam repo_c spam
+'
+
 test_done
-- 
1.8.4.475.g703937e.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread