From: Pierre Habouzit <madcoder@debian•org>
To: git@vger•kernel.org
Cc: gitster@pobox•com, Pierre Habouzit <madcoder@debian•org>
Subject: [PATCH 2/2] git-checkout: improve error messages, detect ambiguities.
Date: Wed, 23 Jul 2008 12:15:33 +0200 [thread overview]
Message-ID: <1216808133-31919-3-git-send-email-madcoder@debian.org> (raw)
In-Reply-To: <1216808133-31919-2-git-send-email-madcoder@debian.org>
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.
The other part rewrites the way to understand arguments so that when
git-checkout fails it does with an understandable message. Compared to the
previous behavior we now have:
- a better error message when doing:
git checkout <blob reference> --
now complains about the reference not pointing to a tree, instead of
things like:
error: pathspec <blob reference> did not match any file(s) known to git.
error: pathspec '--' did not match any file(s) known to git.
you what it spitted before.
- a better error message when doing:
git checkout <path> --
It now complains about <path> not being a reference instead of the
completely obscure:
error: pathspec '--' did not match any file(s) known to git.
- an error when -- wasn't used and the first argument is ambiguous.
Signed-off-by: Pierre Habouzit <madcoder@debian•org>
---
builtin-checkout.c | 71 +++++++++++++++++++++++++++++++----------
t/t2010-checkout-ambiguous.sh | 39 ++++++++++++++++++++++
2 files changed, 93 insertions(+), 17 deletions(-)
create mode 100755 t/t2010-checkout-ambiguous.sh
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 9cadf9c..e9ae834 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -430,6 +430,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
OPT_END(),
};
+ int has_dash_dash;
memset(&opts, 0, sizeof(opts));
memset(&new, 0, sizeof(new));
@@ -440,11 +441,52 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+
+ if (!opts.new_branch && (opts.track != git_branch_track))
+ die("git checkout: --track and --no-track require -b");
+
+ if (opts.force && opts.merge)
+ die("git checkout: -f and -m are incompatible");
+
+ /*
+ * case 1: git checkout <ref> -- [<paths>]
+ *
+ * <ref> must be a valid tree, everything after the '--' must be
+ * a path.
+ *
+ * case 2: git checkout -- [<paths>]
+ *
+ * everything after the '--' must be paths.
+ *
+ * case 3: git checkout <something> [<paths>]
+ *
+ * <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.
+ *
+ */
if (argc) {
+ if (!strcmp(argv[0], "--")) { /* case (2) */
+ argv++;
+ argc--;
+ goto no_reference;
+ }
+
arg = argv[0];
- if (get_sha1(arg, rev))
- ;
- else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
+ has_dash_dash = argc > 1 && !strcmp(argv[1], "--");
+
+ if (get_sha1(arg, rev)) {
+ if (has_dash_dash) /* case (1) */
+ die("invalid reference: %s", arg);
+ goto no_reference; /* case (3 -> 2) */
+ }
+
+ /* we can't end up being in (2) anymore, eat the argument */
+ argv++;
+ argc--;
+
+ if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
new.name = arg;
setup_branch_path(&new);
if (resolve_ref(new.path, rev, 1, NULL))
@@ -453,25 +495,20 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
new.path = NULL;
parse_commit(new.commit);
source_tree = new.commit->tree;
- argv++;
- argc--;
- } else if ((source_tree = parse_tree_indirect(rev))) {
+ } else
+ source_tree = parse_tree_indirect(rev);
+
+ 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);
+ else {
argv++;
argc--;
}
}
- if (argc && !strcmp(argv[0], "--")) {
- argv++;
- argc--;
- }
-
- if (!opts.new_branch && (opts.track != git_branch_track))
- die("git checkout: --track and --no-track require -b");
-
- if (opts.force && opts.merge)
- die("git checkout: -f and -m are incompatible");
-
+no_reference:
if (argc) {
const char **pathspec = get_pathspec(prefix, argv);
diff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh
new file mode 100755
index 0000000..50d1f43
--- /dev/null
+++ b/t/t2010-checkout-ambiguous.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+test_description='checkout and pathspecs/refspecs ambiguities'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo hello >world &&
+ echo hello >all &&
+ git add all world &&
+ git commit -m initial &&
+ git branch world
+'
+
+test_expect_success 'reference must be a tree' '
+ test_must_fail git checkout $(git hash-object ./all) --
+'
+
+test_expect_success 'branch switching' '
+ test "refs/heads/master" = "$(git symbolic-ref HEAD)" &&
+ git checkout world -- &&
+ test "refs/heads/world" = "$(git symbolic-ref HEAD)"
+'
+
+test_expect_success 'checkout world from the index' '
+ echo bye > world &&
+ git checkout -- world &&
+ git diff --exit-code --quiet
+'
+
+test_expect_success 'non ambiguous call' '
+ git checkout all
+'
+
+test_expect_success 'ambiguous call' '
+ test_must_fail git checkout world
+'
+
+test_done
--
1.6.0.rc0.155.g8e50b
next prev parent reply other threads:[~2008-07-23 10:16 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-23 1:02 [PATCH] git-checkout: fix argument parsing to detect ambiguous arguments Pierre Habouzit
2008-07-23 1:13 ` Pierre Habouzit
2008-07-23 1:17 ` Johannes Schindelin
2008-07-23 1:32 ` Pierre Habouzit
2008-07-23 1:27 ` [RESEND PATCH] " Pierre Habouzit
2008-07-23 1:39 ` Pierre Habouzit
2008-07-23 10:10 ` Johannes Schindelin
2008-07-23 10:15 ` Resubmit after a night of sleep Pierre Habouzit
2008-07-23 10:15 ` [PATCH 1/2] git-checkout: fix command line parsing Pierre Habouzit
2008-07-23 10:15 ` Pierre Habouzit [this message]
2008-07-23 23:04 ` [PATCH 2/2] git-checkout: improve error messages, detect ambiguities Junio C Hamano
2008-07-24 2:07 ` Junio C Hamano
2008-07-24 8:33 ` Pierre Habouzit
2008-07-23 11:49 ` [PATCH] checkout: mention '--' in the docs SZEDER Gábor
2008-07-23 11:49 ` [PATCH] bash: offer only paths after '--' for 'git checkout' SZEDER Gábor
2008-07-25 20:34 ` Shawn O. Pearce
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1216808133-31919-3-git-send-email-madcoder@debian.org \
--to=madcoder@debian$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox