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

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

[Rafal Klys]

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.

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);
+
 	lines = read_input_file(file);
 
 	if (in_place)
-- 
2.8.1.68.g625efa9.dirty

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

[Christian Couder]

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);
> +
>         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

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

[Junio C Hamano]

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

> 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?

>>         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

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

[Eric Sunshine]

On Wed, Apr 27, 2016 at 4:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> 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.
>>
>>> 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?

Thanks, I was going to say the same thing.

Also, would it be possible to add a test?