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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> 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 ;-)

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

> 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);
>  }