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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 8, 2016, 19:35 UTC
Message-ID: <xmqqpow7807l.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160208150709.GA13664@sigill.intra.peff.net>

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.

Show 35 quoted lines

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