Re: [PATCH 2/2] git-checkout: improve error messages, detect ambiguities.
Junio C Hamano <gitster@pobox.com> writes:
Show 16 quoted lines
> Pierre Habouzit <madcoder@debian.org> 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
Junio C Hamano <gitster@pobox.com>