threads / patch / 42159

patchtrailer: load config to handle core.commentChar

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

## tl;dr

4 messages between Apr 27, 2016 and Apr 27, 2016. Diffs are folded; open one to read it.

replies: 3people: 4as markdown or json

Rafal Klys· Apr 27, 2016, 19:24 UTC · lore

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(+)
Show changes to trailer.c +3 −0
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
Christian Couder· Apr 27, 2016, 20:13 UTC · re: Rafal Klys · lore

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

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!
Show 26 quoted lines
> 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
Junio C Hamano· Apr 27, 2016, 20:31 UTC · re: Christian Couder · lore

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

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
Eric Sunshine· Apr 27, 2016, 20:33 UTC · re: Junio C Hamano · lore

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

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

← back to recent threads