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

From: Christian Couder <chriscool@tuxfamily.org>
Date: Nov 5, 2013, 05:42 UTC
Message-ID: <20131105.064219.1992046444757435360.chriscool@tuxfamily.org>
In-Reply-To: <CALKQrgcweo+B8JKH85-4-SOHsAi6SAxstnXhRWCst_qEyXYmhA@mail.gmail.com>

From: Johan Herland <johan@herland.net>

Show 14 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,

The users are already used to appending "Acked-by: Joe <joe@example.com>". So I think it makes it easier for the user to accept what they are already used to provide.

Show 17 quoted lines

>> 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 ..."
> 
>  - 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 agree. Also I think it will avoid some mistakes by the users. Because it would require an effort for them to remember that if they want to see "Acked-by: Joe <joe@example.com>" they have to put "Acked-by= Joe <joe@example.com>" on the command line.

> That said, I think this applies only to the formatting on the _command
> line_.

But wouldn't it be nice if the same parsing function could be used for both the command line and the commit message template?

> When it comes to how the resulting footers are formatted in the
> commit message, I would argue it only makes sense to use ':', and I
> think the testcase named 'with config setup and = sign' in the above
> patch is ugly and unnecessary.

I wanted to support configurations like this:

[trailer "ack"]
        value = "Acked-by= "
[trailer "bug"]
        value = "Bug #"

because Peff said that GitHub uses '#' and while at it I suppose some people might prefer '=' over ':'.

Thanks, Christian.