Re: [PATCH] trailer: load config to handle core.commentChar

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 27, 2016, 20:31 UTC
Message-ID: <xmqqlh3y94l0.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAP8UFD1O+V64dr-j9bu7D1mrLxEy3zG017zdJahJTx-MUYv5Pg@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 26 quoted lines

> On Wed, Apr 27, 2016 at 9:24 PM, Rafal Klys <rafalklys@wp.pl> wrote:
>> Add call to git_config(git_default_config, NULL) to update the
>> comment_char_line from default '#' to possible different value set in
>> core.commentChar.
>
> It is "comment_line_char" not "comment_char_line", but otherwise you
> can add "Reviewed-by: Christian Couder <chriscool@tuxfamily.org>".
>
> Thanks!
>
>> Signed-off-by: Rafal Klys <rafalklys@wp.pl>
>> ---
>>  trailer.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/trailer.c b/trailer.c
>> index 8e48a5c..a3700b4 100644
>> --- a/trailer.c
>> +++ b/trailer.c
>> @@ -888,6 +888,9 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
>>         git_config(git_trailer_default_config, NULL);
>>         git_config(git_trailer_config, NULL);
>>
>> +       /* for core.commentChar */
>> +       git_config(git_default_config, NULL);
>> +

I can sort-of see why the original (logically) reads the configuration files twice by making two separate calls to git_config(), but I do not think we should add a third round like this patch does. Shouldn't git-trailer-default-config have a fallthru call to git-default-config instead?

Show 10 quoted lines

>>         lines = read_input_file(file);
>>
>>         if (in_place)
>> --
>> 2.8.1.68.g625efa9.dirty
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html