Subject: Re: [WIP PATCH 0/5] support --exclude for diff/log commands

From: Michael J Gruber <[email-masked]>
Date: Wed, 23 Mar 2011 13:51:18 +0100

Nguyen Thai Ngoc Duy venit, vidit, dixit 23.03.2011 13:10:
> On Wed, Mar 23, 2011 at 6:59 AM, Junio C Hamano <[email-masked]> wrote:
>> Let's clarify by dumping my understanding of what we aim to achieve.
>>
>> ...
>>
>> Both the ":/<path>" proposal (or your ":<path>" proposal) changes only a
>> very small part of the above, namely, "each prefixed with '<prefix>'" is
>> changed to if the element in original pathspec has the magic colon prefix,
>> the magic is stripped away, and the remainder becomes the element in the
>> resulting pathspec array without additional <prefix> in front.
> 
> Correct.
> 
>> If Nguyen's proposal is to also match ":/<path>" (or ":<path>") literally,
>> that part should be scrapped.  If somebody wants to match such an unusual
>> path component, it can always be expressed by quoting it as a glob,
>> i.e. "[:]/<path>" (or "[:]<path").
> 
> OK.
> 
>> I am slightly in favor of ":<path>" syntax (than ":/<path>"), but I do not
>> care too deeply.  Either has the same problem that it will be confusing
>> with existing and well-established syntax (the former would conflict with
>> "name of the blob in the index", the latter with "name of the commit that
>> match the regexp).
> 
> How about ":<path>" for root pathspecs, but reserve ':[^0-9A-Za-z]*'
> for future use?
> 
> (on top of Michael's patch)
> 
> -- 8< --
> diff --git a/setup.c b/setup.c
> index ef55e5d..1ebe1d2 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -146,8 +146,13 @@ const char **get_pathspec(const char *prefix,
> const char **pathspec)
>  	while (*src) {
>  		const char *p;
> 
> -		if ((*src)[0] == ':')
> +		if ((*src)[0] == ':') {
> +			const char *reserved = "~`!@#$%^&*()-_=+[{]}\\|;:'\",<.>/?";
> +			if (strchr(reserved, (*src)[1]))
> +				die("':%c' syntax is not supported. "
> +				    "Quote it to match literally.", (*src)[1]);
>  			p = prefix_path(NULL, 0, (*src)+1);
> +		}
>  		else
>  			p = prefix_path(prefix, prefixlen, *src);
>  		*(dst++) = p;
> -- 8< --

Sounds good to me so far.

Note that there is also possible notational overlap with the WIP "attr:
make attributes depend on file type" at

http://permalink.gmane.org/gmane.comp.version-control.git/168116

which suggested something like

:symlink:* diff=symlinkdiffdriverofyourchoice

as a notation for attributes which depend on the file type. Is that a
problem, should I do something different there? (Also, I still need help
with that...)

Michael