Jeff King wrote:

> 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.

Neat.

[...]
> --- 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;

Yep, this is the "fall back to looking for a file" part of rev-parse,
so erroring out if there as a dashdash coming is the right thing to
do.  And a quick code search for "rev-parse.*\ --\ " reveals that
most callers would simply not be affected by this.

Thanks for a pleasant patch.

For what it's worth,
Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to