git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 1/3] rev-parse: correctly diagnose revision errors before "--"

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Dec 6, 2013, 23:34 UTC
Message-ID
<20131206233410.GO29959@google.com>
In-Reply-To
<20131206220547.GA25620@sigill.intra.peff.net>
Jeff King wrote:
Show 18 quoted lines
> 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.
[...]
Show 31 quoted lines
> --- 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 <jrnieder@gmail.com>
Previous: Jeff KingNext: Jeff King
Message 14 of 28 in “[BUG] redundant error message”
  1. Duy NguyenDec 5, 2013
  2. Jeff KingDec 5, 2013
  3. Junio C HamanoDec 5, 2013
  4. Jeff KingDec 5, 2013
  5. Junio C HamanoDec 5, 2013
  6. Jeff KingDec 5, 2013
  7. Jeff KingDec 5, 2013
  8. Junio C HamanoDec 5, 2013
  9. 0/2 rev-parse and "--"Jeff King, Dec 6, 2013
  10. 1/2 rev-parse: correctly diagnose revision errors before "--"Jeff King, Dec 6, 2013
  11. 2/2 rev-parse: diagnose ambiguous revision/filename argumentsJeff King, Dec 6, 2013
  12. 0/3 rev-parse and "--"Jeff King, Dec 6, 2013
  13. 1/3 rev-parse: correctly diagnose revision errors before "--"Jeff King, Dec 6, 2013
  14. Jonathan NiederDec 6, 2013
  15. 2/3 rev-parse: be more careful with munging argumentsJeff King, Dec 6, 2013
  16. Jonathan NiederDec 7, 2013
  17. Eric SunshineDec 9, 2013
  18. 3/3 rev-parse: diagnose ambiguous revision/filename argumentsJeff King, Dec 6, 2013
  19. Jonathan NiederDec 6, 2013
  20. Jeff KingDec 6, 2013
  21. Junio C HamanoDec 9, 2013
  22. Jonathan NiederDec 9, 2013
  23. Jonathan NiederDec 9, 2013
  24. Junio C HamanoDec 9, 2013
  25. Jonathan NiederDec 9, 2013
  26. Junio C HamanoDec 9, 2013
  27. Duy NguyenDec 6, 2013
  28. Jeff KingDec 6, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.