{"thread":{"id":"63699","subject":"[GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","startedAt":"2025-06-26T13:23:36Z","lastAt":"2025-07-16T15:29:57Z","messageCount":41,"participants":["Ayush Chandekar","Junio C Hamano","Kristoffer Haugsbakk","Phillip Wood","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"520758","messageId":"20250626132233.414789-1-ayu.chandekar@gmail.com","threadId":"63699","inReplyTo":null,"subject":"[GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-06-26T13:22:33Z","receivedAt":"2025-06-26T13:23:36Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"When core.commentChar is set to \"auto\", Git selects a comment character\nby scanning the commit message contents and avoiding any character\nalready present in the message.\n\nIf the message still contains old conflict comments (starting with a\ncomment character), Git assumes that character is in use and chooses a\ndifferent one. As a result, those existing comment lines are no longer\nrecognized as comments and end up being included in the final commit\nmessage.\n\nTo avoid this, skip scanning the trailing comment block when selecting\nthe comment character. This allows Git to safely reuse the original\ncharacter when appropriate, keeping the commit message clean and free of\nleftover conflict information.\n\nBackground:\n\nThe \"auto\" value for core.commentchar was introduced in the commit\n`84c9dc2` (commit: allow core.commentChar=auto for character auto\nselection) but did not exhibt this issue at that time.\n\nThe bug was introduced in commit `a6c2654` (rebase -m: fix --signoff\nwith conflicts) where Git started writing conflict comments to the file\nat 'rebase_path_message()'.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n\nI came across this bug when working on a patch series that removes \nthe global variables related to the \"commentChar\" config options.\n\n builtin/commit.c | 6 +++++-\n t/t3418-rebase-continue.sh | 18 ++++++++++++++++++\n 2 files changed, 23 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex fba0dded64..63e7158e98 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tchar candidates[] = \"#;@!$%^&|:\";\n \tchar *candidate;\n \tconst char *p;\n+\tsize_t cutoff;\n+\n+\t/* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n+\tcutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n \n \tif (!memchr(sb->buf, candidates[0], sb->len)) {\n \t\tfree(comment_line_str_to_free);\n@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tcandidate = strchr(candidates, *p);\n \tif (candidate)\n \t\t*candidate = ' ';\n-\tfor (p = sb->buf; *p; p++) {\n+\tfor (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {\n \t\tif ((p[0] == '\\n' || p[0] == '\\r') && p[1]) {\n \t\t\tcandidate = strchr(candidates, p[1]);\n \t\t\tif (candidate)\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 127216f722..a8e89a250b 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -328,6 +328,24 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas\n \ttest_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec\n '\n \n+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n+\ttest_commit base file &&\n+\tgit checkout -b branch-a &&\n+\ttest_commit A file &&\n+\tgit checkout -b branch-b base &&\n+\ttest_commit B file &&\n+\ttest_must_fail git rebase branch-a &&\n+\tprintf \"B\\nA\\n\" >file &&\n+\tgit add file &&\n+\twrite_script fake-editor <<-\\EOF &&\n+\texit 0\n+\tEOF\n+\tFAKE_EDITOR=\"$(pwd)/fake-editor\" &&\n+\tGIT_EDITOR=\"\\\"\\$FAKE_EDITOR\\\"\" git -c core.commentChar=auto rebase --continue &&\n+\t# Check that \"#\" is still the comment character.\n+\ttest_grep \"# Changes to be committed:\" .git/COMMIT_EDITMSG\n+'\n+\n test_orig_head_helper () {\n \ttest_when_finished 'git rebase --abort &&\n \t\tgit checkout topic &&\n-- \n2.49.0\n\n"},{"id":"520762","messageId":"xmqq5xgir2ry.fsf@gitster.g","threadId":"63699","inReplyTo":"20250626132233.414789-1-ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-26T14:33:21Z","receivedAt":"2025-06-26T14:33:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar writes:\n\n> When core.commentChar is set to \"auto\", Git selects a comment character\n> by scanning the commit message contents and avoiding any character\n> already present in the message.\n>\n> If the message still contains old conflict comments (starting with a\n> comment character), Git assumes that character is in use and chooses a\n> different one. As a result, those existing comment lines are no longer\n> recognized as comments and end up being included in the final commit\n> message.\n>\n> To avoid this, skip scanning the trailing comment block when selecting\n> the comment character. This allows Git to safely reuse the original\n> character when appropriate, keeping the commit message clean and free of\n> leftover conflict information.\n>\n> Background:\n>\n> The \"auto\" value for core.commentchar was introduced in the commit\n> `84c9dc2` (commit: allow core.commentChar=auto for character auto\n> selection) but did not exhibt this issue at that time.\n\nUse \"git log -1 --format=reference\", i.e.\n\n84c9dc2c (commit: allow core.commentChar=auto for character auto\nselection, 2014-05-17)\n\n> The bug was introduced in commit `a6c2654` (rebase -m: fix --signoff\n> with conflicts) where Git started writing conflict comments to the file\n> at 'rebase_path_message()'.\n\nDitto.\n\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index fba0dded64..63e7158e98 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n> \tchar candidates[] = \"#;@!$%^&|:\";\n> \tchar *candidate;\n> \tconst char *p;\n> +\tsize_t cutoff;\n> +\n> +\t/* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n> +\tcutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n> \n> \tif (!memchr(sb->buf, candidates[0], sb->len)) {\n> \t\tfree(comment_line_str_to_free);\n> @@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n> \tcandidate = strchr(candidates, *p);\n> \tif (candidate)\n> \t\t*candidate = ' ';\n> -\tfor (p = sb->buf; *p; p++) {\n> +\tfor (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {\n> \t\tif ((p[0] == '\\n' || p[0] == '\\r') && p[1]) {\n> \t\t\tcandidate = strchr(candidates, p[1]);\n> \t\t\tif (candidate)\n\nLooks quite straight-forward. Nice.\n\nThanks.\n"},{"id":"520766","messageId":"ca8e7670-cf4f-4915-a37f-09d2e4b7c62a@app.fastmail.com","threadId":"63699","inReplyTo":"20250626132233.414789-1-ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-06-26T15:40:12Z","receivedAt":"2025-06-26T15:40:38Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Thu, Jun 26, 2025, at 15:22, Ayush Chandekar wrote:\n> When core.commentChar is set to \"auto\", Git selects a comment character\n> by scanning the commit message contents and avoiding any character\n> already present in the message.\n>\n> If the message still contains old conflict comments (starting with a\n> comment character), Git assumes that character is in use and chooses a\n> different one. As a result, those existing comment lines are no longer\n> recognized as comments and end up being included in the final commit\n> message.\n>\n> To avoid this, skip scanning the trailing comment block when selecting\n> the comment character. This allows Git to safely reuse the original\n> character when appropriate, keeping the commit message clean and free of\n> leftover conflict information.\n>\n> Background:\n>\n> The \"auto\" value for core.commentchar was introduced in the commit\n> `84c9dc2` (commit: allow core.commentChar=auto for character auto\n> selection) but did not exhibt this issue at that time.\n>\n> The bug was introduced in commit `a6c2654` (rebase -m: fix --signoff\n> with conflicts) where Git started writing conflict comments to the file\n> at 'rebase_path_message()'.\n>\n> Mentored-by: Christian Couder \n> Mentored-by: Ghanshyam Thakkar \n> Signed-off-by: Ayush Chandekar \n> ---\n\nNice explanation.\n\n> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> index 127216f722..a8e89a250b 100755\n> --- a/t/t3418-rebase-continue.sh\n> +++ b/t/t3418-rebase-continue.sh\n> @@ -328,6 +328,24 @@ test_expect_success 'there is no\n> --no-reschedule-failed-exec in an ongoing rebas\n> \ttest_expect_code 129 git rebase --edit-todo\n> --no-reschedule-failed-exec\n> '\n>\n> +test_expect_success 'no change in comment character due to conflicts\n> markers with core.commentChar=auto' '\n> +\ttest_commit base file &&\n> +\tgit checkout -b branch-a &&\n> +\ttest_commit A file &&\n> +\tgit checkout -b branch-b base &&\n> +\ttest_commit B file &&\n> +\ttest_must_fail git rebase branch-a &&\n> +\tprintf \"B\\nA\\n\" >file &&\n> +\tgit add file &&\n> +\twrite_script fake-editor <<-\\EOF &&\n> +\texit 0\n> +\tEOF\n> +\tFAKE_EDITOR=\"$(pwd)/fake-editor\" &&\n> +\tGIT_EDITOR=\"\\\"\\$FAKE_EDITOR\\\"\" git -c core.commentChar=auto rebase --continue &&\n\nHow about\n\n GIT_EDITOR=\"cat >actual\"\n\nThen you can `test_grep` on that. Like in\n\nhttps://lore.kernel.org/git/5ed77fab-678d-4a06-bbd0-ea25462a7562@gmail.com/\n\n> +\t# Check that \"#\" is still the comment character.\n> +\ttest_grep \"# Changes to be committed:\" .git/COMMIT_EDITMSG\n\nNit:\n\n test_grep \"^# Changes to be committed:$\"\n\n> +'\n> +\n> test_orig_head_helper () {\n> \ttest_when_finished 'git rebase --abort &&\n> \t\tgit checkout topic &&\n> --\n> 2.49.0\n"},{"id":"520786","messageId":"CAE7as+aSG0BKeGDFs_GnHjo7juTv1jhKzRgTKGeoH+2X_-O=CA@mail.gmail.com","threadId":"63699","inReplyTo":"ca8e7670-cf4f-4915-a37f-09d2e4b7c62a@app.fastmail.com","subject":"Re: [GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-06-26T21:28:21Z","receivedAt":"2025-06-26T21:28:33Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Thu, Jun 26, 2025 at 9:10 PM Kristoffer Haugsbakk\n wrote:\n\n> >\n> > +test_expect_success 'no change in comment character due to conflicts\n> > markers with core.commentChar=auto' '\n> > + test_commit base file &&\n> > + git checkout -b branch-a &&\n> > + test_commit A file &&\n> > + git checkout -b branch-b base &&\n> > + test_commit B file &&\n> > + test_must_fail git rebase branch-a &&\n> > + printf \"B\\nA\\n\" >file &&\n> > + git add file &&\n> > + write_script fake-editor <<-\\EOF &&\n> > + exit 0\n> > + EOF\n> > + FAKE_EDITOR=\"$(pwd)/fake-editor\" &&\n> > + GIT_EDITOR=\"\\\"\\$FAKE_EDITOR\\\"\" git -c core.commentChar=auto rebase --continue &&\n>\n> How about\n>\n> GIT_EDITOR=\"cat >actual\"\n>\n> Then you can `test_grep` on that. Like in\n>\n> https://lore.kernel.org/git/5ed77fab-678d-4a06-bbd0-ea25462a7562@gmail.com/\n>\n> > + # Check that \"#\" is still the comment character.\n> > + test_grep \"# Changes to be committed:\" .git/COMMIT_EDITMSG\n>\n\nThanks, that's much cleaner and faster!\n\n> Nit:\n>\n> test_grep \"^# Changes to be committed:$\"\n>\nThanks for the catch. I'll fix it.\n"},{"id":"520787","messageId":"CAE7as+b=9sKLU1pG4xDJ+D4C=UNYUH2cpP13VaqwLfsQmLUVQQ@mail.gmail.com","threadId":"63699","inReplyTo":"xmqq5xgir2ry.fsf@gitster.g","subject":"Re: [GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-06-26T21:30:26Z","receivedAt":"2025-06-26T21:30:38Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Thu, Jun 26, 2025 at 8:03 PM Junio C Hamano wrote:\n>\n> Ayush Chandekar writes:\n>\n> > When core.commentChar is set to \"auto\", Git selects a comment character\n> > by scanning the commit message contents and avoiding any character\n> > already present in the message.\n> >\n> > If the message still contains old conflict comments (starting with a\n> > comment character), Git assumes that character is in use and chooses a\n> > different one. As a result, those existing comment lines are no longer\n> > recognized as comments and end up being included in the final commit\n> > message.\n> >\n> > To avoid this, skip scanning the trailing comment block when selecting\n> > the comment character. This allows Git to safely reuse the original\n> > character when appropriate, keeping the commit message clean and free of\n> > leftover conflict information.\n> >\n> > Background:\n> >\n> > The \"auto\" value for core.commentchar was introduced in the commit\n> > `84c9dc2` (commit: allow core.commentChar=auto for character auto\n> > selection) but did not exhibt this issue at that time.\n>\n> Use \"git log -1 --format=reference\", i.e.\n>\n> 84c9dc2c (commit: allow core.commentChar=auto for character auto\n> selection, 2014-05-17)\n>\nGot it, Thanks! I'll update it.\n"},{"id":"520788","messageId":"20250626221631.457725-1-ayu.chandekar@gmail.com","threadId":"63699","inReplyTo":"20250626132233.414789-1-ayu.chandekar@gmail.com","subject":"[GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-06-26T22:16:31Z","receivedAt":"2025-06-26T22:17:27Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"When core.commentChar is set to \"auto\", Git selects a comment character\nby scanning the commit message contents and avoiding any character\nalready present in the message.\n\nIf the message still contains old conflict comments (starting with a\ncomment character), Git assumes that character is in use and chooses a\ndifferent one. As a result, those existing comment lines are no longer\nrecognized as comments and end up being included in the final commit\nmessage.\n\nTo avoid this, skip scanning the trailing comment block when selecting\nthe comment character. This allows Git to safely reuse the original\ncharacter when appropriate, keeping the commit message clean and free of\nleftover conflict information.\n\nBackground:\n\nThe \"auto\" value for core.commentchar was introduced in the commit\n84c9dc2c5a (commit: allow core.commentChar=auto for character auto\nselection, 2014-05-17) but did not exhibt this issue at that time.\n\nThe bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff\nwith conflicts, 2024-04-18) where Git started writing conflict comments\nto the file at 'rebase_path_message()'.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n\nThanks to Christian for mentoring, and to Kristopher and Junio for their reviews!\n\n builtin/commit.c | 6 +++++-\n t/t3418-rebase-continue.sh | 14 ++++++++++++++\n 2 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex fba0dded64..63e7158e98 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tchar candidates[] = \"#;@!$%^&|:\";\n \tchar *candidate;\n \tconst char *p;\n+\tsize_t cutoff;\n+\n+\t/* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n+\tcutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n \n \tif (!memchr(sb->buf, candidates[0], sb->len)) {\n \t\tfree(comment_line_str_to_free);\n@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tcandidate = strchr(candidates, *p);\n \tif (candidate)\n \t\t*candidate = ' ';\n-\tfor (p = sb->buf; *p; p++) {\n+\tfor (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {\n \t\tif ((p[0] == '\\n' || p[0] == '\\r') && p[1]) {\n \t\t\tcandidate = strchr(candidates, p[1]);\n \t\t\tif (candidate)\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 127216f722..ccfe77af6c 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -328,6 +328,20 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas\n \ttest_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec\n '\n \n+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n+\ttest_commit base file &&\n+\tgit checkout -b branch-a &&\n+\ttest_commit A file &&\n+\tgit checkout -b branch-b base &&\n+\ttest_commit B file &&\n+\ttest_must_fail git rebase branch-a &&\n+\tprintf \"B\\nA\\n\" >file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"cat >actual\" git -c core.commentChar=auto rebase --continue &&\n+\t# Check that \"#\" is still the comment character.\n+\ttest_grep \"^# Changes to be committed:$\" actual\n+'\n+\n test_orig_head_helper () {\n \ttest_when_finished 'git rebase --abort &&\n \t\tgit checkout topic &&\n-- \n2.49.0\n\n"},{"id":"520799","messageId":"91982162-b138-4bb1-81fd-6f9185801c99@gmail.com","threadId":"63699","inReplyTo":"20250626221631.457725-1-ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-06-27T08:34:30Z","receivedAt":"2025-06-27T08:34:33Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 26/06/2025 23:16, Ayush Chandekar wrote:\n> When core.commentChar is set to \"auto\", Git selects a comment character\n> by scanning the commit message contents and avoiding any character\n> already present in the message.\n> \n> If the message still contains old conflict comments (starting with a\n> comment character), Git assumes that character is in use and chooses a\n> different one. As a result, those existing comment lines are no longer\n> recognized as comments and end up being included in the final commit\n> message.\n> \n> To avoid this, skip scanning the trailing comment block when selecting\n> the comment character. This allows Git to safely reuse the original\n> character when appropriate, keeping the commit message clean and free of\n> leftover conflict information.\n\nThis is a good explanation of the problem. Maybe this is another reason \nto consider removing support for commentChar=auto [1]\n\n[1] https://lore.kernel.org/git/xmqqa59i45wc.fsf@gitster.g/\n\n> Background:\n> \n> The \"auto\" value for core.commentchar was introduced in the commit\n> 84c9dc2c5a (commit: allow core.commentChar=auto for character auto\n> selection, 2014-05-17) but did not exhibt this issue at that time.\n> \n> The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff\n> with conflicts, 2024-04-18) where Git started writing conflict comments\n> to the file at 'rebase_path_message()'.\n> \n> Mentored-by: Christian Couder \n> Mentored-by: Ghanshyam Thakkar \n> Signed-off-by: Ayush Chandekar \n> ---\n> \n> Thanks to Christian for mentoring, and to Kristopher and Junio for their reviews!\n> \n> builtin/commit.c | 6 +++++-\n> t/t3418-rebase-continue.sh | 14 ++++++++++++++\n> 2 files changed, 19 insertions(+), 1 deletion(-)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index fba0dded64..63e7158e98 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n> \tchar candidates[] = \"#;@!$%^&|:\";\n> \tchar *candidate;\n> \tconst char *p;\n> +\tsize_t cutoff;\n> +\n> +\t/* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n> +\tcutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n\nThis finds the \"Conflicts:\" line. I was surprised to see that the string \nit looks for is hard coded and not translated, however the sequencer \n(also surprisingly) does not translate that message either so it should \nwork.\n\n> \n> \tif (!memchr(sb->buf, candidates[0], sb->len)) {\n> \t\tfree(comment_line_str_to_free);\n> @@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n> \tcandidate = strchr(candidates, *p);\n> \tif (candidate)\n> \t\t*candidate = ' ';\n> -\tfor (p = sb->buf; *p; p++) {\n> +\tfor (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {\n> \t\tif ((p[0] == '\\n' || p[0] == '\\r') && p[1]) {\n> \t\t\tcandidate = strchr(candidates, p[1]);\n> \t\t\tif (candidate)\n> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> index 127216f722..ccfe77af6c 100755\n> --- a/t/t3418-rebase-continue.sh\n> +++ b/t/t3418-rebase-continue.sh\n> @@ -328,6 +328,20 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas\n> \ttest_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec\n> '\n> \n> +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n> +\ttest_commit base file &&\n\nIf you used an existing file (F1 or F2) like most of the rest of the \ntests in this file we could avoid creating this commit and save \nourselves a couple of processes.\n\n> +\tgit checkout -b branch-a &&\n> +\ttest_commit A file &&\n> +\tgit checkout -b branch-b base &&\n> +\ttest_commit B file &&\n> +\ttest_must_fail git rebase branch-a &&\n> +\tprintf \"B\\nA\\n\" >file &&\n> +\tgit add file &&\n> +\tGIT_EDITOR=\"cat >actual\" git -c core.commentChar=auto rebase --continue &&\n> +\t# Check that \"#\" is still the comment character.\n> +\ttest_grep \"^# Changes to be committed:$\" actual\n\nI agree that it is a good idea to anchor the start of the message, but \nI'm not sure it is helpful to anchor the end of the message as we don't \nwant the test to fail just because an unrelated change adds some \nwhitespace to the end of this line. I'd be tempted to drop the ':' for \nthe same reason.\n\nThanks for fixing this\n\nPhillip\n\n> +'\n> +\n> test_orig_head_helper () {\n> \ttest_when_finished 'git rebase --abort &&\n> \t\tgit checkout topic &&\n\n"},{"id":"520804","messageId":"CAP8UFD1nCVGCK-PMzRzqFqp9WEDbTtpaSOzpCZrL-74wmUA2kw@mail.gmail.com","threadId":"63699","inReplyTo":"20250626221631.457725-1-ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-06-27T09:04:59Z","receivedAt":"2025-06-27T09:05:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Jun 27, 2025 at 12:17 AM Ayush Chandekar\n wrote:\n\n> The \"auto\" value for core.commentchar was introduced in the commit\n> 84c9dc2c5a (commit: allow core.commentChar=auto for character auto\n> selection, 2014-05-17) but did not exhibt this issue at that time.\n\nNit: s/exhibt/exhibit/\n\nThanks!\n"},{"id":"520810","messageId":"xmqqms9t8cfd.fsf@gitster.g","threadId":"63699","inReplyTo":"91982162-b138-4bb1-81fd-6f9185801c99@gmail.com","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-27T14:52:06Z","receivedAt":"2025-06-27T14:52:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>> +\tsize_t cutoff;\n>> +\n>> +\t/* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n>> +\tcutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n>\n> This finds the \"Conflicts:\" line. I was surprised to see that the\n> string it looks for is hard coded and not translated, however the\n> sequencer (also surprisingly) does not translate that message either\n> so it should work.\n\nThere is a funny chicken-and-egg problem, though. It limits the\nsearch for \"Conflicts\" by using wt_status_locate_end() based on the\ncurrent value of comment_line_str. When core.commentstring is set\nto \"auto\", the code that reads the configuration does not touch the\ncomment_line_str variable, which is initialized to '#'. So\n\n\t[core]\n\t commentstring = '%'\n\t commentstring = auto\n\nwould have '%' in comment_line_str upon entering this codepath, let\nwt_status_locate_end() use '%' as the comment string to find the end\nof the log message, and then looks for \"Conflicts:\" in the result.\n\nWhich may or may not be what you want.\n\n> If you used an existing file (F1 or F2) like most of the rest of the\n> tests in this file we could avoid creating this commit and save\n> ourselves a couple of processes.\n\nExcellent suggestion.\n\n>> +\ttest_grep \"^# Changes to be committed:$\" actual\n>\n> I agree that it is a good idea to anchor the start of the message, but\n> I'm not sure it is helpful to anchor the end of the message as we\n> don't want the test to fail just because an unrelated change adds some\n> whitespace to the end of this line. I'd be tempted to drop the ':' for\n> the same reason.\n\nAgain, excellent.\n\n\n> Thanks for fixing this\n>\n> Phillip\n\nThanks.\n"},{"id":"520842","messageId":"CAE7as+acyM4G0wHmxY3AWX9i0pSWa_C-_d3LFxXezrmkSNNsbg@mail.gmail.com","threadId":"63699","inReplyTo":"91982162-b138-4bb1-81fd-6f9185801c99@gmail.com","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-06-28T10:18:13Z","receivedAt":"2025-06-28T10:18:25Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Fri, Jun 27, 2025 at 2:04 PM Phillip Wood wrote:\n> >\n> > if (!memchr(sb->buf, candidates[0], sb->len)) {\n> > free(comment_line_str_to_free);\n> > @@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n> > candidate = strchr(candidates, *p);\n> > if (candidate)\n> > *candidate = ' ';\n> > - for (p = sb->buf; *p; p++) {\n> > + for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {\n> > if ((p[0] == '\\n' || p[0] == '\\r') && p[1]) {\n> > candidate = strchr(candidates, p[1]);\n> > if (candidate)\n> > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> > index 127216f722..ccfe77af6c 100755\n> > --- a/t/t3418-rebase-continue.sh\n> > +++ b/t/t3418-rebase-continue.sh\n> > @@ -328,6 +328,20 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas\n> > test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec\n> > '\n> >\n> > +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n> > + test_commit base file &&\n>\n> If you used an existing file (F1 or F2) like most of the rest of the\n> tests in this file we could avoid creating this commit and save\n> ourselves a couple of processes.\n>\n\nYeah, right, I'll update it.\n\n> > + git checkout -b branch-a &&\n> > + test_commit A file &&\n> > + git checkout -b branch-b base &&\n> > + test_commit B file &&\n> > + test_must_fail git rebase branch-a &&\n> > + printf \"B\\nA\\n\" >file &&\n> > + git add file &&\n> > + GIT_EDITOR=\"cat >actual\" git -c core.commentChar=auto rebase --continue &&\n> > + # Check that \"#\" is still the comment character.\n> > + test_grep \"^# Changes to be committed:$\" actual\n>\n> I agree that it is a good idea to anchor the start of the message, but\n> I'm not sure it is helpful to anchor the end of the message as we don't\n> want the test to fail just because an unrelated change adds some\n> whitespace to the end of this line. I'd be tempted to drop the ':' for\n> the same reason.\n>\n\nMakes sense, I'll fix it.\nThanks a lot for reviewing!\n"},{"id":"520843","messageId":"CAE7as+ZBOJ_4LvHQua9bOw7+Y8cMpdo-sf8hThSkFC4rWCEt9g@mail.gmail.com","threadId":"63699","inReplyTo":"xmqqms9t8cfd.fsf@gitster.g","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-06-28T10:37:15Z","receivedAt":"2025-06-28T10:37:27Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Fri, Jun 27, 2025 at 8:22 PM Junio C Hamano wrote:\n>\n> Phillip Wood writes:\n>\n> >> + size_t cutoff;\n> >> +\n> >> + /* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n> >> + cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n> >\n> > This finds the \"Conflicts:\" line. I was surprised to see that the\n> > string it looks for is hard coded and not translated, however the\n> > sequencer (also surprisingly) does not translate that message either\n> > so it should work.\n>\n> There is a funny chicken-and-egg problem, though. It limits the\n> search for \"Conflicts\" by using wt_status_locate_end() based on the\n> current value of comment_line_str. When core.commentstring is set\n> to \"auto\", the code that reads the configuration does not touch the\n> comment_line_str variable, which is initialized to '#'. So\n>\n> [core]\n> commentstring = '%'\n> commentstring = auto\n>\n> would have '%' in comment_line_str upon entering this codepath, let\n> wt_status_locate_end() use '%' as the comment string to find the end\n> of the log message, and then looks for \"Conflicts:\" in the result.\n>\n> Which may or may not be what you want.\n>\n\nThis is also being used to append signoff, just before the\n`adjust_comment_line_char()` function.\nAnother thing that could be done is to return the function\n(adjust_comment_line_char()) when we find a \"Conflicts:\" line. Because\nI don't think there's sense in adjusting the comment character when\nthe \"Conflicts:\" line already has a comment character. But, I would\nlike to have some views on this.\n\nThanks.\n"},{"id":"520846","messageId":"f39a3285-574a-45c6-9646-04eb175f4770@gmail.com","threadId":"63699","inReplyTo":"xmqqms9t8cfd.fsf@gitster.g","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-06-28T13:38:01Z","receivedAt":"2025-06-28T13:38:04Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 27/06/2025 15:52, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>>> +\tsize_t cutoff;\n>>> +\n>>> +\t/* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n>>> +\tcutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n>>\n>> This finds the \"Conflicts:\" line. I was surprised to see that the\n>> string it looks for is hard coded and not translated, however the\n>> sequencer (also surprisingly) does not translate that message either\n>> so it should work.\n> \n> There is a funny chicken-and-egg problem, though. It limits the\n> search for \"Conflicts\" by using wt_status_locate_end() based on the\n> current value of comment_line_str. When core.commentstring is set\n> to \"auto\", the code that reads the configuration does not touch the\n> comment_line_str variable, which is initialized to '#'. So\n> \n> \t[core]\n> \t commentstring = '%'\n> \t commentstring = auto\n> \n> would have '%' in comment_line_str upon entering this codepath, let\n> wt_status_locate_end() use '%' as the comment string to find the end\n> of the log message, and then looks for \"Conflicts:\" in the result.\n> \n> Which may or may not be what you want.\n\nOh, good point - I'd not looked at the config parsing. So we'd create \nconflict comments that look like\n\n % Conflicts:\n % some-file.c\n\nbut so long as the commit message did not contain a '#' character [1] \n\"git commit\" would select '#' as the automatic comment char and our \nconflicts lines would not be treated as a comment.\n\nShould we be resetting comment_line_str to '#' when core.commentString \nis set to \"auto\"? That wont help if the commit message contains a '#' \nbut at least it would be consistently broken.\n\nWe could move adjust_comment_line_char() into libgit.a, use that to \nselect the comment character used in append_conflicts_hint() and set \ncore.commentString to that character when we run \"git commit\". I think \ndoing that would mean that appending conflict comments would always work \nwith core.commentString=auto but it is a more complex solution as we \nwould need to remember the comment character and then pass it to git \ncommit once the user had fixed the conflicts.\n\nOne final note - although the commit message mentions a change to \"git \nrebase\" I think this problem already affected cherry-pick, revert and \nmerge before that change. In practice I suspect it is only cherry-pick \nwhere one is likely to see this problem because the template messages \nfor the other two commands are unlikely to contain a '#'.\n\nThanks\n\nPhillip\n\n[1] For some reason adjust_comment_line_char() will not select '#' as \nthe comment char if it occurs anywhere in the message but the other \ncandidates are selected so long as they are not the first character on \nany line.\n"},{"id":"520847","messageId":"CAE7as+aUcd65vPwwRh_C89vQbMjKQh0Y6LF7WDq1Whyj6iYfLg@mail.gmail.com","threadId":"63699","inReplyTo":"f39a3285-574a-45c6-9646-04eb175f4770@gmail.com","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-06-28T14:33:12Z","receivedAt":"2025-06-28T14:33:24Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Sat, Jun 28, 2025 at 7:08 PM Phillip Wood wrote:\n>\n> On 27/06/2025 15:52, Junio C Hamano wrote:\n> > Phillip Wood writes:\n> >\n> >>> + size_t cutoff;\n> >>> +\n> >>> + /* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n> >>> + cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n> >>\n> >> This finds the \"Conflicts:\" line. I was surprised to see that the\n> >> string it looks for is hard coded and not translated, however the\n> >> sequencer (also surprisingly) does not translate that message either\n> >> so it should work.\n> >\n> > There is a funny chicken-and-egg problem, though. It limits the\n> > search for \"Conflicts\" by using wt_status_locate_end() based on the\n> > current value of comment_line_str. When core.commentstring is set\n> > to \"auto\", the code that reads the configuration does not touch the\n> > comment_line_str variable, which is initialized to '#'. So\n> >\n> > [core]\n> > commentstring = '%'\n> > commentstring = auto\n> >\n> > would have '%' in comment_line_str upon entering this codepath, let\n> > wt_status_locate_end() use '%' as the comment string to find the end\n> > of the log message, and then looks for \"Conflicts:\" in the result.\n> >\n> > Which may or may not be what you want.\n>\n> Oh, good point - I'd not looked at the config parsing. So we'd create\n> conflict comments that look like\n>\n> % Conflicts:\n> % some-file.c\n>\n> but so long as the commit message did not contain a '#' character [1]\n> \"git commit\" would select '#' as the automatic comment char and our\n> conflicts lines would not be treated as a comment.\n>\n> Should we be resetting comment_line_str to '#' when core.commentString\n> is set to \"auto\"? That wont help if the commit message contains a '#'\n> but at least it would be consistently broken.\n>\n> We could move adjust_comment_line_char() into libgit.a, use that to\n> select the comment character used in append_conflicts_hint() and set\n> core.commentString to that character when we run \"git commit\". I think\n> doing that would mean that appending conflict comments would always work\n> with core.commentString=auto but it is a more complex solution as we\n> would need to remember the comment character and then pass it to git\n> commit once the user had fixed the conflicts.\n>\n\nSo, my GSoC project is refactoring in order to reduce the global state\nin Git. I was trying to remove the global variables related to comment\ncharacters. What I tried is to create one single function which\nreturns the comment string, and we could then pass a strbuf in case of\ncore.commentString=auto. You can check my attempts on my fork here [2]\n(check repo_get_comment_line_str() in config.c), and also mentioned\nthis in my blog [3]. I thought I had it figured out, but turns out I\nfailed one test where core.commentString=auto. It was that moment I\nrealised that I would need to remember the comment character or the\nstrbuf in functions. Just wanted to share this in case anything\nstrikes you when looking at the approach.\n\n> One final note - although the commit message mentions a change to \"git\n> rebase\" I think this problem already affected cherry-pick, revert and\n> merge before that change. In practice I suspect it is only cherry-pick\n> where one is likely to see this problem because the template messages\n> for the other two commands are unlikely to contain a '#'.\n>\n> Thanks\n>\n> Phillip\n>\n> [1] For some reason adjust_comment_line_char() will not select '#' as\n> the comment char if it occurs anywhere in the message but the other\n> candidates are selected so long as they are not the first character on\n> any line.\n\nThanks:)\n\n[2] : https://github.com/ayu-ch/git/commits/comment-line-str-4\n[3] : https://ayu-ch.github.io/2025/06/09/gsoc-week-1.html\n"},{"id":"520848","messageId":"e1e31f27-4c17-4a48-b46d-47c56d2d290f@gmail.com","threadId":"63699","inReplyTo":"f39a3285-574a-45c6-9646-04eb175f4770@gmail.com","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-06-28T15:10:54Z","receivedAt":"2025-06-28T15:10:58Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 28/06/2025 14:38, Phillip Wood wrote:\n> [1] For some reason adjust_comment_line_char() will not select '#' as \n> the comment char if it occurs anywhere in the message but the other \n> candidates are selected so long as they are not the first character on \n> any line.\n\nSorry, forget that - I'd misread the code\n"},{"id":"520907","messageId":"d25bf6c5-e56c-48d4-95b6-c714ee14ab78@gmail.com","threadId":"63699","inReplyTo":"CAE7as+aUcd65vPwwRh_C89vQbMjKQh0Y6LF7WDq1Whyj6iYfLg@mail.gmail.com","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-06-30T08:59:33Z","receivedAt":"2025-06-30T08:59:39Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 28/06/2025 15:33, Ayush Chandekar wrote:\n> \n> So, my GSoC project is refactoring in order to reduce the global state\n> in Git. I was trying to remove the global variables related to comment\n> characters. What I tried is to create one single function which\n> returns the comment string, and we could then pass a strbuf in case of\n> core.commentString=auto. You can check my attempts on my fork here [2]\n> (check repo_get_comment_line_str() in config.c), and also mentioned\n> this in my blog [3]. I thought I had it figured out, but turns out I\n> failed one test where core.commentString=auto. It was that moment I\n> realised that I would need to remember the comment character or the\n> strbuf in functions. Just wanted to share this in case anything\n> strikes you when looking at the approach.\n\nThanks for that context. I'm not sure about having a single function \nthat handles both cases. There is only one caller that cares about \n\"auto\" and the support for that has so many corner cases that don't work \nI'm putting a patch together to deprecate it and remove it when Git 3.0 \nis released.\n\nLooking at your code it seems to break the \"last one wins\" between \ncore.commentChar and core.commentString. Also deferring the parsing \nuntil the comment character is used changes the behavior of things like \n\"git -c core.commentchar=$'\\n' commit -p\". Instead of erroring out \nstraight away it will let the user carefully select what they want to \ncommit and then die which is not very user friendly.\n\nKeeping the current parsing logic and storing the result in struct \nrepository might be a better approach though we should think about how \ncommands that run without a repository will be able to access the system \nand user config settings.\n\nThanks\n\nPhillip\n\n"},{"id":"520919","messageId":"xmqqjz4t5ngj.fsf@gitster.g","threadId":"63699","inReplyTo":"f39a3285-574a-45c6-9646-04eb175f4770@gmail.com","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-30T14:11:08Z","receivedAt":"2025-06-30T14:11:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Should we be resetting comment_line_str to '#' when core.commentString\n> is set to \"auto\"? That wont help if the commit message contains a '#'\n> but at least it would be consistently broken.\n\nYeah, while I was re-reading the code to parse the configuration file,\nthat was exactly what came to my mind. I offhand did not think of any\ndownside of doing so, but I cannot claim that I have spent enough brain\ncycles to make sure it is free of bad unintended consequences.\n\nThanks.\n"},{"id":"520940","messageId":"CAE7as+Zaixy460a07G935JXt03XQftf7y8YixrPoOw8akNW=1A@mail.gmail.com","threadId":"63699","inReplyTo":"d25bf6c5-e56c-48d4-95b6-c714ee14ab78@gmail.com","subject":"Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-06-30T17:34:50Z","receivedAt":"2025-06-30T17:35:01Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Mon, Jun 30, 2025 at 2:29 PM Phillip Wood wrote:\n>\n> Hi Ayush\n>\n> On 28/06/2025 15:33, Ayush Chandekar wrote:\n> >\n> > So, my GSoC project is refactoring in order to reduce the global state\n> > in Git. I was trying to remove the global variables related to comment\n> > characters. What I tried is to create one single function which\n> > returns the comment string, and we could then pass a strbuf in case of\n> > core.commentString=auto. You can check my attempts on my fork here [2]\n> > (check repo_get_comment_line_str() in config.c), and also mentioned\n> > this in my blog [3]. I thought I had it figured out, but turns out I\n> > failed one test where core.commentString=auto. It was that moment I\n> > realised that I would need to remember the comment character or the\n> > strbuf in functions. Just wanted to share this in case anything\n> > strikes you when looking at the approach.\n>\n> Thanks for that context. I'm not sure about having a single function\n> that handles both cases. There is only one caller that cares about\n> \"auto\" and the support for that has so many corner cases that don't work\n> I'm putting a patch together to deprecate it and remove it when Git 3.0\n> is released.\n>\n> Looking at your code it seems to break the \"last one wins\" between\n> core.commentChar and core.commentString. Also deferring the parsing\n> until the comment character is used changes the behavior of things like\n> \"git -c core.commentchar=$'\\n' commit -p\". Instead of erroring out\n> straight away it will let the user carefully select what they want to\n> commit and then die which is not very user friendly.\n>\n\nThanks for taking a look! I was experimenting with this to see if I\ncould simplify the logic, but I didn't realize upfront how many corner\ncases it would run into.\n\n> Keeping the current parsing logic and storing the result in struct\n> repository might be a better approach though we should think about how\n> commands that run without a repository will be able to access the system\n> and user config settings.\n>\n\nYeah, I will follow that approach as I did with other patch series.\n\n> Thanks\n>\n> Phillip\n>\n\nThanks a lot!\n\nAyush:)\n"},{"id":"520948","messageId":"20250630182527.69167-1-ayu.chandekar@gmail.com","threadId":"63699","inReplyTo":"20250626132233.414789-1-ayu.chandekar@gmail.com","subject":"[GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-06-30T18:25:27Z","receivedAt":"2025-06-30T18:25:56Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"When core.commentChar is set to \"auto\", Git selects a comment character\nby scanning the commit message contents and avoiding any character\nalready present in the message.\n\nIf the message still contains old conflict comments (starting with a\ncomment character), Git assumes that character is in use and chooses a\ndifferent one. As a result, those existing comment lines are no longer\nrecognized as comments and end up being included in the final commit\nmessage.\n\nTo avoid this, skip scanning the trailing comment block when selecting\nthe comment character. This allows Git to safely reuse the original\ncharacter when appropriate, keeping the commit message clean and free of\nleftover conflict information.\n\nBackground:\n\nThe \"auto\" value for core.commentchar was introduced in the commit\n84c9dc2c5a (commit: allow core.commentChar=auto for character auto\nselection, 2014-05-17) but did not exhibit this issue at that time.\n\nThe bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff\nwith conflicts, 2024-04-18) where Git started writing conflict comments\nto the file at 'rebase_path_message()'.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n\nThanks to Christian, Kristoffer, Junio and Phillip for their reviews!\n\nRange-diff with v2:\n1: 4e74e7a9a6 ! 1: 693f890a36 commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"\n @@ Commit message\n \n The \"auto\" value for core.commentchar was introduced in the commit\n 84c9dc2c5a (commit: allow core.commentChar=auto for character auto\n - selection, 2014-05-17) but did not exhibt this issue at that time.\n + selection, 2014-05-17) but did not exhibit this issue at that time.\n \n The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff\n with conflicts, 2024-04-18) where Git started writing conflict comments\n @@ t/t3418-rebase-continue.sh: test_expect_success 'there is no --no-reschedule-fai\n '\n \n +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n -+\ttest_commit base file &&\n +\tgit checkout -b branch-a &&\n -+\ttest_commit A file &&\n -+\tgit checkout -b branch-b base &&\n -+\ttest_commit B file &&\n ++\ttest_commit A F1 &&\n ++\tgit checkout -b branch-b HEAD^ &&\n ++\ttest_commit B F1 &&\n +\ttest_must_fail git rebase branch-a &&\n -+\tprintf \"B\\nA\\n\" >file &&\n -+\tgit add file &&\n ++\tprintf \"B\\nA\\n\" >F1 &&\n ++\tgit add F1 &&\n +\tGIT_EDITOR=\"cat >actual\" git -c core.commentChar=auto rebase --continue &&\n +\t# Check that \"#\" is still the comment character.\n -+\ttest_grep \"^# Changes to be committed:$\" actual\n ++\ttest_grep \"^# Changes to be committed\" actual\n +'\n +\n test_orig_head_helper () {\n\n\n builtin/commit.c | 6 +++++-\n t/t3418-rebase-continue.sh | 13 +++++++++++++\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex fba0dded64..63e7158e98 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tchar candidates[] = \"#;@!$%^&|:\";\n \tchar *candidate;\n \tconst char *p;\n+\tsize_t cutoff;\n+\n+\t/* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n+\tcutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n \n \tif (!memchr(sb->buf, candidates[0], sb->len)) {\n \t\tfree(comment_line_str_to_free);\n@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tcandidate = strchr(candidates, *p);\n \tif (candidate)\n \t\t*candidate = ' ';\n-\tfor (p = sb->buf; *p; p++) {\n+\tfor (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {\n \t\tif ((p[0] == '\\n' || p[0] == '\\r') && p[1]) {\n \t\t\tcandidate = strchr(candidates, p[1]);\n \t\t\tif (candidate)\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 127216f722..b8a8dd77e7 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -328,6 +328,19 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas\n \ttest_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec\n '\n \n+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n+\tgit checkout -b branch-a &&\n+\ttest_commit A F1 &&\n+\tgit checkout -b branch-b HEAD^ &&\n+\ttest_commit B F1 &&\n+\ttest_must_fail git rebase branch-a &&\n+\tprintf \"B\\nA\\n\" >F1 &&\n+\tgit add F1 &&\n+\tGIT_EDITOR=\"cat >actual\" git -c core.commentChar=auto rebase --continue &&\n+\t# Check that \"#\" is still the comment character.\n+\ttest_grep \"^# Changes to be committed\" actual\n+'\n+\n test_orig_head_helper () {\n \ttest_when_finished 'git rebase --abort &&\n \t\tgit checkout topic &&\n-- \n2.49.0\n\n"},{"id":"521044","messageId":"f22e864e-669d-457c-838e-961bbc977c4b@gmail.com","threadId":"63699","inReplyTo":"20250630182527.69167-1-ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-07-01T13:17:42Z","receivedAt":"2025-07-01T13:17:46Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 30/06/2025 19:25, Ayush Chandekar wrote:\n> \n> Range-diff with v2:\n> 1: 4e74e7a9a6 ! 1: 693f890a36 commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"\n> @@ Commit message\n> \n> The \"auto\" value for core.commentchar was introduced in the commit\n> 84c9dc2c5a (commit: allow core.commentChar=auto for character auto\n> - selection, 2014-05-17) but did not exhibt this issue at that time.\n> + selection, 2014-05-17) but did not exhibit this issue at that time.\n> \n> The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff\n> with conflicts, 2024-04-18) where Git started writing conflict comments\n> @@ t/t3418-rebase-continue.sh: test_expect_success 'there is no --no-reschedule-fai\n> '\n> \n> +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n> -+\ttest_commit base file &&\n> +\tgit checkout -b branch-a &&\n> -+\ttest_commit A file &&\n> -+\tgit checkout -b branch-b base &&\n> -+\ttest_commit B file &&\n> ++\ttest_commit A F1 &&\n> ++\tgit checkout -b branch-b HEAD^ &&\n> ++\ttest_commit B F1 &&\n> +\ttest_must_fail git rebase branch-a &&\n> -+\tprintf \"B\\nA\\n\" >file &&\n> -+\tgit add file &&\n> ++\tprintf \"B\\nA\\n\" >F1 &&\n> ++\tgit add F1 &&\n> +\tGIT_EDITOR=\"cat >actual\" git -c core.commentChar=auto rebase --continue &&\n> +\t# Check that \"#\" is still the comment character.\n> -+\ttest_grep \"^# Changes to be committed:$\" actual\n> ++\ttest_grep \"^# Changes to be committed\" actual\n> +'\n> +\n> test_orig_head_helper () {\n\nThe changes here look good but I think we want to update the config \nparsing as well so that comment_line_str is reset to '#' when \ncore.commentString=auto. We probably want to do that in its own commit.\n\nThanks\n\nPhillip\n\n"},{"id":"521095","messageId":"CAE7as+Z7GXMB4LJGwESK3Pj63ppfFMKDq-xw46YCELJ7E3p+DA@mail.gmail.com","threadId":"63699","inReplyTo":"f22e864e-669d-457c-838e-961bbc977c4b@gmail.com","subject":"Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-01T18:33:54Z","receivedAt":"2025-07-01T18:34:07Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Tue, Jul 1, 2025 at 6:47 PM Phillip Wood wrote:\n>\n> Hi Ayush\n>\n> On 30/06/2025 19:25, Ayush Chandekar wrote:\n> >\n> > Range-diff with v2:\n> > 1: 4e74e7a9a6 ! 1: 693f890a36 commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"\n> > @@ Commit message\n> >\n> > The \"auto\" value for core.commentchar was introduced in the commit\n> > 84c9dc2c5a (commit: allow core.commentChar=auto for character auto\n> > - selection, 2014-05-17) but did not exhibt this issue at that time.\n> > + selection, 2014-05-17) but did not exhibit this issue at that time.\n> >\n> > The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff\n> > with conflicts, 2024-04-18) where Git started writing conflict comments\n> > @@ t/t3418-rebase-continue.sh: test_expect_success 'there is no --no-reschedule-fai\n> > '\n> >\n> > +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n> > -+ test_commit base file &&\n> > + git checkout -b branch-a &&\n> > -+ test_commit A file &&\n> > -+ git checkout -b branch-b base &&\n> > -+ test_commit B file &&\n> > ++ test_commit A F1 &&\n> > ++ git checkout -b branch-b HEAD^ &&\n> > ++ test_commit B F1 &&\n> > + test_must_fail git rebase branch-a &&\n> > -+ printf \"B\\nA\\n\" >file &&\n> > -+ git add file &&\n> > ++ printf \"B\\nA\\n\" >F1 &&\n> > ++ git add F1 &&\n> > + GIT_EDITOR=\"cat >actual\" git -c core.commentChar=auto rebase --continue &&\n> > + # Check that \"#\" is still the comment character.\n> > -+ test_grep \"^# Changes to be committed:$\" actual\n> > ++ test_grep \"^# Changes to be committed\" actual\n> > +'\n> > +\n> > test_orig_head_helper () {\n>\n> The changes here look good but I think we want to update the config\n> parsing as well so that comment_line_str is reset to '#' when\n> core.commentString=auto. We probably want to do that in its own commit.\n>\n> Thanks\n>\n> Phillip\n>\n\nmaybe something like this?\n\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -912,8 +912,10 @@ static int prepare_to_commit(const char\n*index_file, const char *prefix,\n if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\n die_errno(_(\"could not write commit template\"));\n\n- if (auto_comment_line_char)\n+ if (auto_comment_line_char){\n+ comment_line_str = \"#\";\n adjust_comment_line_char(&sb);\n+ }\n strbuf_release(&sb);\n\nor we can do it inside the `adjust_comment_line()` function.\n\nThanks!\n\nAyush\n"},{"id":"521097","messageId":"9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com","threadId":"63699","inReplyTo":"CAE7as+Z7GXMB4LJGwESK3Pj63ppfFMKDq-xw46YCELJ7E3p+DA@mail.gmail.com","subject":"Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-07-01T19:31:58Z","receivedAt":"2025-07-01T19:32:02Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 01/07/2025 19:33, Ayush Chandekar wrote:\n> On Tue, Jul 1, 2025 at 6:47 PM Phillip Wood wrote:\n>>\n>> The changes here look good but I think we want to update the config\n>> parsing as well so that comment_line_str is reset to '#' when\n>> core.commentString=auto. We probably want to do that in its own commit.\n> \n> maybe something like this?\n> \n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -912,8 +912,10 @@ static int prepare_to_commit(const char\n> *index_file, const char *prefix,\n> if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\n> die_errno(_(\"could not write commit template\"));\n> \n> - if (auto_comment_line_char)\n> + if (auto_comment_line_char){\n> + comment_line_str = \"#\";\n> adjust_comment_line_char(&sb);\n> + }\n> strbuf_release(&sb);\n> \n> or we can do it inside the `adjust_comment_line()` function.\n\nWe need to do it when we parse the config so that \nappend_conflicts_comment() uses '#' as the comment char. See the \n(whitespace damaged) diff below\n\nThanks\n\nPhillip\n\ndiff --git a/config.c b/config.c\nindex eb60c293ab3..bb75bdc65d3 100644\n--- a/config.c\n+++ b/config.c\n@@ -1537,9 +1537,11 @@ static int git_default_core_config(const char \n*var, const char *value,\n !strcmp(var, \"core.commentstring\")) {\n if (!value)\n return config_error_nonbool(var);\n- else if (!strcasecmp(value, \"auto\"))\n+ else if (!strcasecmp(value, \"auto\")) {\n auto_comment_line_char = 1;\n- else if (value[0]) {\n+ FREE_AND_NULL(comment_line_str_to_free);\n+ comment_line_str = \"#\";\n+ } else if (value[0]) {\n if (strchr(value, '\\n'))\n return error(_(\"%s cannot contain \nnewline\"), var);\n comment_line_str = value;\n\n"},{"id":"521220","messageId":"CAE7as+abNzqbGSCWsuYe8D_c5dBUuRdDEbHL0pVW5j3kTMER4Q@mail.gmail.com","threadId":"63699","inReplyTo":"9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com","subject":"Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-02T23:46:28Z","receivedAt":"2025-07-02T23:46:40Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Hi Phillip,\n\nOn Wed, Jul 2, 2025 at 1:02 AM Phillip Wood wrote:\n>\n> Hi Ayush\n>\n> On 01/07/2025 19:33, Ayush Chandekar wrote:\n> > On Tue, Jul 1, 2025 at 6:47 PM Phillip Wood wrote:\n> >>\n> >> The changes here look good but I think we want to update the config\n> >> parsing as well so that comment_line_str is reset to '#' when\n> >> core.commentString=auto. We probably want to do that in its own commit.\n> >\n> > maybe something like this?\n> >\n> > --- a/builtin/commit.c\n> > +++ b/builtin/commit.c\n> > @@ -912,8 +912,10 @@ static int prepare_to_commit(const char\n> > *index_file, const char *prefix,\n> > if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\n> > die_errno(_(\"could not write commit template\"));\n> >\n> > - if (auto_comment_line_char)\n> > + if (auto_comment_line_char){\n> > + comment_line_str = \"#\";\n> > adjust_comment_line_char(&sb);\n> > + }\n> > strbuf_release(&sb);\n> >\n> > or we can do it inside the `adjust_comment_line()` function.\n>\n> We need to do it when we parse the config so that\n> append_conflicts_comment() uses '#' as the comment char. See the\n> (whitespace damaged) diff below\n>\n> Thanks\n>\n> Phillip\n>\n> diff --git a/config.c b/config.c\n> index eb60c293ab3..bb75bdc65d3 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char\n> *var, const char *value,\n> !strcmp(var, \"core.commentstring\")) {\n> if (!value)\n> return config_error_nonbool(var);\n> - else if (!strcasecmp(value, \"auto\"))\n> + else if (!strcasecmp(value, \"auto\")) {\n> auto_comment_line_char = 1;\n> - else if (value[0]) {\n> + FREE_AND_NULL(comment_line_str_to_free);\n> + comment_line_str = \"#\";\n> + } else if (value[0]) {\n> if (strchr(value, '\\n'))\n> return error(_(\"%s cannot contain\n> newline\"), var);\n> comment_line_str = value;\n>\n\nThanks, I understood it.\n\nWhat if we simply return the function `adjust_comment_line_char()` if\nwe get a non-zero value from `ignored_log_message_bytes()`, i.e we\nwon't scan the commit message in case conflict message exists, and we\nlet the old code exist as it is?\n\n+ if(ignored_log_message_bytes(sb->buf, sb->len))\n+ return;\n"},{"id":"521298","messageId":"062e7abd-97b1-4806-9753-338906642265@gmail.com","threadId":"63699","inReplyTo":"CAE7as+abNzqbGSCWsuYe8D_c5dBUuRdDEbHL0pVW5j3kTMER4Q@mail.gmail.com","subject":"Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-07-04T08:23:02Z","receivedAt":"2025-07-04T08:23:07Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 03/07/2025 00:46, Ayush Chandekar wrote:\n> On Wed, Jul 2, 2025 at 1:02 AM Phillip Wood wrote:\n>> diff --git a/config.c b/config.c\n>> index eb60c293ab3..bb75bdc65d3 100644\n>> --- a/config.c\n>> +++ b/config.c\n>> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char\n>> *var, const char *value,\n>> !strcmp(var, \"core.commentstring\")) {\n>> if (!value)\n>> return config_error_nonbool(var);\n>> - else if (!strcasecmp(value, \"auto\"))\n>> + else if (!strcasecmp(value, \"auto\")) {\n>> auto_comment_line_char = 1;\n>> - else if (value[0]) {\n>> + FREE_AND_NULL(comment_line_str_to_free);\n>> + comment_line_str = \"#\";\n>> + } else if (value[0]) {\n>> if (strchr(value, '\\n'))\n>> return error(_(\"%s cannot contain\n>> newline\"), var);\n>> comment_line_str = value;\n>>\n> \n> Thanks, I understood it.\n> \n> What if we simply return the function `adjust_comment_line_char()` if\n> we get a non-zero value from `ignored_log_message_bytes()`, i.e we\n> won't scan the commit message in case conflict message exists, and we\n> let the old code exist as it is?\n> \n> + if(ignored_log_message_bytes(sb->buf, sb->len))\n> + return;\n\nSo we'd ignore core.commentChar=auto if we detected conflict comments? \nThat might be surprising to the user - it would mean that we'd always \navoid adding the conflict comments to the commit message but we'd lose \nany lines that begin with the comment string. I think I'm leaning \nslightly towards the original solution but it is not clear to me that \none option is much better that the other.\n\nThanks\n\nPhillip\n\n"},{"id":"521552","messageId":"CAE7as+Yp9GWRohqe4oHHmYa1MfuKbyg9qKRf_z6N50bCSZ8vzQ@mail.gmail.com","threadId":"63699","inReplyTo":"062e7abd-97b1-4806-9753-338906642265@gmail.com","subject":"Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-08T15:47:09Z","receivedAt":"2025-07-08T15:47:21Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Hey, Phillip\n\nOn Fri, Jul 4, 2025 at 1:53 PM Phillip Wood wrote:\n>\n> Hi Ayush\n>\n> On 03/07/2025 00:46, Ayush Chandekar wrote:\n> > On Wed, Jul 2, 2025 at 1:02 AM Phillip Wood wrote:\n> >> diff --git a/config.c b/config.c\n> >> index eb60c293ab3..bb75bdc65d3 100644\n> >> --- a/config.c\n> >> +++ b/config.c\n> >> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char\n> >> *var, const char *value,\n> >> !strcmp(var, \"core.commentstring\")) {\n> >> if (!value)\n> >> return config_error_nonbool(var);\n> >> - else if (!strcasecmp(value, \"auto\"))\n> >> + else if (!strcasecmp(value, \"auto\")) {\n> >> auto_comment_line_char = 1;\n> >> - else if (value[0]) {\n> >> + FREE_AND_NULL(comment_line_str_to_free);\n> >> + comment_line_str = \"#\";\n> >> + } else if (value[0]) {\n> >> if (strchr(value, '\\n'))\n> >> return error(_(\"%s cannot contain\n> >> newline\"), var);\n> >> comment_line_str = value;\n> >>\n> >\n> > Thanks, I understood it.\n> >\n> > What if we simply return the function `adjust_comment_line_char()` if\n> > we get a non-zero value from `ignored_log_message_bytes()`, i.e we\n> > won't scan the commit message in case conflict message exists, and we\n> > let the old code exist as it is?\n> >\n> > + if(ignored_log_message_bytes(sb->buf, sb->len))\n> > + return;\n>\n> So we'd ignore core.commentChar=auto if we detected conflict comments?\n> That might be surprising to the user - it would mean that we'd always\n> avoid adding the conflict comments to the commit message but we'd lose\n> any lines that begin with the comment string. I think I'm leaning\n> slightly towards the original solution but it is not clear to me that\n> one option is much better that the other.\n>\n> Thanks\n>\n> Phillip\n>\n\nNow that we're planning to get rid of the 'auto' keyword from\ncommentChar [1], do you think it would be better if we just ignored\nthe keyword when we detect conflict comments? Also, how is it that a\nuser will end up having lines starting with the character being the\nsame as the conflict comment's character?\n\nThanks!\nAyush\n\n[1]: https://lore.kernel.org/git/cover.1751983009.git.phillip.wood@dunelm.org.uk/\n"},{"id":"521677","messageId":"0570c2fb-115d-483d-ad7f-35786994f5d1@gmail.com","threadId":"63699","inReplyTo":"CAE7as+Yp9GWRohqe4oHHmYa1MfuKbyg9qKRf_z6N50bCSZ8vzQ@mail.gmail.com","subject":"Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-07-09T14:17:56Z","receivedAt":"2025-07-09T14:17:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 08/07/2025 16:47, Ayush Chandekar wrote:\n> \n> Now that we're planning to get rid of the 'auto' keyword from\n> commentChar [1], do you think it would be better if we just ignored\n> the keyword when we detect conflict comments? Also, how is it that a\n> user will end up having lines starting with the character being the\n> same as the conflict comment's character?\n\nLet see if Junio agrees with depreciation and removing support for \ncommentChar=auto first. If we do deprecate it then we should still \nsupport it until it is removed so I'm leaning towards fixing the config \nparsing to reset comment_line_char to '#' instead.\n\nThanks\n\nPhillip\n\n"},{"id":"522023","messageId":"cover.1752602474.git.ayu.chandekar@gmail.com","threadId":"63699","inReplyTo":"20250626132233.414789-1-ayu.chandekar@gmail.com","subject":"[GSOC PATCH 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-15T18:51:24Z","receivedAt":"2025-07-15T18:52:05Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Hey everyone,\n\nThe aim of this patch series is to improve the behaviour of core.commentChar=auto by the following patches:\n1/2 - Fix a bug which reads comment character of the comments in commit message leading to change in the value of `comment_line_str` and thus resulting the comments in the final commit message.\n2/2 - Standardizes the behaviour of code by resetting the 'comment_line_str' to \"#\" when core.commentChar is set to auto. \n\nThanks to Junio, Phillip and Kristoffer for reviewing the patches and also Christian for the reviews and mentoring me.\n\nAyush Chandekar (2):\n commit: avoid scanning trailing comments when 'core.commentChar' is\n \"auto\"\n config: set comment_line_str to \"#\" when core.commentChar=auto\n\n builtin/commit.c | 6 +++++-\n config.c | 6 ++++--\n t/t3418-rebase-continue.sh | 13 +++++++++++++\n 3 files changed, 22 insertions(+), 3 deletions(-)\n\n-- \n2.49.0\n\n"},{"id":"522024","messageId":"fbee656fb80ef673ea0ee4fafdf4baa9f18b5619.1752602474.git.ayu.chandekar@gmail.com","threadId":"63699","inReplyTo":"cover.1752602474.git.ayu.chandekar@gmail.com","subject":"[GSOC PATCH 1/2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-15T18:51:25Z","receivedAt":"2025-07-15T18:52:16Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"When core.commentChar is set to \"auto\", Git selects a comment character\nby scanning the commit message contents and avoiding any character\nalready present in the message.\n\nIf the message still contains old conflict comments (starting with a\ncomment character), Git assumes that character is in use and chooses a\ndifferent one. As a result, those existing comment lines are no longer\nrecognized as comments and end up being included in the final commit\nmessage.\n\nTo avoid this, skip scanning the trailing comment block when selecting\nthe comment character. This allows Git to safely reuse the original\ncharacter when appropriate, keeping the commit message clean and free of\nleftover conflict information.\n\nBackground:\n\nThe \"auto\" value for core.commentchar was introduced in the commit\n84c9dc2c5a (commit: allow core.commentChar=auto for character auto\nselection, 2014-05-17) but did not exhibit this issue at that time.\n\nThe bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff\nwith conflicts, 2024-04-18) where Git started writing conflict comments\nto the file at 'rebase_path_message()'.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n builtin/commit.c | 6 +++++-\n t/t3418-rebase-continue.sh | 13 +++++++++++++\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex fba0dded64..63e7158e98 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tchar candidates[] = \"#;@!$%^&|:\";\n \tchar *candidate;\n \tconst char *p;\n+\tsize_t cutoff;\n+\n+\t/* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n+\tcutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n \n \tif (!memchr(sb->buf, candidates[0], sb->len)) {\n \t\tfree(comment_line_str_to_free);\n@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tcandidate = strchr(candidates, *p);\n \tif (candidate)\n \t\t*candidate = ' ';\n-\tfor (p = sb->buf; *p; p++) {\n+\tfor (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {\n \t\tif ((p[0] == '\\n' || p[0] == '\\r') && p[1]) {\n \t\t\tcandidate = strchr(candidates, p[1]);\n \t\t\tif (candidate)\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 127216f722..b8a8dd77e7 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -328,6 +328,19 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas\n \ttest_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec\n '\n \n+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n+\tgit checkout -b branch-a &&\n+\ttest_commit A F1 &&\n+\tgit checkout -b branch-b HEAD^ &&\n+\ttest_commit B F1 &&\n+\ttest_must_fail git rebase branch-a &&\n+\tprintf \"B\\nA\\n\" >F1 &&\n+\tgit add F1 &&\n+\tGIT_EDITOR=\"cat >actual\" git -c core.commentChar=auto rebase --continue &&\n+\t# Check that \"#\" is still the comment character.\n+\ttest_grep \"^# Changes to be committed\" actual\n+'\n+\n test_orig_head_helper () {\n \ttest_when_finished 'git rebase --abort &&\n \t\tgit checkout topic &&\n-- \n2.49.0\n\n"},{"id":"522025","messageId":"2a3c2d323bdb520a37a099b361be9ec5f2d5d46f.1752602474.git.ayu.chandekar@gmail.com","threadId":"63699","inReplyTo":"cover.1752602474.git.ayu.chandekar@gmail.com","subject":"[GSOC PATCH 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-15T18:51:26Z","receivedAt":"2025-07-15T18:52:22Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"If conflict comments already use a comment character that isn't \"#\", and\ncore.commentChar is set \"auto\", Git will ignore these lines during the\nscan using ignored_log_message_bytes() and pick a new comment character\nbased on the rest of the message. The newly chosen character may be\ndifferent from the one used in the conflict comments and therefore,\nthese are no longer treated as comments and end up in the final commit\nmessage.\n\nFor example, during a rebase if the user previously set\ncore.commentChar=% and then encounters a conflict, conflict comments\nlike \"% Conflicts:\" are generated. If the user subsequently sets\ncore.commentChar=auto before running `rebase --continue`, Git parses the\n\"auto\" setting and begins scanning. It first uses the existing\n'comment_line_str' (which is '%') to detect and ignore conflict comments\nvia ignored_log_message_bytes().\n\nThen, Git scans the rest of the message (excluding conflict comments),\nsees that none of the remaining lines start with '#' and decides to set\ncomment_line_str to '#'. Since the final commit character differs from\nthe one used in the conflict comments, those lines are no longer\nconsidered comments and get included in the final commit message.\n\nSet 'comment_line_str' to '#' when core.commentChar is set to 'auto' to\nreset any previously set value.\n\nWhile this does not solve the issue of conflict comment inclusion and\nthe user visible behaviour stays tha same, it standardizes the behaviour\nof the code by always resetting 'comment_line_str' to '#' when\ncore.commentChar=auto is parsed.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n config.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex eb60c293ab..bb75bdc65d 100644\n--- a/config.c\n+++ b/config.c\n@@ -1537,9 +1537,11 @@ static int git_default_core_config(const char *var, const char *value,\n \t !strcmp(var, \"core.commentstring\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\telse if (!strcasecmp(value, \"auto\"))\n+\t\telse if (!strcasecmp(value, \"auto\")) {\n \t\t\tauto_comment_line_char = 1;\n-\t\telse if (value[0]) {\n+\t\t\tFREE_AND_NULL(comment_line_str_to_free);\n+\t\t\tcomment_line_str = \"#\";\n+\t\t} else if (value[0]) {\n \t\t\tif (strchr(value, '\\n'))\n \t\t\t\treturn error(_(\"%s cannot contain newline\"), var);\n \t\t\tcomment_line_str = value;\n-- \n2.49.0\n\n"},{"id":"522046","messageId":"xmqq1pqhgnby.fsf@gitster.g","threadId":"63699","inReplyTo":"2a3c2d323bdb520a37a099b361be9ec5f2d5d46f.1752602474.git.ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-15T21:23:45Z","receivedAt":"2025-07-15T21:23:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar writes:\n\n> If conflict comments already use a comment character that isn't \"#\", and\n> core.commentChar is set \"auto\", Git will ignore these lines during the\n> scan using ignored_log_message_bytes() and pick a new comment character\n> based on the rest of the message. The newly chosen character may be\n> different from the one used in the conflict comments and therefore,\n> these are no longer treated as comments and end up in the final commit\n> message.\n>\n> For example, during a rebase if the user previously set\n> core.commentChar=% and then encounters a conflict, conflict comments\n> like \"% Conflicts:\" are generated. If the user subsequently sets\n> core.commentChar=auto before running `rebase --continue`, Git parses the\n> \"auto\" setting and begins scanning. It first uses the existing\n> 'comment_line_str' (which is '%') to detect and ignore conflict comments\n> via ignored_log_message_bytes().\n>\n> Then, Git scans the rest of the message (excluding conflict comments),\n> sees that none of the remaining lines start with '#' and decides to set\n> comment_line_str to '#'. Since the final commit character differs from\n> the one used in the conflict comments, those lines are no longer\n> considered comments and get included in the final commit message.\n>\n> Set 'comment_line_str' to '#' when core.commentChar is set to 'auto' to\n> reset any previously set value.\n>\n> While this does not solve the issue of conflict comment inclusion and\n> the user visible behaviour stays tha same, it standardizes the behaviour\n> of the code by always resetting 'comment_line_str' to '#' when\n> core.commentChar=auto is parsed.\n>\n> Mentored-by: Christian Couder \n> Mentored-by: Ghanshyam Thakkar \n> Signed-off-by: Ayush Chandekar \n> ---\n> config.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n>\n> diff --git a/config.c b/config.c\n> index eb60c293ab..bb75bdc65d 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char *var, const char *value,\n> \t !strcmp(var, \"core.commentstring\")) {\n> \t\tif (!value)\n> \t\t\treturn config_error_nonbool(var);\n> -\t\telse if (!strcasecmp(value, \"auto\"))\n> +\t\telse if (!strcasecmp(value, \"auto\")) {\n> \t\t\tauto_comment_line_char = 1;\n> -\t\telse if (value[0]) {\n> +\t\t\tFREE_AND_NULL(comment_line_str_to_free);\n> +\t\t\tcomment_line_str = \"#\";\n> +\t\t} else if (value[0]) {\n> \t\t\tif (strchr(value, '\\n'))\n> \t\t\t\treturn error(_(\"%s cannot contain newline\"), var);\n> \t\t\tcomment_line_str = value;\n\nThis patch is exactly what Phillip suggested in\n\nhttps://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/\n\nisn't it? Makes sense to me.\n\n"},{"id":"522053","messageId":"CAE7as+aN+j4CteHUrr+R+CbZ=qi=mehYW2xQEG4ZcQYvXqJsaQ@mail.gmail.com","threadId":"63699","inReplyTo":"xmqq1pqhgnby.fsf@gitster.g","subject":"Re: [GSOC PATCH 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-15T22:15:52Z","receivedAt":"2025-07-15T22:16:05Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Hi Junio,\n\nOn Wed, Jul 16, 2025 at 2:53 AM Junio C Hamano wrote:\n>\n[snip]\n>\n> This patch is exactly what Phillip suggested in\n>\n> https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/\n>\n> isn't it? Makes sense to me.\n>\n\nYes, you're right. I should add the suggested-by trailer for this patch.\n\nThanks\nAyush\n"},{"id":"522055","messageId":"xmqqcya1f2vr.fsf@gitster.g","threadId":"63699","inReplyTo":"CAE7as+aN+j4CteHUrr+R+CbZ=qi=mehYW2xQEG4ZcQYvXqJsaQ@mail.gmail.com","subject":"Re: [GSOC PATCH 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-15T23:30:48Z","receivedAt":"2025-07-15T23:30:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar writes:\n\n> On Wed, Jul 16, 2025 at 2:53 AM Junio C Hamano wrote:\n>>\n> [snip]\n>>\n>> This patch is exactly what Phillip suggested in\n>>\n>> https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/\n>>\n>> isn't it? Makes sense to me.\n>>\n>\n> Yes, you're right. I should add the suggested-by trailer for this patch.\n\nI am not sure about that, though. A verbatim copy is stronger than\nimplementing what was suggested by another person. If I were in\nyour position, I'll probably say something like\n\n\tThe patch text was taken from Phillip Wood's message [*URL*],\n\twith the commit log message written by me.\n\n\tBased-on-a-patch-by: Phillip Wood <...>\n\tSigned-off-by: Ayush Chandekar <...>\n\nIn any case, this overlaps both textually but also intent-wise with\nPhillip's \"let's mark core.commentchar=auto deprecated and remove\nthe support at 3.0 boundary\", which is planned to be rerolled to\nmake it a failure when the user uses core.commentchar=auto. It\nwould be a while before we tag Git 3.0, so the fix in this topic\nwill be necessary until then.\n\nThanks.\n\n"},{"id":"522092","messageId":"CAE7as+YxajFO0FfMe2wYpT9okYQoevZAghDD29d7E0P82-A_Hw@mail.gmail.com","threadId":"63699","inReplyTo":"xmqqcya1f2vr.fsf@gitster.g","subject":"Re: [GSOC PATCH 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-16T11:04:36Z","receivedAt":"2025-07-16T11:04:48Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Wed, Jul 16, 2025 at 5:00 AM Junio C Hamano wrote:\n>\n> Ayush Chandekar writes:\n>\n> > On Wed, Jul 16, 2025 at 2:53 AM Junio C Hamano wrote:\n> >>\n> > [snip]\n> >>\n> >> This patch is exactly what Phillip suggested in\n> >>\n> >> https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/\n> >>\n> >> isn't it? Makes sense to me.\n> >>\n> >\n> > Yes, you're right. I should add the suggested-by trailer for this patch.\n>\n> I am not sure about that, though. A verbatim copy is stronger than\n> implementing what was suggested by another person. If I were in\n> your position, I'll probably say something like\n>\n> The patch text was taken from Phillip Wood's message [*URL*],\n> with the commit log message written by me.\n>\n> Based-on-a-patch-by: Phillip Wood <...>\n> Signed-off-by: Ayush Chandekar <...>\n>\n> In any case, this overlaps both textually but also intent-wise with\n> Phillip's \"let's mark core.commentchar=auto deprecated and remove\n> the support at 3.0 boundary\", which is planned to be rerolled to\n> make it a failure when the user uses core.commentchar=auto. It\n> would be a while before we tag Git 3.0, so the fix in this topic\n> will be necessary until then.\n>\n> Thanks.\n>\n\nYeah, Phillip should actually get the primary credit for this patch\nand Suggested-by does not do enough justice.\nI will send a new version right away.\n\nThanks!\nAyush\n"},{"id":"522094","messageId":"cover.1752665506.git.ayu.chandekar@gmail.com","threadId":"63699","inReplyTo":"20250626132233.414789-1-ayu.chandekar@gmail.com","subject":"[GSOC PATCH v5 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-16T11:43:27Z","receivedAt":"2025-07-16T11:43:45Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"\nHey everyone,\n\nThe aim of this patch series is to improve the behaviour of core.commentChar=auto by the following patches:\n1/2 - Fix a bug which reads comment character of the comments in commit message leading to change in the value of `comment_line_str` and thus resulting the comments in the final commit message.\n2/2 - Standardizes the behaviour of code by resetting the 'comment_line_str' to \"#\" when 'core.commentChar' is set to \"auto\". \n\nThanks to Junio, Phillip and Kristoffer for reviewing the patches and also Christian for the reviews and mentoring me.\n\nThe only difference between this version (v5) and the previous one is that I've added credit to Phillip for patch (2/2).\n\nAyush Chandekar (2):\n commit: avoid scanning trailing comments when 'core.commentChar' is\n \"auto\"\n config: set comment_line_str to \"#\" when core.commentChar=auto\n\n builtin/commit.c | 6 +++++-\n config.c | 6 ++++--\n t/t3418-rebase-continue.sh | 13 +++++++++++++\n 3 files changed, 22 insertions(+), 3 deletions(-)\n\n-- \n2.49.0\n\n"},{"id":"522095","messageId":"fbee656fb80ef673ea0ee4fafdf4baa9f18b5619.1752665506.git.ayu.chandekar@gmail.com","threadId":"63699","inReplyTo":"cover.1752665506.git.ayu.chandekar@gmail.com","subject":"[GSOC PATCH v5 1/2] commit: avoid scanning trailing comments when 'core.commentChar' is \"auto\"","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-16T11:43:28Z","receivedAt":"2025-07-16T11:43:51Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"When core.commentChar is set to \"auto\", Git selects a comment character\nby scanning the commit message contents and avoiding any character\nalready present in the message.\n\nIf the message still contains old conflict comments (starting with a\ncomment character), Git assumes that character is in use and chooses a\ndifferent one. As a result, those existing comment lines are no longer\nrecognized as comments and end up being included in the final commit\nmessage.\n\nTo avoid this, skip scanning the trailing comment block when selecting\nthe comment character. This allows Git to safely reuse the original\ncharacter when appropriate, keeping the commit message clean and free of\nleftover conflict information.\n\nBackground:\n\nThe \"auto\" value for core.commentchar was introduced in the commit\n84c9dc2c5a (commit: allow core.commentChar=auto for character auto\nselection, 2014-05-17) but did not exhibit this issue at that time.\n\nThe bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff\nwith conflicts, 2024-04-18) where Git started writing conflict comments\nto the file at 'rebase_path_message()'.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n builtin/commit.c | 6 +++++-\n t/t3418-rebase-continue.sh | 13 +++++++++++++\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex fba0dded64..63e7158e98 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tchar candidates[] = \"#;@!$%^&|:\";\n \tchar *candidate;\n \tconst char *p;\n+\tsize_t cutoff;\n+\n+\t/* Ignore comment chars in trailing comments (e.g., Conflicts:) */\n+\tcutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);\n \n \tif (!memchr(sb->buf, candidates[0], sb->len)) {\n \t\tfree(comment_line_str_to_free);\n@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)\n \tcandidate = strchr(candidates, *p);\n \tif (candidate)\n \t\t*candidate = ' ';\n-\tfor (p = sb->buf; *p; p++) {\n+\tfor (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {\n \t\tif ((p[0] == '\\n' || p[0] == '\\r') && p[1]) {\n \t\t\tcandidate = strchr(candidates, p[1]);\n \t\t\tif (candidate)\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 127216f722..b8a8dd77e7 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -328,6 +328,19 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas\n \ttest_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec\n '\n \n+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '\n+\tgit checkout -b branch-a &&\n+\ttest_commit A F1 &&\n+\tgit checkout -b branch-b HEAD^ &&\n+\ttest_commit B F1 &&\n+\ttest_must_fail git rebase branch-a &&\n+\tprintf \"B\\nA\\n\" >F1 &&\n+\tgit add F1 &&\n+\tGIT_EDITOR=\"cat >actual\" git -c core.commentChar=auto rebase --continue &&\n+\t# Check that \"#\" is still the comment character.\n+\ttest_grep \"^# Changes to be committed\" actual\n+'\n+\n test_orig_head_helper () {\n \ttest_when_finished 'git rebase --abort &&\n \t\tgit checkout topic &&\n-- \n2.49.0\n\n"},{"id":"522096","messageId":"ffe16a257f5cff54630aac0b9af601705b2865d6.1752665506.git.ayu.chandekar@gmail.com","threadId":"63699","inReplyTo":"cover.1752665506.git.ayu.chandekar@gmail.com","subject":"[GSOC PATCH v5 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-16T11:43:29Z","receivedAt":"2025-07-16T11:43:56Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"If conflict comments already use a comment character that isn't \"#\", and\ncore.commentChar is set \"auto\", Git will ignore these lines during the\nscan using ignored_log_message_bytes() and pick a new comment character\nbased on the rest of the message. The newly chosen character may be\ndifferent from the one used in the conflict comments and therefore,\nthese are no longer treated as comments and end up in the final commit\nmessage.\n\nFor example, during a rebase if the user previously set\ncore.commentChar=% and then encounters a conflict, conflict comments\nlike \"% Conflicts:\" are generated. If the user subsequently sets\ncore.commentChar=auto before running `rebase --continue`, Git parses the\n\"auto\" setting and begins scanning. It first uses the existing\n'comment_line_str' (which is '%') to detect and ignore conflict comments\nvia ignored_log_message_bytes().\n\nThen, Git scans the rest of the message (excluding conflict comments),\nsees that none of the remaining lines start with '#' and decides to set\ncomment_line_str to '#'. Since the final commit character differs from\nthe one used in the conflict comments, those lines are no longer\nconsidered comments and get included in the final commit message.\n\nSet 'comment_line_str' to '#' when core.commentChar is set to 'auto' to\nreset any previously set value.\n\nWhile this does not solve the issue of conflict comment inclusion and\nthe user visible behaviour stays tha same, it standardizes the behaviour\nof the code by always resetting 'comment_line_str' to '#' when\ncore.commentChar=auto is parsed.\n\nThe patch text is based on Phillip Wood's message:\nhttps://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/\nand the commit log message is wriiten by me.\n\nBased-on-a-patch-by: Phillip Wood \nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n config.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex eb60c293ab..bb75bdc65d 100644\n--- a/config.c\n+++ b/config.c\n@@ -1537,9 +1537,11 @@ static int git_default_core_config(const char *var, const char *value,\n \t !strcmp(var, \"core.commentstring\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\telse if (!strcasecmp(value, \"auto\"))\n+\t\telse if (!strcasecmp(value, \"auto\")) {\n \t\t\tauto_comment_line_char = 1;\n-\t\telse if (value[0]) {\n+\t\t\tFREE_AND_NULL(comment_line_str_to_free);\n+\t\t\tcomment_line_str = \"#\";\n+\t\t} else if (value[0]) {\n \t\t\tif (strchr(value, '\\n'))\n \t\t\t\treturn error(_(\"%s cannot contain newline\"), var);\n \t\t\tcomment_line_str = value;\n-- \n2.49.0\n\n"},{"id":"522107","messageId":"51e75a0f-fc6c-452c-b1c3-2836d1508308@gmail.com","threadId":"63699","inReplyTo":"cover.1752665506.git.ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH v5 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-07-16T14:28:00Z","receivedAt":"2025-07-16T14:28:10Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 16/07/2025 12:43, Ayush Chandekar wrote:\n> \n> Hey everyone,\n> \n> The aim of this patch series is to improve the behaviour of core.commentChar=auto by the following patches:\n> 1/2 - Fix a bug which reads comment character of the comments in commit message leading to change in the value of `comment_line_str` and thus resulting the comments in the final commit message.\n> 2/2 - Standardizes the behaviour of code by resetting the 'comment_line_str' to \"#\" when 'core.commentChar' is set to \"auto\".\n\nThis version looks good to me, thanks for working on it.\n\nJunio - shall I rebase 'pw/3.0-commentchar-auto-deprecation' on top of \nthis when I re-roll to avoid conflicts?\n\nThanks\n\nPhillip\n\n> Thanks to Junio, Phillip and Kristoffer for reviewing the patches and also Christian for the reviews and mentoring me.\n> \n> The only difference between this version (v5) and the previous one is that I've added credit to Phillip for patch (2/2).\n> \n> Ayush Chandekar (2):\n> commit: avoid scanning trailing comments when 'core.commentChar' is\n> \"auto\"\n> config: set comment_line_str to \"#\" when core.commentChar=auto\n> \n> builtin/commit.c | 6 +++++-\n> config.c | 6 ++++--\n> t/t3418-rebase-continue.sh | 13 +++++++++++++\n> 3 files changed, 22 insertions(+), 3 deletions(-)\n> \nJ\n\n"},{"id":"522114","messageId":"xmqq1pqgduvo.fsf@gitster.g","threadId":"63699","inReplyTo":"CAE7as+YxajFO0FfMe2wYpT9okYQoevZAghDD29d7E0P82-A_Hw@mail.gmail.com","subject":"Re: [GSOC PATCH 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-16T15:21:15Z","receivedAt":"2025-07-16T15:21:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar writes:\n\n> Yeah, Phillip should actually get the primary credit for this patch\n> and Suggested-by does not do enough justice.\n> I will send a new version right away.\n\nThanks. Don't forget to ask him to sign-off.\n\n"},{"id":"522115","messageId":"b16c6f79-c021-4068-9c95-09625ca058c7@gmail.com","threadId":"63699","inReplyTo":"xmqq1pqgduvo.fsf@gitster.g","subject":"Re: [GSOC PATCH 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-07-16T15:24:29Z","receivedAt":"2025-07-16T15:24:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 16/07/2025 16:21, Junio C Hamano wrote:\n> Ayush Chandekar writes:\n> \n>> Yeah, Phillip should actually get the primary credit for this patch\n>> and Suggested-by does not do enough justice.\n>> I will send a new version right away.\n> \n> Thanks. Don't forget to ask him to sign-off.\n\nHere it is\n\nSigned-off-by: \n\n"},{"id":"522116","messageId":"xmqqwm88cfyz.fsf@gitster.g","threadId":"63699","inReplyTo":"ffe16a257f5cff54630aac0b9af601705b2865d6.1752665506.git.ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH v5 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-16T15:28:36Z","receivedAt":"2025-07-16T15:28:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar writes:\n\n> If conflict comments already use a comment character that isn't \"#\", and\n> ...\n> The patch text is based on Phillip Wood's message:\n> https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/\n> and the commit log message is wriiten by me.\n>\n> Based-on-a-patch-by: Phillip Wood \n> Mentored-by: Christian Couder \n> Mentored-by: Ghanshyam Thakkar \n> Signed-off-by: Ayush Chandekar \n> ---\n\nEarlier in response to your \"Phillip should actually get the primary\ncredit\" I said to ask for his sign-off, because I took it as you are\nactually making Phillip the author of the patch. But it is fine\neither way. Phillip has given his permission to add a sign-off, so\nwe have everything to move this topic forward.\n\nThanks, all!\n\n\n> config.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/config.c b/config.c\n> index eb60c293ab..bb75bdc65d 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char *var, const char *value,\n> \t !strcmp(var, \"core.commentstring\")) {\n> \t\tif (!value)\n> \t\t\treturn config_error_nonbool(var);\n> -\t\telse if (!strcasecmp(value, \"auto\"))\n> +\t\telse if (!strcasecmp(value, \"auto\")) {\n> \t\t\tauto_comment_line_char = 1;\n> -\t\telse if (value[0]) {\n> +\t\t\tFREE_AND_NULL(comment_line_str_to_free);\n> +\t\t\tcomment_line_str = \"#\";\n> +\t\t} else if (value[0]) {\n> \t\t\tif (strchr(value, '\\n'))\n> \t\t\t\treturn error(_(\"%s cannot contain newline\"), var);\n> \t\t\tcomment_line_str = value;\n"},{"id":"522117","messageId":"xmqqseiwcfyb.fsf@gitster.g","threadId":"63699","inReplyTo":"b16c6f79-c021-4068-9c95-09625ca058c7@gmail.com","subject":"Re: [GSOC PATCH 2/2] config: set comment_line_str to \"#\" when core.commentChar=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-16T15:29:00Z","receivedAt":"2025-07-16T15:29:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 16/07/2025 16:21, Junio C Hamano wrote:\n>> Ayush Chandekar writes:\n>> \n>>> Yeah, Phillip should actually get the primary credit for this patch\n>>> and Suggested-by does not do enough justice.\n>>> I will send a new version right away.\n>> Thanks. Don't forget to ask him to sign-off.\n>\n> Here it is\n>\n> Signed-off-by: \n\nThanks.\n"},{"id":"522118","messageId":"xmqqo6tkcfws.fsf@gitster.g","threadId":"63699","inReplyTo":"51e75a0f-fc6c-452c-b1c3-2836d1508308@gmail.com","subject":"Re: [GSOC PATCH v5 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-16T15:29:55Z","receivedAt":"2025-07-16T15:29:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> This version looks good to me, thanks for working on it.\n>\n> Junio - shall I rebase 'pw/3.0-commentchar-auto-deprecation' on top of\n> this when I re-roll to avoid conflicts?\n\nSounds sensible. I can drop my merge-fix then.\n\nThanks.\n"}]}