{"thread":{"id":"42159","subject":"[PATCH] trailer: load config to handle core.commentChar","startedAt":"2016-04-27T19:24:22Z","lastAt":"2016-04-27T20:33:04Z","messageCount":4,"participants":["Rafal Klys","Christian Couder","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"284711","messageId":"1461785062-23523-1-git-send-email-rafalklys@wp.pl","threadId":"42159","inReplyTo":null,"subject":"[PATCH] trailer: load config to handle core.commentChar","fromName":"Rafal Klys","fromEmail":"rafalklys@wp.pl","sentAt":"2016-04-27T19:24:22Z","receivedAt":"2016-04-27T19:24:22Z","isPatch":true,"sender":{"key":"rafalklys@wp.pl","avatar":null},"body":"Add call to git_config(git_default_config, NULL) to update the\ncomment_char_line from default '#' to possible different value set in\ncore.commentChar.\n\nSigned-off-by: Rafal Klys <rafalklys@wp.pl>\n---\n trailer.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/trailer.c b/trailer.c\nindex 8e48a5c..a3700b4 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -888,6 +888,9 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n \tgit_config(git_trailer_default_config, NULL);\n \tgit_config(git_trailer_config, NULL);\n \n+\t/* for core.commentChar */\n+\tgit_config(git_default_config, NULL);\n+\n \tlines = read_input_file(file);\n \n \tif (in_place)\n-- \n2.8.1.68.g625efa9.dirty\n"},{"id":"284733","messageId":"CAP8UFD1O+V64dr-j9bu7D1mrLxEy3zG017zdJahJTx-MUYv5Pg@mail.gmail.com","threadId":"42159","inReplyTo":"1461785062-23523-1-git-send-email-rafalklys@wp.pl","subject":"Re: [PATCH] trailer: load config to handle core.commentChar","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-04-27T20:13:09Z","receivedAt":"2016-04-27T20:13:09Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Apr 27, 2016 at 9:24 PM, Rafal Klys <rafalklys@wp.pl> wrote:\n> Add call to git_config(git_default_config, NULL) to update the\n> comment_char_line from default '#' to possible different value set in\n> core.commentChar.\n\nIt is \"comment_line_char\" not \"comment_char_line\", but otherwise you\ncan add \"Reviewed-by: Christian Couder <chriscool@tuxfamily.org>\".\n\nThanks!\n\n> Signed-off-by: Rafal Klys <rafalklys@wp.pl>\n> ---\n>  trailer.c | 3 +++\n>  1 file changed, 3 insertions(+)\n>\n> diff --git a/trailer.c b/trailer.c\n> index 8e48a5c..a3700b4 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -888,6 +888,9 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n>         git_config(git_trailer_default_config, NULL);\n>         git_config(git_trailer_config, NULL);\n>\n> +       /* for core.commentChar */\n> +       git_config(git_default_config, NULL);\n> +\n>         lines = read_input_file(file);\n>\n>         if (in_place)\n> --\n> 2.8.1.68.g625efa9.dirty\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"284738","messageId":"xmqqlh3y94l0.fsf@gitster.mtv.corp.google.com","threadId":"42159","inReplyTo":"CAP8UFD1O+V64dr-j9bu7D1mrLxEy3zG017zdJahJTx-MUYv5Pg@mail.gmail.com","subject":"Re: [PATCH] trailer: load config to handle core.commentChar","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-27T20:31:07Z","receivedAt":"2016-04-27T20:31:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> On Wed, Apr 27, 2016 at 9:24 PM, Rafal Klys <rafalklys@wp.pl> wrote:\n>> Add call to git_config(git_default_config, NULL) to update the\n>> comment_char_line from default '#' to possible different value set in\n>> core.commentChar.\n>\n> It is \"comment_line_char\" not \"comment_char_line\", but otherwise you\n> can add \"Reviewed-by: Christian Couder <chriscool@tuxfamily.org>\".\n>\n> Thanks!\n>\n>> Signed-off-by: Rafal Klys <rafalklys@wp.pl>\n>> ---\n>>  trailer.c | 3 +++\n>>  1 file changed, 3 insertions(+)\n>>\n>> diff --git a/trailer.c b/trailer.c\n>> index 8e48a5c..a3700b4 100644\n>> --- a/trailer.c\n>> +++ b/trailer.c\n>> @@ -888,6 +888,9 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n>>         git_config(git_trailer_default_config, NULL);\n>>         git_config(git_trailer_config, NULL);\n>>\n>> +       /* for core.commentChar */\n>> +       git_config(git_default_config, NULL);\n>> +\n\nI can sort-of see why the original (logically) reads the\nconfiguration files twice by making two separate calls to\ngit_config(), but I do not think we should add a third round like\nthis patch does.  Shouldn't git-trailer-default-config have a\nfallthru call to git-default-config instead?\n\n>>         lines = read_input_file(file);\n>>\n>>         if (in_place)\n>> --\n>> 2.8.1.68.g625efa9.dirty\n>>\n>> --\n>> To unsubscribe from this list: send the line \"unsubscribe git\" in\n>> the body of a message to majordomo@vger.kernel.org\n>> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"284739","messageId":"CAPig+cQWOr5rQ65RJkyJZGPjYDWMv7nhL8Pbh55KGD9wZzrDHQ@mail.gmail.com","threadId":"42159","inReplyTo":"xmqqlh3y94l0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] trailer: load config to handle core.commentChar","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-27T20:33:04Z","receivedAt":"2016-04-27T20:33:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Apr 27, 2016 at 4:31 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> On Wed, Apr 27, 2016 at 9:24 PM, Rafal Klys <rafalklys@wp.pl> wrote:\n>>> Add call to git_config(git_default_config, NULL) to update the\n>>> comment_char_line from default '#' to possible different value set in\n>>> core.commentChar.\n>>\n>>> Signed-off-by: Rafal Klys <rafalklys@wp.pl>\n>>> ---\n>>>  trailer.c | 3 +++\n>>>  1 file changed, 3 insertions(+)\n>>>\n>>> diff --git a/trailer.c b/trailer.c\n>>> index 8e48a5c..a3700b4 100644\n>>> --- a/trailer.c\n>>> +++ b/trailer.c\n>>> @@ -888,6 +888,9 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str\n>>>         git_config(git_trailer_default_config, NULL);\n>>>         git_config(git_trailer_config, NULL);\n>>>\n>>> +       /* for core.commentChar */\n>>> +       git_config(git_default_config, NULL);\n>>> +\n>\n> I can sort-of see why the original (logically) reads the\n> configuration files twice by making two separate calls to\n> git_config(), but I do not think we should add a third round like\n> this patch does.  Shouldn't git-trailer-default-config have a\n> fallthru call to git-default-config instead?\n\nThanks, I was going to say the same thing.\n\nAlso, would it be possible to add a test?\n"}]}