[PATCH v2 1/3] rev-parse: correctly diagnose revision errors before "--"
- From
Jeff King <peff@peff.net>
- Date
- Dec 6, 2013, 22:05 UTC
- Message-ID
- <20131206220547.GA25620@sigill.intra.peff.net>
- In-Reply-To
- <20131206220520.GA30652@sigill.intra.peff.net>
Rev-parse understands that a "--" may separate revisions and filenames, and that anything after the "--" is taken as-is. However, it does not understand that anything before the token must be a revision (which is the usual rule implemented by the setup_revisions parser).
Since rev-parse prefers revisions to files when parsing before the "--", we end up with the correct result (if such an argument is a revision, we parse it as one, and if it is not, it is an error either way). However, we misdiagnose the errors:
$ git rev-parse foobar -- >/dev/null fatal: ambiguous argument 'foobar': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]'
$ >foobar $ git rev-parse foobar -- >/dev/null fatal: bad flag '--' used after filename
In both cases, we should know that the real error is that "foobar" is meant to be a revision, but could not be resolved.
Signed-off-by: Jeff King <peff@peff.net> --- builtin/rev-parse.c | 10 ++++++++++ t/t1506-rev-parse-diagnosis.sh | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 1d9ecaf..bb694ab 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -488,6 +488,7 @@ N_("git rev-parse --parseopt [options] -- [<args>...]\n" int cmd_rev_parse(int argc, const char **argv, const char *prefix) { int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0; + int has_dashdash = 0; int output_prefix = 0; unsigned char sha1[20]; const char *name = NULL; @@ -501,6 +502,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) if (argc > 1 && !strcmp("-h", argv[1])) usage(builtin_rev_parse_usage); + for (i = 1; i < argc; i++) { + if (!strcmp(argv[i], "--")) { + has_dashdash = 1; + break; + } + } + prefix = setup_git_directory(); git_config(git_default_config, NULL); for (i = 1; i < argc; i++) { @@ -788,6 +796,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) } if (verify) die_no_single_rev(quiet); + if (has_dashdash) + die("bad revision '%s'", arg); as_is = 1; if (!show_file(arg, output_prefix)) continue; diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh index f950c10..613d9bf 100755 --- a/t/t1506-rev-parse-diagnosis.sh +++ b/t/t1506-rev-parse-diagnosis.sh @@ -196,4 +196,28 @@ test_expect_success 'dotdot is not an empty set' ' test_cmp expect actual ' +test_expect_success 'arg before dashdash must be a revision (missing)' ' + test_must_fail git rev-parse foobar -- 2>stderr && + test_i18ngrep "bad revision" stderr +' + +test_expect_success 'arg before dashdash must be a revision (file)' ' + >foobar && + test_must_fail git rev-parse foobar -- 2>stderr && + test_i18ngrep "bad revision" stderr +' + +test_expect_success 'arg before dashdash must be a revision (ambiguous)' ' + >foobar && + git update-ref refs/heads/foobar HEAD && + { + # we do not want to use rev-parse here, because + # we are testing it + cat .git/refs/heads/foobar && + printf "%s\n" -- + } >expect && + git rev-parse foobar -- >actual && + test_cmp expect actual +' + test_done
-- 1.8.5.524.g6743da6