On Mon, May 16, 2016 at 11:52 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> So "warn and ignore" for data from .gitattributes and die for
>> commandline arguments? That makes sense.
>
> Yes.
>
> On the "command line" front, because we may want to give different
> meanings to these two entries in the future:
>
> :(label=-doc)Documentation/
> :(label=!doc)Documentation/
Yes but both of these already errors out with:
Label -doc': Label names must start with an
alphabetic character and use only alphanumeric
characters, dashes and underscores.
fatal: Labels in the wrong syntax are prohibited.
The command line arguments are not parsed via the attr system.
Only data from .gitattributes are parsed via the attr system.
I can add tests for those to fail though.
>
> we should diagnose -doc (FALSE) as an error, not treating it as the
> same as !doc (UNSET). And we should warn and ignore -doc (FALSE) in
> .gitattributes. Yes, ignoring it would be more or less equivalent
> to treating it as UNSET, but because we may use -doc (FALSE) for a
> better purpose later, we should still warn.
>
>> Ok, so here is the warn-and-ignore code:
>>
>>
>> if (ATTR_TRUE(check.value))
>> ret = 1; /* has all the labels */
>> else if (ATTR_FALSE(check.value)) {
>> warning(_("Path '%s': Label must not be false. Treat
>> as if no label was set"), path);
>> ret = 0;
>
> s/Treat as if .../The -label may be used differently in future
> versions of Git, so do not use it/;
"... but for now Git treats it as if it is not set at all" is a valuable
information to the user, still?
That code path is only used for data coming from .gitattributes,
so we can bike shed the best warning message.
>
> But if we are going in the direction of :(attr:crlf=auto), all this
> discussion is moot, isn't it? I haven't formed a firm opinion on
> this, but it sure does sound tempting, doesn't it?
>
That direction sounds scary as people may use any sort of labels, so
we can no longer add labels later on sanely.
:(attr:random-attr=foo)
should also fall into the "warn and ignore" space. We only want to allow
:(attr:known-attr)
:(attr:label=..)
instead of label= we may want to allow a little bit more, such as
abbreviated 'l='
or 'group=', but not any sort of labeling.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html