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

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

Jeff King <peff@peff.net> writes:

> The patch for that might look like this. I like it for its relative
> simplicity, though it does make the rules even harder to explain to a
> user...

True.

To be bluntly honest, I do not see the current "string containing
wildcard characters are taken as path, not rev, unless you use the
double dash to disambiguate." all bad.  Isn't it sort of crazy to
have square brackets in paths and if it requires clarification by
the user, I do not particulasrly see it as a problem.

Having said that, I do not think of a big reason to say this patch
is a wrong thing to do, either.

> This breaks the second test in t2019 added by ae454f6, but I am not sure
> that test is doing the right thing (I'm also not sure t2019 is the best
> place for these tests; I added new ones here in a separate script).

I am inclined to agree that that particular test is casting an
implementation limitation in stone.

> We can afford to be fairly slack in our parsing here. We are
> not making a real decision on "this is or is not definitely
> a revision" here, but rather just deciding whether or not
> the extra "wildcards mean pathspecs" magic kicks in.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  setup.c                      | 21 ++++++++++++++++++++-
>  t/t6133-pathspec-rev-dwim.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+), 1 deletion(-)
>  create mode 100755 t/t6133-pathspec-rev-dwim.sh
>
> diff --git a/setup.c b/setup.c
> index 2c4b22c..03ee4eb 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -130,6 +130,25 @@ int path_inside_repo(const char *prefix, const char *path)
>  	return 0;
>  }
>  
> +static int dwim_as_wildcard(const char *arg)
> +{
> +	const char *p;
> +
> +	if (no_wildcard(arg))
> +		return 0;
> +	if (strstr(arg, "^{"))
> +		return 0; /* probably "^{something}" */
> +	if (strstr(arg, "@{"))
> +		return 0; /* probably "ref@{something}" */
> +
> +	/* catch "tree:path", but not ":(magic)" */
> +	p = strchr(arg, ':');
> +	if (p && p[1] != '(')
> +		return 0;

You seem to reject ":(" specifically, but I am not sure whom is it
designed to help to special case ":(".  Those who write ":(top)"
would not have to disambiguate with "--", but their preference is to
spell things in longhand for more explicit control, so I do not
think they mind typing "--".  On the other hand, those who write
":/" and ":!" (":(top)" and ":(exclude)") would need to disambiguate
with "--" with the change.

That somehow feels backwards.

"A pathspec element with the magic prefix" is hard to tell from
"Look for a path in the index" but not from "Look for a path in a
tree-ish", so if you get (p && p != arg), you know it is tree:path,
I think.