Re: [PATCH 2/2] git-checkout: improve error messages, detect ambiguities.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Pierre Habouzit <madcoder@xxxxxxxxxx> writes:
>
>> The patch is twofold: it moves the option consistency checks just under
>> the parse_options call so that it doesn't get in the way of the tree
>> reference vs. pathspecs desambiguation.
>
> I think this goes a bit too far.
>
> Even if you have a file called 'master' tracked in your project, when you
> say:
>
>     $ git checkout master
>
> that's almost always branch switching.  Forcing "git checkout master --"
> disambiguation for such a common case is simply a wrong thing to do from
> the usability point of view.

So something like this on top of your patch.

 builtin-checkout.c            |   19 +++++++++++++++----
 t/t2010-checkout-ambiguous.sh |   15 +++++++++++++--
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index d99c1c0..411cc51 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -460,7 +460,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	 *
 	 * case 3: git checkout <something> [<paths>]
 	 *
-	 *   <something> shall not be ambiguous.
+	 *   With no paths, if <something> is a commit, that is to
+	 *   switch to the branch or detach HEAD at it.
+	 *
+	 *   Otherwise <something> shall not be ambiguous.
 	 *   - If it's *only* a reference, treat it like case (1).
 	 *   - If it's only a path, treat it like case (2).
 	 *   - else: fail.
@@ -474,7 +477,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		}
 
 		arg = argv[0];
-		has_dash_dash = argc > 1 && !strcmp(argv[1], "--");
+		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
 
 		if (get_sha1(arg, rev)) {
 			if (has_dash_dash)          /* case (1) */
@@ -500,8 +503,16 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 
 		if (!source_tree)                   /* case (1): want a tree */
 			die("reference is not a tree: %s", arg);
-		if (!has_dash_dash)                 /* case (3 -> 1) */
-			verify_non_filename(NULL, arg);
+		if (!has_dash_dash) {/* case (3 -> 1) */
+			/*
+			 * Do not complain the most common case
+			 *	git checkout branch
+			 * even if there happen to be a file called 'branch';
+			 * it would be extremely annoying.
+			 */
+			if (argc)
+				verify_non_filename(NULL, arg);
+		}
 		else {
 			argv++;
 			argc--;
diff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh
index 50d1f43..7cc0a35 100755
--- a/t/t2010-checkout-ambiguous.sh
+++ b/t/t2010-checkout-ambiguous.sh
@@ -32,8 +32,19 @@ test_expect_success 'non ambiguous call' '
 	git checkout all
 '
 
-test_expect_success 'ambiguous call' '
-	test_must_fail git checkout world
+test_expect_success 'allow the most common case' '
+	git checkout world &&
+	test "refs/heads/world" = "$(git symbolic-ref HEAD)"
+'
+
+test_expect_success 'check ambiguity' '
+	test_must_fail git checkout world all
+'
+
+test_expect_success 'disambiguate checking out from a tree-ish' '
+	echo bye > world &&
+	git checkout world -- world &&
+	git diff --exit-code --quiet
 '
 
 test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux