Re: [RFC/PATCH] Add interpret-trailers builtin

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 5, 2013, 19:09 UTC
Message-ID: <xmqqli12acoh.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CALKQrgcweo+B8JKH85-4-SOHsAi6SAxstnXhRWCst_qEyXYmhA@mail.gmail.com>

Johan Herland <johan@herland.net> writes:

Show 26 quoted lines

> On Mon, Nov 4, 2013 at 8:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Johan Herland <johan@herland.net> writes:
>>>> +{
>>>> +       char *end = strchr(arg, '=');
>>>> +       if (!end)
>>>> +               end = strchr(arg, ':');
>>>
>>> So both '=' (preferred) and ':' are accepted as field/value
>>> separators. That's ok for the command-line, I believe.
>>
>> Why?
>>
>> Sometimes you have to be loose from the beginning _if_ some existing
>> uses and established conventions make it easier for the users, but
>> if you do not have to start from being loose, it is almost always a
>> mistake to do so.  The above code just closed the door to use ":"
>> for some other useful purposes we may later discover, and will make
>> us regret for doing so.
>
> Although I agree with the principle, I think there are (at least) two
> established conventions that will be commonly used from the start, and
> that we should support:
>
>  - Using short forms with '=', e.g. "ack=Peff". There is already a
> convention on how we specify <name> + <value> pairs on the command
> line, e.g. "git -c foo=bar ..."

I do not have much problem with this.

>  - Copy-pasting footers from existing commit messages. These will have
> the same format as the expected output of this command, and not
> accepting the same format in its input seems silly, IMHO.

I am not sure about this, but syntactically, it is very similar to

    --message "CC: Johan Herland <j@h.net>"

so probably it is OK, but then we do not even have to limit it to colon, no? E.g. appending an arbitrary footer, with its literal value, may be done with something like:

    --footer "CC: Johan Herland <j@h.net>"
    --footer "Closes #12345"

Show 7 quoted lines

>> Also, there is a distinction between fields with the same field name
>> appearing twice and fields with the same field name and the same
>> field value appearing twice. Some headers do want to have them, some
>> do not.
>
> True. This complexity is partly why I initially wanted to leave this
> whole thing up to hooks, but I guess now we have to deal with it...

If we are adding anything, it has to be able to express what we already do for Signed-off-by: line, so we cannot start from somewhere any simpler than this, I am afraid.