Re: [PATCH 2/2] git-checkout: improve error messages, detect ambiguities.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:00
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH 2/2] git-checkout: improve error messages, detect ambiguities. · Pierre Habouzit <hidden>
- 2016-06-15 · Re: [PATCH 2/2] git-checkout: improve error messages, detect ambiguities. · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH 2/2] git-checkout: improve error messages, detect ambiguities. · Pierre Habouzit <hidden>
Junio C Hamano [off-list ref] writes:
Pierre Habouzit [off-list ref] writes:quoted
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