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

Re: git show doesn't work on file names with square brackets

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 10, 2016, 17:35 UTC
Message-ID
<xmqqpow4zcwd.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20160210161548.GC19867@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 6 quoted lines
> Yes, because ":/" is treated specially in check_filename(), and avoids
> kicking in the wildcard behavior. That is certainly preferring revs to
> pathspecs, but I think preferring one over the other is preferable to
> barfing. If the user wants carefulness, they should use "--"
> unconditionally. If they want to DWIM, we should make it as painless as
> possible, even if we sometimes guess wrong.
OK, I think that is sensible.
> But I have a feeling from what you've written that you do not agree with
> the "err and allow something possibly ambiguous" philosophy.
Not anymore ;-)
Show 18 quoted lines
>> I actually think that no_wildcard() check added in check_filename()
>> was the original mistake.  If we revert the check_filename() to a
>> simple "Is this a filename?" and move the "does this thing have a
>> wildcard" aka "can this be a pathspec even when check_filename()
>> says there is no file with that exact name?" to the code that tries
>> to allow users omit "--", i.e. the caller of check_filename(), would
>> that make the code structure and the semantics much cleaner, I
>> wonder...
>
> Yes. After writing the above, I was envisioning pushing the "err on this
> side" logic into check_filename() with a flag. The main callers are
> verify_filename() and verify_non_filename(), and they would use opposite
> flags from each other.  But pulling that logic out to the caller would
> be fine, too.
>
> IOW, something like this implements the "permissive" thing I wrote above
> (i.e., be inclusive when seeing if something could plausibly be a
> filename, but exclusive when complaining that it _could_ be one):
Yup, I think that is probably a better first step.
Show 24 quoted lines
> diff --git a/setup.c b/setup.c
> index 2c4b22c..995e924 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -139,9 +139,7 @@ int check_filename(const char *prefix, const char *arg)
>  		if (arg[2] == '\0') /* ":/" is root dir, always exists */
>  			return 1;
>  		name = arg + 2;
> -	} else if (!no_wildcard(arg))
> -		return 1;
> -	else if (prefix)
> +	} else if (prefix)
>  		name = prefix_filename(prefix, strlen(prefix), arg);
>  	else
>  		name = arg;
> @@ -202,7 +200,7 @@ void verify_filename(const char *prefix,
>  {
>  	if (*arg == '-')
>  		die("bad flag '%s' used after filename", arg);
> -	if (check_filename(prefix, arg))
> +	if (check_filename(prefix, arg) || !no_wildcard(arg))
>  		return;
>  	die_verify_filename(prefix, arg, diagnose_misspelt_rev);
>  }
Previous: Jeff KingNext: Jeff King
Message 19 of 27 in “git show doesn't work on file names with square brackets”
  1. Kirill LikhodedovFeb 6, 2016
  2. Johannes SchindelinFeb 6, 2016
  3. Kirill LikhodedovFeb 6, 2016
  4. Johannes SchindelinFeb 6, 2016
  5. Duy NguyenFeb 6, 2016
  6. Kirill LikhodedovFeb 7, 2016
  7. Duy NguyenFeb 8, 2016
  8. Jeff KingFeb 8, 2016
  9. Jeff KingFeb 8, 2016
  10. Jeff KingFeb 8, 2016
  11. Junio C HamanoFeb 8, 2016
  12. Jeff KingFeb 8, 2016
  13. Jeff KingFeb 8, 2016
  14. Jeff KingFeb 8, 2016
  15. Junio C HamanoFeb 8, 2016
  16. Jeff KingFeb 10, 2016
  17. Junio C HamanoFeb 9, 2016
  18. Jeff KingFeb 10, 2016
  19. Junio C HamanoFeb 10, 2016
  20. Jeff KingFeb 10, 2016
  21. 1/3 checkout: reorder check_filename conditionalJeff King, Feb 10, 2016
  22. Junio C HamanoFeb 10, 2016
  23. 2/3 check_filename: tighten dwim-wildcard ambiguityJeff King, Feb 10, 2016
  24. 3/3 get_sha1: don't die() on bogus search stringsJeff King, Feb 10, 2016
  25. Junio C HamanoFeb 10, 2016
  26. Kirill LikhodedovFeb 7, 2016
  27. Johannes SchindelinFeb 7, 2016

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.