[GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

When core.commentChar is set to "auto", Git selects a comment character
by scanning the commit message contents and avoiding any character
already present in the message.

If the message still contains old conflict comments (starting with a
comment character), Git assumes that character is in use and chooses a
different one. As a result, those existing comment lines are no longer
recognized as comments and end up being included in the final commit
message.

To avoid this, skip scanning the trailing comment block when selecting
the comment character. This allows Git to safely reuse the original
character when appropriate, keeping the commit message clean and free of
leftover conflict information.

Background:

The "auto" value for core.commentchar was introduced in the commit
`84c9dc2` (commit: allow core.commentChar=auto for character auto
selection) but did not exhibt this issue at that time.

The bug was introduced in commit `a6c2654` (rebase -m: fix --signoff
with conflicts) where Git started writing conflict comments to the file
at 'rebase_path_message()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---

I came across this bug when working on a patch series that removes 
the global variables related to the "commentChar" config options.

 builtin/commit.c           |  6 +++++-
 t/t3418-rebase-continue.sh | 18 ++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index fba0dded64..63e7158e98 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	char candidates[] = "#;@!$%^&|:";
 	char *candidate;
 	const char *p;
+	size_t cutoff;
+
+	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
+	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
 
 	if (!memchr(sb->buf, candidates[0], sb->len)) {
 		free(comment_line_str_to_free);
@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	candidate = strchr(candidates, *p);
 	if (candidate)
 		*candidate = ' ';
-	for (p = sb->buf; *p; p++) {
+	for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
 		if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
 			candidate = strchr(candidates, p[1]);
 			if (candidate)
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 127216f722..a8e89a250b 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -328,6 +328,24 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
 	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
 '
 
+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
+	test_commit base file &&
+	git checkout -b branch-a &&
+	test_commit A file &&
+	git checkout -b branch-b base &&
+	test_commit B file &&
+	test_must_fail git rebase branch-a &&
+	printf "B\nA\n" >file &&
+	git add file &&
+	write_script fake-editor <<-\EOF &&
+	exit 0
+	EOF
+	FAKE_EDITOR="$(pwd)/fake-editor" &&
+	GIT_EDITOR="\"\$FAKE_EDITOR\"" git -c core.commentChar=auto rebase --continue &&
+	# Check that "#" is still the comment character.
+	test_grep "# Changes to be committed:" .git/COMMIT_EDITMSG
+'
+
 test_orig_head_helper () {
 	test_when_finished 'git rebase --abort &&
 		git checkout topic &&
-- 
2.49.0

Re: [GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> When core.commentChar is set to "auto", Git selects a comment character
> by scanning the commit message contents and avoiding any character
> already present in the message.
>
> If the message still contains old conflict comments (starting with a
> comment character), Git assumes that character is in use and chooses a
> different one. As a result, those existing comment lines are no longer
> recognized as comments and end up being included in the final commit
> message.
>
> To avoid this, skip scanning the trailing comment block when selecting
> the comment character. This allows Git to safely reuse the original
> character when appropriate, keeping the commit message clean and free of
> leftover conflict information.
>
> Background:
>
> The "auto" value for core.commentchar was introduced in the commit
> `84c9dc2` (commit: allow core.commentChar=auto for character auto
> selection) but did not exhibt this issue at that time.

Use "git log -1 --format=reference", i.e.

84c9dc2c (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17)

> The bug was introduced in commit `a6c2654` (rebase -m: fix --signoff
> with conflicts) where Git started writing conflict comments to the file
> at 'rebase_path_message()'.

Ditto.

> diff --git a/builtin/commit.c b/builtin/commit.c
> index fba0dded64..63e7158e98 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)
>  	char candidates[] = "#;@!$%^&|:";
>  	char *candidate;
>  	const char *p;
> +	size_t cutoff;
> +
> +	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
> +	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
>  
>  	if (!memchr(sb->buf, candidates[0], sb->len)) {
>  		free(comment_line_str_to_free);
> @@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
>  	candidate = strchr(candidates, *p);
>  	if (candidate)
>  		*candidate = ' ';
> -	for (p = sb->buf; *p; p++) {
> +	for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
>  		if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
>  			candidate = strchr(candidates, p[1]);
>  			if (candidate)

Looks quite straight-forward.  Nice.

Thanks.

Re: [GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

On Thu, Jun 26, 2025 at 8:03 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>
> > When core.commentChar is set to "auto", Git selects a comment character
> > by scanning the commit message contents and avoiding any character
> > already present in the message.
> >
> > If the message still contains old conflict comments (starting with a
> > comment character), Git assumes that character is in use and chooses a
> > different one. As a result, those existing comment lines are no longer
> > recognized as comments and end up being included in the final commit
> > message.
> >
> > To avoid this, skip scanning the trailing comment block when selecting
> > the comment character. This allows Git to safely reuse the original
> > character when appropriate, keeping the commit message clean and free of
> > leftover conflict information.
> >
> > Background:
> >
> > The "auto" value for core.commentchar was introduced in the commit
> > `84c9dc2` (commit: allow core.commentChar=auto for character auto
> > selection) but did not exhibt this issue at that time.
>
> Use "git log -1 --format=reference", i.e.
>
> 84c9dc2c (commit: allow core.commentChar=auto for character auto
> selection, 2014-05-17)
>
Got it, Thanks! I'll update it.

Re: [GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Kristoffer Haugsbakk]

On Thu, Jun 26, 2025, at 15:22, Ayush Chandekar wrote:
> When core.commentChar is set to "auto", Git selects a comment character
> by scanning the commit message contents and avoiding any character
> already present in the message.
>
> If the message still contains old conflict comments (starting with a
> comment character), Git assumes that character is in use and chooses a
> different one. As a result, those existing comment lines are no longer
> recognized as comments and end up being included in the final commit
> message.
>
> To avoid this, skip scanning the trailing comment block when selecting
> the comment character. This allows Git to safely reuse the original
> character when appropriate, keeping the commit message clean and free of
> leftover conflict information.
>
> Background:
>
> The "auto" value for core.commentchar was introduced in the commit
> `84c9dc2` (commit: allow core.commentChar=auto for character auto
> selection) but did not exhibt this issue at that time.
>
> The bug was introduced in commit `a6c2654` (rebase -m: fix --signoff
> with conflicts) where Git started writing conflict comments to the file
> at 'rebase_path_message()'.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---

Nice explanation.

> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index 127216f722..a8e89a250b 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -328,6 +328,24 @@ test_expect_success 'there is no
> --no-reschedule-failed-exec in an ongoing rebas
>  	test_expect_code 129 git rebase --edit-todo
> --no-reschedule-failed-exec
>  '
>
> +test_expect_success 'no change in comment character due to conflicts
> markers with core.commentChar=auto' '
> +	test_commit base file &&
> +	git checkout -b branch-a &&
> +	test_commit A file &&
> +	git checkout -b branch-b base &&
> +	test_commit B file &&
> +	test_must_fail git rebase branch-a &&
> +	printf "B\nA\n" >file &&
> +	git add file &&
> +	write_script fake-editor <<-\EOF &&
> +	exit 0
> +	EOF
> +	FAKE_EDITOR="$(pwd)/fake-editor" &&
> +	GIT_EDITOR="\"\$FAKE_EDITOR\"" git -c core.commentChar=auto rebase --continue &&

How about

    GIT_EDITOR="cat >actual"

Then you can `test_grep` on that.  Like in

https://lore.kernel.org/git/5ed77fab-678d-4a06-bbd0-ea25462a7562@gmail.com/

> +	# Check that "#" is still the comment character.
> +	test_grep "# Changes to be committed:" .git/COMMIT_EDITMSG

Nit:

    test_grep "^# Changes to be committed:$"

> +'
> +
>  test_orig_head_helper () {
>  	test_when_finished 'git rebase --abort &&
>  		git checkout topic &&
> --
> 2.49.0

Re: [GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

On Thu, Jun 26, 2025 at 9:10 PM Kristoffer Haugsbakk
<kristofferhaugsbakk@fastmail.com> wrote:

> >
> > +test_expect_success 'no change in comment character due to conflicts
> > markers with core.commentChar=auto' '
> > +     test_commit base file &&
> > +     git checkout -b branch-a &&
> > +     test_commit A file &&
> > +     git checkout -b branch-b base &&
> > +     test_commit B file &&
> > +     test_must_fail git rebase branch-a &&
> > +     printf "B\nA\n" >file &&
> > +     git add file &&
> > +     write_script fake-editor <<-\EOF &&
> > +     exit 0
> > +     EOF
> > +     FAKE_EDITOR="$(pwd)/fake-editor" &&
> > +     GIT_EDITOR="\"\$FAKE_EDITOR\"" git -c core.commentChar=auto rebase --continue &&
>
> How about
>
>     GIT_EDITOR="cat >actual"
>
> Then you can `test_grep` on that.  Like in
>
> https://lore.kernel.org/git/5ed77fab-678d-4a06-bbd0-ea25462a7562@gmail.com/
>
> > +     # Check that "#" is still the comment character.
> > +     test_grep "# Changes to be committed:" .git/COMMIT_EDITMSG
>

Thanks, that's much cleaner and faster!

> Nit:
>
>     test_grep "^# Changes to be committed:$"
>
Thanks for the catch. I'll fix it.

[GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

When core.commentChar is set to "auto", Git selects a comment character
by scanning the commit message contents and avoiding any character
already present in the message.

If the message still contains old conflict comments (starting with a
comment character), Git assumes that character is in use and chooses a
different one. As a result, those existing comment lines are no longer
recognized as comments and end up being included in the final commit
message.

To avoid this, skip scanning the trailing comment block when selecting
the comment character. This allows Git to safely reuse the original
character when appropriate, keeping the commit message clean and free of
leftover conflict information.

Background:

The "auto" value for core.commentchar was introduced in the commit
84c9dc2c5a (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17) but did not exhibt this issue at that time.

The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
with conflicts, 2024-04-18) where Git started writing conflict comments
to the file at 'rebase_path_message()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---

Thanks to Christian for mentoring, and to Kristopher and Junio for their reviews!

 builtin/commit.c           |  6 +++++-
 t/t3418-rebase-continue.sh | 14 ++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index fba0dded64..63e7158e98 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	char candidates[] = "#;@!$%^&|:";
 	char *candidate;
 	const char *p;
+	size_t cutoff;
+
+	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
+	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
 
 	if (!memchr(sb->buf, candidates[0], sb->len)) {
 		free(comment_line_str_to_free);
@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	candidate = strchr(candidates, *p);
 	if (candidate)
 		*candidate = ' ';
-	for (p = sb->buf; *p; p++) {
+	for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
 		if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
 			candidate = strchr(candidates, p[1]);
 			if (candidate)
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 127216f722..ccfe77af6c 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -328,6 +328,20 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
 	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
 '
 
+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
+	test_commit base file &&
+	git checkout -b branch-a &&
+	test_commit A file &&
+	git checkout -b branch-b base &&
+	test_commit B file &&
+	test_must_fail git rebase branch-a &&
+	printf "B\nA\n" >file &&
+	git add file &&
+	GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
+	# Check that "#" is still the comment character.
+	test_grep "^# Changes to be committed:$" actual
+'
+
 test_orig_head_helper () {
 	test_when_finished 'git rebase --abort &&
 		git checkout topic &&
-- 
2.49.0

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Phillip Wood]

Hi Ayush

On 26/06/2025 23:16, Ayush Chandekar wrote:
> When core.commentChar is set to "auto", Git selects a comment character
> by scanning the commit message contents and avoiding any character
> already present in the message.
> 
> If the message still contains old conflict comments (starting with a
> comment character), Git assumes that character is in use and chooses a
> different one. As a result, those existing comment lines are no longer
> recognized as comments and end up being included in the final commit
> message.
> 
> To avoid this, skip scanning the trailing comment block when selecting
> the comment character. This allows Git to safely reuse the original
> character when appropriate, keeping the commit message clean and free of
> leftover conflict information.

This is a good explanation of the problem. Maybe this is another reason 
to consider removing support for commentChar=auto [1]

[1] https://lore.kernel.org/git/xmqqa59i45wc.fsf@gitster.g/

> Background:
> 
> The "auto" value for core.commentchar was introduced in the commit
> 84c9dc2c5a (commit: allow core.commentChar=auto for character auto
> selection, 2014-05-17) but did not exhibt this issue at that time.
> 
> The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
> with conflicts, 2024-04-18) where Git started writing conflict comments
> to the file at 'rebase_path_message()'.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---
> 
> Thanks to Christian for mentoring, and to Kristopher and Junio for their reviews!
> 
>   builtin/commit.c           |  6 +++++-
>   t/t3418-rebase-continue.sh | 14 ++++++++++++++
>   2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index fba0dded64..63e7158e98 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)
>   	char candidates[] = "#;@!$%^&|:";
>   	char *candidate;
>   	const char *p;
> +	size_t cutoff;
> +
> +	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
> +	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);

This finds the "Conflicts:" line. I was surprised to see that the string 
it looks for is hard coded and not translated, however the sequencer 
(also surprisingly) does not translate that message either so it should 
work.

>   
>   	if (!memchr(sb->buf, candidates[0], sb->len)) {
>   		free(comment_line_str_to_free);
> @@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
>   	candidate = strchr(candidates, *p);
>   	if (candidate)
>   		*candidate = ' ';
> -	for (p = sb->buf; *p; p++) {
> +	for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
>   		if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
>   			candidate = strchr(candidates, p[1]);
>   			if (candidate)
> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index 127216f722..ccfe77af6c 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -328,6 +328,20 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
>   	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
>   '
>   
> +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
> +	test_commit base file &&

If you used an existing file (F1 or F2) like most of the rest of the 
tests in this file we could avoid creating this commit and save 
ourselves a couple of processes.

> +	git checkout -b branch-a &&
> +	test_commit A file &&
> +	git checkout -b branch-b base &&
> +	test_commit B file &&
> +	test_must_fail git rebase branch-a &&
> +	printf "B\nA\n" >file &&
> +	git add file &&
> +	GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
> +	# Check that "#" is still the comment character.
> +	test_grep "^# Changes to be committed:$" actual

I agree that it is a good idea to anchor the start of the message, but 
I'm not sure it is helpful to anchor the end of the message as we don't 
want the test to fail just because an unrelated change adds some 
whitespace to the end of this line. I'd be tempted to drop the ':' for 
the same reason.

Thanks for fixing this

Phillip

> +'
> +
>   test_orig_head_helper () {
>   	test_when_finished 'git rebase --abort &&
>   		git checkout topic &&

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

>> +	size_t cutoff;
>> +
>> +	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
>> +	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
>
> This finds the "Conflicts:" line. I was surprised to see that the
> string it looks for is hard coded and not translated, however the
> sequencer (also surprisingly) does not translate that message either
> so it should work.

There is a funny chicken-and-egg problem, though.  It limits the
search for "Conflicts" by using wt_status_locate_end() based on the
current value of comment_line_str.  When core.commentstring is set
to "auto", the code that reads the configuration does not touch the
comment_line_str variable, which is initialized to '#'.  So

	[core]
	    commentstring = '%'
	    commentstring = auto

would have '%' in comment_line_str upon entering this codepath, let
wt_status_locate_end() use '%' as the comment string to find the end
of the log message, and then looks for "Conflicts:" in the result.

Which may or may not be what you want.

> If you used an existing file (F1 or F2) like most of the rest of the
> tests in this file we could avoid creating this commit and save
> ourselves a couple of processes.

Excellent suggestion.

>> +	test_grep "^# Changes to be committed:$" actual
>
> I agree that it is a good idea to anchor the start of the message, but
> I'm not sure it is helpful to anchor the end of the message as we
> don't want the test to fail just because an unrelated change adds some
> whitespace to the end of this line. I'd be tempted to drop the ':' for
> the same reason.

Again, excellent.


> Thanks for fixing this
>
> Phillip

Thanks.

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

On Fri, Jun 27, 2025 at 8:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> >> +    size_t cutoff;
> >> +
> >> +    /* Ignore comment chars in trailing comments (e.g., Conflicts:) */
> >> +    cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
> >
> > This finds the "Conflicts:" line. I was surprised to see that the
> > string it looks for is hard coded and not translated, however the
> > sequencer (also surprisingly) does not translate that message either
> > so it should work.
>
> There is a funny chicken-and-egg problem, though.  It limits the
> search for "Conflicts" by using wt_status_locate_end() based on the
> current value of comment_line_str.  When core.commentstring is set
> to "auto", the code that reads the configuration does not touch the
> comment_line_str variable, which is initialized to '#'.  So
>
>         [core]
>             commentstring = '%'
>             commentstring = auto
>
> would have '%' in comment_line_str upon entering this codepath, let
> wt_status_locate_end() use '%' as the comment string to find the end
> of the log message, and then looks for "Conflicts:" in the result.
>
> Which may or may not be what you want.
>

This is also being used to append signoff, just before the
`adjust_comment_line_char()` function.
Another thing that could be done is to return the function
(adjust_comment_line_char()) when we find a "Conflicts:" line. Because
I don't think there's sense in adjusting the comment character when
the "Conflicts:" line already has a comment character. But, I would
like to have some views on this.

Thanks.

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Phillip Wood]

On 27/06/2025 15:52, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>>> +	size_t cutoff;
>>> +
>>> +	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
>>> +	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
>>
>> This finds the "Conflicts:" line. I was surprised to see that the
>> string it looks for is hard coded and not translated, however the
>> sequencer (also surprisingly) does not translate that message either
>> so it should work.
> 
> There is a funny chicken-and-egg problem, though.  It limits the
> search for "Conflicts" by using wt_status_locate_end() based on the
> current value of comment_line_str.  When core.commentstring is set
> to "auto", the code that reads the configuration does not touch the
> comment_line_str variable, which is initialized to '#'.  So
> 
> 	[core]
> 	    commentstring = '%'
> 	    commentstring = auto
> 
> would have '%' in comment_line_str upon entering this codepath, let
> wt_status_locate_end() use '%' as the comment string to find the end
> of the log message, and then looks for "Conflicts:" in the result.
> 
> Which may or may not be what you want.

Oh, good point - I'd not looked at the config parsing. So we'd create 
conflict comments that look like

     % Conflicts:
     %    some-file.c

but so long as the commit message did not contain a '#' character [1] 
"git commit" would select '#' as the automatic comment char and our 
conflicts lines would not be treated as a comment.

Should we be resetting comment_line_str to '#' when core.commentString 
is set to "auto"? That wont help if the commit message contains a '#' 
but at least it would be consistently broken.

We could move adjust_comment_line_char() into libgit.a, use that to 
select the comment character used in append_conflicts_hint() and set 
core.commentString to that character when we run "git commit". I think 
doing that would mean that appending conflict comments would always work 
with core.commentString=auto but it is a more complex solution as we 
would need to remember the comment character and then pass it to git 
commit once the user had fixed the conflicts.

One final note - although the commit message mentions a change to "git 
rebase" I think this problem already affected cherry-pick, revert and 
merge before that change. In practice I suspect it is only cherry-pick 
where one is likely to see this problem because the template messages 
for the other two commands are unlikely to contain a '#'.

Thanks

Phillip

[1] For some reason adjust_comment_line_char() will not select '#' as 
the comment char if it occurs anywhere in the message but the other 
candidates are selected so long as they are not the first character on 
any line.

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

On Sat, Jun 28, 2025 at 7:08 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 27/06/2025 15:52, Junio C Hamano wrote:
> > Phillip Wood <phillip.wood123@gmail.com> writes:
> >
> >>> +   size_t cutoff;
> >>> +
> >>> +   /* Ignore comment chars in trailing comments (e.g., Conflicts:) */
> >>> +   cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
> >>
> >> This finds the "Conflicts:" line. I was surprised to see that the
> >> string it looks for is hard coded and not translated, however the
> >> sequencer (also surprisingly) does not translate that message either
> >> so it should work.
> >
> > There is a funny chicken-and-egg problem, though.  It limits the
> > search for "Conflicts" by using wt_status_locate_end() based on the
> > current value of comment_line_str.  When core.commentstring is set
> > to "auto", the code that reads the configuration does not touch the
> > comment_line_str variable, which is initialized to '#'.  So
> >
> >       [core]
> >           commentstring = '%'
> >           commentstring = auto
> >
> > would have '%' in comment_line_str upon entering this codepath, let
> > wt_status_locate_end() use '%' as the comment string to find the end
> > of the log message, and then looks for "Conflicts:" in the result.
> >
> > Which may or may not be what you want.
>
> Oh, good point - I'd not looked at the config parsing. So we'd create
> conflict comments that look like
>
>      % Conflicts:
>      %    some-file.c
>
> but so long as the commit message did not contain a '#' character [1]
> "git commit" would select '#' as the automatic comment char and our
> conflicts lines would not be treated as a comment.
>
> Should we be resetting comment_line_str to '#' when core.commentString
> is set to "auto"? That wont help if the commit message contains a '#'
> but at least it would be consistently broken.
>
> We could move adjust_comment_line_char() into libgit.a, use that to
> select the comment character used in append_conflicts_hint() and set
> core.commentString to that character when we run "git commit". I think
> doing that would mean that appending conflict comments would always work
> with core.commentString=auto but it is a more complex solution as we
> would need to remember the comment character and then pass it to git
> commit once the user had fixed the conflicts.
>

So, my GSoC project is refactoring in order to reduce the global state
in Git. I was trying to remove the global variables related to comment
characters. What I tried is to create one single function which
returns the comment string, and we could then pass a strbuf in case of
core.commentString=auto. You can check my attempts on my fork here [2]
(check repo_get_comment_line_str() in config.c), and also mentioned
this in my blog [3]. I thought I had it figured out, but turns out I
failed one test where core.commentString=auto. It was that moment I
realised that I would need to remember the comment character or the
strbuf in functions. Just wanted to share this in case anything
strikes you when looking at the approach.

> One final note - although the commit message mentions a change to "git
> rebase" I think this problem already affected cherry-pick, revert and
> merge before that change. In practice I suspect it is only cherry-pick
> where one is likely to see this problem because the template messages
> for the other two commands are unlikely to contain a '#'.
>
> Thanks
>
> Phillip
>
> [1] For some reason adjust_comment_line_char() will not select '#' as
> the comment char if it occurs anywhere in the message but the other
> candidates are selected so long as they are not the first character on
> any line.

Thanks:)

[2] : https://github.com/ayu-ch/git/commits/comment-line-str-4
[3] : https://ayu-ch.github.io/2025/06/09/gsoc-week-1.html

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Phillip Wood]

Hi Ayush

On 28/06/2025 15:33, Ayush Chandekar wrote:
> 
> So, my GSoC project is refactoring in order to reduce the global state
> in Git. I was trying to remove the global variables related to comment
> characters. What I tried is to create one single function which
> returns the comment string, and we could then pass a strbuf in case of
> core.commentString=auto. You can check my attempts on my fork here [2]
> (check repo_get_comment_line_str() in config.c), and also mentioned
> this in my blog [3]. I thought I had it figured out, but turns out I
> failed one test where core.commentString=auto. It was that moment I
> realised that I would need to remember the comment character or the
> strbuf in functions. Just wanted to share this in case anything
> strikes you when looking at the approach.

Thanks for that context. I'm not sure about having a single function 
that handles both cases. There is only one caller that cares about 
"auto" and the support for that has so many corner cases that don't work 
I'm putting a patch together to deprecate it and remove it when Git 3.0 
is released.

Looking at your code it seems to break the "last one wins" between 
core.commentChar and core.commentString. Also deferring the parsing 
until the comment character is used changes the behavior of things like 
"git -c core.commentchar=$'\n' commit -p". Instead of erroring out 
straight away it will let the user carefully select what they want to 
commit and then die which is not very user friendly.

Keeping the current parsing logic and storing the result in struct 
repository might be a better approach though we should think about how 
commands that run without a repository will be able to access the system 
and user config settings.

Thanks

Phillip

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

On Mon, Jun 30, 2025 at 2:29 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Ayush
>
> On 28/06/2025 15:33, Ayush Chandekar wrote:
> >
> > So, my GSoC project is refactoring in order to reduce the global state
> > in Git. I was trying to remove the global variables related to comment
> > characters. What I tried is to create one single function which
> > returns the comment string, and we could then pass a strbuf in case of
> > core.commentString=auto. You can check my attempts on my fork here [2]
> > (check repo_get_comment_line_str() in config.c), and also mentioned
> > this in my blog [3]. I thought I had it figured out, but turns out I
> > failed one test where core.commentString=auto. It was that moment I
> > realised that I would need to remember the comment character or the
> > strbuf in functions. Just wanted to share this in case anything
> > strikes you when looking at the approach.
>
> Thanks for that context. I'm not sure about having a single function
> that handles both cases. There is only one caller that cares about
> "auto" and the support for that has so many corner cases that don't work
> I'm putting a patch together to deprecate it and remove it when Git 3.0
> is released.
>
> Looking at your code it seems to break the "last one wins" between
> core.commentChar and core.commentString. Also deferring the parsing
> until the comment character is used changes the behavior of things like
> "git -c core.commentchar=$'\n' commit -p". Instead of erroring out
> straight away it will let the user carefully select what they want to
> commit and then die which is not very user friendly.
>

Thanks for taking a look! I was experimenting with this to see if I
could simplify the logic, but I didn't realize upfront how many corner
cases it would run into.

> Keeping the current parsing logic and storing the result in struct
> repository might be a better approach though we should think about how
> commands that run without a repository will be able to access the system
> and user config settings.
>

Yeah, I will follow that approach as I did with other patch series.

> Thanks
>
> Phillip
>

Thanks a lot!

Ayush:)

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Phillip Wood]

On 28/06/2025 14:38, Phillip Wood wrote:
> [1] For some reason adjust_comment_line_char() will not select '#' as 
> the comment char if it occurs anywhere in the message but the other 
> candidates are selected so long as they are not the first character on 
> any line.

Sorry, forget that - I'd misread the code

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Should we be resetting comment_line_str to '#' when core.commentString
> is set to "auto"? That wont help if the commit message contains a '#'
> but at least it would be consistently broken.

Yeah, while I was re-reading the code to parse the configuration file,
that was exactly what came to my mind.  I offhand did not think of any
downside of doing so, but I cannot claim that I have spent enough brain
cycles to make sure it is free of bad unintended consequences.

Thanks.

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

On Fri, Jun 27, 2025 at 2:04 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >
> >       if (!memchr(sb->buf, candidates[0], sb->len)) {
> >               free(comment_line_str_to_free);
> > @@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
> >       candidate = strchr(candidates, *p);
> >       if (candidate)
> >               *candidate = ' ';
> > -     for (p = sb->buf; *p; p++) {
> > +     for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
> >               if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
> >                       candidate = strchr(candidates, p[1]);
> >                       if (candidate)
> > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> > index 127216f722..ccfe77af6c 100755
> > --- a/t/t3418-rebase-continue.sh
> > +++ b/t/t3418-rebase-continue.sh
> > @@ -328,6 +328,20 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
> >       test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
> >   '
> >
> > +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
> > +     test_commit base file &&
>
> If you used an existing file (F1 or F2) like most of the rest of the
> tests in this file we could avoid creating this commit and save
> ourselves a couple of processes.
>

Yeah, right, I'll update it.

> > +     git checkout -b branch-a &&
> > +     test_commit A file &&
> > +     git checkout -b branch-b base &&
> > +     test_commit B file &&
> > +     test_must_fail git rebase branch-a &&
> > +     printf "B\nA\n" >file &&
> > +     git add file &&
> > +     GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
> > +     # Check that "#" is still the comment character.
> > +     test_grep "^# Changes to be committed:$" actual
>
> I agree that it is a good idea to anchor the start of the message, but
> I'm not sure it is helpful to anchor the end of the message as we don't
> want the test to fail just because an unrelated change adds some
> whitespace to the end of this line. I'd be tempted to drop the ':' for
> the same reason.
>

Makes sense, I'll fix it.
Thanks a lot for reviewing!

Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Christian Couder]

On Fri, Jun 27, 2025 at 12:17 AM Ayush Chandekar
<ayu.chandekar@gmail.com> wrote:

> The "auto" value for core.commentchar was introduced in the commit
> 84c9dc2c5a (commit: allow core.commentChar=auto for character auto
> selection, 2014-05-17) but did not exhibt this issue at that time.

Nit: s/exhibt/exhibit/

Thanks!

[GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

When core.commentChar is set to "auto", Git selects a comment character
by scanning the commit message contents and avoiding any character
already present in the message.

If the message still contains old conflict comments (starting with a
comment character), Git assumes that character is in use and chooses a
different one. As a result, those existing comment lines are no longer
recognized as comments and end up being included in the final commit
message.

To avoid this, skip scanning the trailing comment block when selecting
the comment character. This allows Git to safely reuse the original
character when appropriate, keeping the commit message clean and free of
leftover conflict information.

Background:

The "auto" value for core.commentchar was introduced in the commit
84c9dc2c5a (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17) but did not exhibit this issue at that time.

The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
with conflicts, 2024-04-18) where Git started writing conflict comments
to the file at 'rebase_path_message()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---

Thanks to Christian, Kristoffer, Junio and Phillip for their reviews!

Range-diff with v2:
1:  4e74e7a9a6 ! 1:  693f890a36 commit: avoid scanning trailing comments when 'core.commentChar' is "auto"
    @@ Commit message
     
         The "auto" value for core.commentchar was introduced in the commit
         84c9dc2c5a (commit: allow core.commentChar=auto for character auto
    -    selection, 2014-05-17) but did not exhibt this issue at that time.
    +    selection, 2014-05-17) but did not exhibit this issue at that time.
     
         The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
         with conflicts, 2024-04-18) where Git started writing conflict comments
    @@ t/t3418-rebase-continue.sh: test_expect_success 'there is no --no-reschedule-fai
      '
      
     +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
    -+	test_commit base file &&
     +	git checkout -b branch-a &&
    -+	test_commit A file &&
    -+	git checkout -b branch-b base &&
    -+	test_commit B file &&
    ++	test_commit A F1 &&
    ++	git checkout -b branch-b HEAD^ &&
    ++	test_commit B F1 &&
     +	test_must_fail git rebase branch-a &&
    -+	printf "B\nA\n" >file &&
    -+	git add file &&
    ++	printf "B\nA\n" >F1 &&
    ++	git add F1 &&
     +	GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
     +	# Check that "#" is still the comment character.
    -+	test_grep "^# Changes to be committed:$" actual
    ++	test_grep "^# Changes to be committed" actual
     +'
     +
      test_orig_head_helper () {


 builtin/commit.c           |  6 +++++-
 t/t3418-rebase-continue.sh | 13 +++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index fba0dded64..63e7158e98 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	char candidates[] = "#;@!$%^&|:";
 	char *candidate;
 	const char *p;
+	size_t cutoff;
+
+	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
+	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
 
 	if (!memchr(sb->buf, candidates[0], sb->len)) {
 		free(comment_line_str_to_free);
@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	candidate = strchr(candidates, *p);
 	if (candidate)
 		*candidate = ' ';
-	for (p = sb->buf; *p; p++) {
+	for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
 		if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
 			candidate = strchr(candidates, p[1]);
 			if (candidate)
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 127216f722..b8a8dd77e7 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -328,6 +328,19 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
 	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
 '
 
+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
+	git checkout -b branch-a &&
+	test_commit A F1 &&
+	git checkout -b branch-b HEAD^ &&
+	test_commit B F1 &&
+	test_must_fail git rebase branch-a &&
+	printf "B\nA\n" >F1 &&
+	git add F1 &&
+	GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
+	# Check that "#" is still the comment character.
+	test_grep "^# Changes to be committed" actual
+'
+
 test_orig_head_helper () {
 	test_when_finished 'git rebase --abort &&
 		git checkout topic &&
-- 
2.49.0

Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Phillip Wood]

Hi Ayush

On 30/06/2025 19:25, Ayush Chandekar wrote:
> 
> Range-diff with v2:
> 1:  4e74e7a9a6 ! 1:  693f890a36 commit: avoid scanning trailing comments when 'core.commentChar' is "auto"
>      @@ Commit message
>       
>           The "auto" value for core.commentchar was introduced in the commit
>           84c9dc2c5a (commit: allow core.commentChar=auto for character auto
>      -    selection, 2014-05-17) but did not exhibt this issue at that time.
>      +    selection, 2014-05-17) but did not exhibit this issue at that time.
>       
>           The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
>           with conflicts, 2024-04-18) where Git started writing conflict comments
>      @@ t/t3418-rebase-continue.sh: test_expect_success 'there is no --no-reschedule-fai
>        '
>        
>       +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
>      -+	test_commit base file &&
>       +	git checkout -b branch-a &&
>      -+	test_commit A file &&
>      -+	git checkout -b branch-b base &&
>      -+	test_commit B file &&
>      ++	test_commit A F1 &&
>      ++	git checkout -b branch-b HEAD^ &&
>      ++	test_commit B F1 &&
>       +	test_must_fail git rebase branch-a &&
>      -+	printf "B\nA\n" >file &&
>      -+	git add file &&
>      ++	printf "B\nA\n" >F1 &&
>      ++	git add F1 &&
>       +	GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
>       +	# Check that "#" is still the comment character.
>      -+	test_grep "^# Changes to be committed:$" actual
>      ++	test_grep "^# Changes to be committed" actual
>       +'
>       +
>        test_orig_head_helper () {

The changes here look good but I think we want to update the config 
parsing as well so that comment_line_str is reset to '#' when 
core.commentString=auto. We probably want to do that in its own commit.

Thanks

Phillip

Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

On Tue, Jul 1, 2025 at 6:47 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Ayush
>
> On 30/06/2025 19:25, Ayush Chandekar wrote:
> >
> > Range-diff with v2:
> > 1:  4e74e7a9a6 ! 1:  693f890a36 commit: avoid scanning trailing comments when 'core.commentChar' is "auto"
> >      @@ Commit message
> >
> >           The "auto" value for core.commentchar was introduced in the commit
> >           84c9dc2c5a (commit: allow core.commentChar=auto for character auto
> >      -    selection, 2014-05-17) but did not exhibt this issue at that time.
> >      +    selection, 2014-05-17) but did not exhibit this issue at that time.
> >
> >           The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
> >           with conflicts, 2024-04-18) where Git started writing conflict comments
> >      @@ t/t3418-rebase-continue.sh: test_expect_success 'there is no --no-reschedule-fai
> >        '
> >
> >       +test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
> >      -+       test_commit base file &&
> >       +       git checkout -b branch-a &&
> >      -+       test_commit A file &&
> >      -+       git checkout -b branch-b base &&
> >      -+       test_commit B file &&
> >      ++       test_commit A F1 &&
> >      ++       git checkout -b branch-b HEAD^ &&
> >      ++       test_commit B F1 &&
> >       +       test_must_fail git rebase branch-a &&
> >      -+       printf "B\nA\n" >file &&
> >      -+       git add file &&
> >      ++       printf "B\nA\n" >F1 &&
> >      ++       git add F1 &&
> >       +       GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
> >       +       # Check that "#" is still the comment character.
> >      -+       test_grep "^# Changes to be committed:$" actual
> >      ++       test_grep "^# Changes to be committed" actual
> >       +'
> >       +
> >        test_orig_head_helper () {
>
> The changes here look good but I think we want to update the config
> parsing as well so that comment_line_str is reset to '#' when
> core.commentString=auto. We probably want to do that in its own commit.
>
> Thanks
>
> Phillip
>

maybe something like this?

--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -912,8 +912,10 @@ static int prepare_to_commit(const char
*index_file, const char *prefix,
        if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
                die_errno(_("could not write commit template"));

-       if (auto_comment_line_char)
+       if (auto_comment_line_char){
+               comment_line_str = "#";
                adjust_comment_line_char(&sb);
+       }
        strbuf_release(&sb);

or we can do it inside the `adjust_comment_line()` function.

Thanks!

Ayush

Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Phillip Wood]

Hi Ayush

On 01/07/2025 19:33, Ayush Chandekar wrote:
> On Tue, Jul 1, 2025 at 6:47 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> The changes here look good but I think we want to update the config
>> parsing as well so that comment_line_str is reset to '#' when
>> core.commentString=auto. We probably want to do that in its own commit.
> 
> maybe something like this?
> 
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -912,8 +912,10 @@ static int prepare_to_commit(const char
> *index_file, const char *prefix,
>          if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
>                  die_errno(_("could not write commit template"));
> 
> -       if (auto_comment_line_char)
> +       if (auto_comment_line_char){
> +               comment_line_str = "#";
>                  adjust_comment_line_char(&sb);
> +       }
>          strbuf_release(&sb);
> 
> or we can do it inside the `adjust_comment_line()` function.

We need to do it when we parse the config so that 
append_conflicts_comment() uses '#' as the comment char. See the 
(whitespace damaged) diff below

Thanks

Phillip

diff --git a/config.c b/config.c
index eb60c293ab3..bb75bdc65d3 100644
--- a/config.c
+++ b/config.c
@@ -1537,9 +1537,11 @@ static int git_default_core_config(const char 
*var, const char *value,
              !strcmp(var, "core.commentstring")) {
                  if (!value)
                          return config_error_nonbool(var);
-                else if (!strcasecmp(value, "auto"))
+                else if (!strcasecmp(value, "auto")) {
                          auto_comment_line_char = 1;
-                else if (value[0]) {
+                        FREE_AND_NULL(comment_line_str_to_free);
+                        comment_line_str = "#";
+                } else if (value[0]) {
                          if (strchr(value, '\n'))
                                  return error(_("%s cannot contain 
newline"), var);
                          comment_line_str = value;

Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

Hi Phillip,

On Wed, Jul 2, 2025 at 1:02 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Ayush
>
> On 01/07/2025 19:33, Ayush Chandekar wrote:
> > On Tue, Jul 1, 2025 at 6:47 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >>
> >> The changes here look good but I think we want to update the config
> >> parsing as well so that comment_line_str is reset to '#' when
> >> core.commentString=auto. We probably want to do that in its own commit.
> >
> > maybe something like this?
> >
> > --- a/builtin/commit.c
> > +++ b/builtin/commit.c
> > @@ -912,8 +912,10 @@ static int prepare_to_commit(const char
> > *index_file, const char *prefix,
> >          if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
> >                  die_errno(_("could not write commit template"));
> >
> > -       if (auto_comment_line_char)
> > +       if (auto_comment_line_char){
> > +               comment_line_str = "#";
> >                  adjust_comment_line_char(&sb);
> > +       }
> >          strbuf_release(&sb);
> >
> > or we can do it inside the `adjust_comment_line()` function.
>
> We need to do it when we parse the config so that
> append_conflicts_comment() uses '#' as the comment char. See the
> (whitespace damaged) diff below
>
> Thanks
>
> Phillip
>
> diff --git a/config.c b/config.c
> index eb60c293ab3..bb75bdc65d3 100644
> --- a/config.c
> +++ b/config.c
> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char
> *var, const char *value,
>               !strcmp(var, "core.commentstring")) {
>                   if (!value)
>                           return config_error_nonbool(var);
> -                else if (!strcasecmp(value, "auto"))
> +                else if (!strcasecmp(value, "auto")) {
>                           auto_comment_line_char = 1;
> -                else if (value[0]) {
> +                        FREE_AND_NULL(comment_line_str_to_free);
> +                        comment_line_str = "#";
> +                } else if (value[0]) {
>                           if (strchr(value, '\n'))
>                                   return error(_("%s cannot contain
> newline"), var);
>                           comment_line_str = value;
>

Thanks, I understood it.

What if we simply return the function `adjust_comment_line_char()` if
we get a non-zero value from `ignored_log_message_bytes()`, i.e we
won't scan the commit message in case conflict message exists, and we
let the old code exist as it is?

+       if(ignored_log_message_bytes(sb->buf, sb->len))
+               return;

Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Phillip Wood]

Hi Ayush

On 03/07/2025 00:46, Ayush Chandekar wrote:
> On Wed, Jul 2, 2025 at 1:02 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> diff --git a/config.c b/config.c
>> index eb60c293ab3..bb75bdc65d3 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char
>> *var, const char *value,
>>                !strcmp(var, "core.commentstring")) {
>>                    if (!value)
>>                            return config_error_nonbool(var);
>> -                else if (!strcasecmp(value, "auto"))
>> +                else if (!strcasecmp(value, "auto")) {
>>                            auto_comment_line_char = 1;
>> -                else if (value[0]) {
>> +                        FREE_AND_NULL(comment_line_str_to_free);
>> +                        comment_line_str = "#";
>> +                } else if (value[0]) {
>>                            if (strchr(value, '\n'))
>>                                    return error(_("%s cannot contain
>> newline"), var);
>>                            comment_line_str = value;
>>
> 
> Thanks, I understood it.
> 
> What if we simply return the function `adjust_comment_line_char()` if
> we get a non-zero value from `ignored_log_message_bytes()`, i.e we
> won't scan the commit message in case conflict message exists, and we
> let the old code exist as it is?
> 
> +       if(ignored_log_message_bytes(sb->buf, sb->len))
> +               return;

So we'd ignore core.commentChar=auto if we detected conflict comments? 
That might be surprising to the user - it would mean that we'd always 
avoid adding the conflict comments to the commit message but we'd lose 
any lines that begin with the comment string. I think I'm leaning 
slightly towards the original solution but it is not clear to me that 
one option is much better that the other.

Thanks

Phillip

Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

Hey, Phillip

On Fri, Jul 4, 2025 at 1:53 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Ayush
>
> On 03/07/2025 00:46, Ayush Chandekar wrote:
> > On Wed, Jul 2, 2025 at 1:02 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >> diff --git a/config.c b/config.c
> >> index eb60c293ab3..bb75bdc65d3 100644
> >> --- a/config.c
> >> +++ b/config.c
> >> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char
> >> *var, const char *value,
> >>                !strcmp(var, "core.commentstring")) {
> >>                    if (!value)
> >>                            return config_error_nonbool(var);
> >> -                else if (!strcasecmp(value, "auto"))
> >> +                else if (!strcasecmp(value, "auto")) {
> >>                            auto_comment_line_char = 1;
> >> -                else if (value[0]) {
> >> +                        FREE_AND_NULL(comment_line_str_to_free);
> >> +                        comment_line_str = "#";
> >> +                } else if (value[0]) {
> >>                            if (strchr(value, '\n'))
> >>                                    return error(_("%s cannot contain
> >> newline"), var);
> >>                            comment_line_str = value;
> >>
> >
> > Thanks, I understood it.
> >
> > What if we simply return the function `adjust_comment_line_char()` if
> > we get a non-zero value from `ignored_log_message_bytes()`, i.e we
> > won't scan the commit message in case conflict message exists, and we
> > let the old code exist as it is?
> >
> > +       if(ignored_log_message_bytes(sb->buf, sb->len))
> > +               return;
>
> So we'd ignore core.commentChar=auto if we detected conflict comments?
> That might be surprising to the user - it would mean that we'd always
> avoid adding the conflict comments to the commit message but we'd lose
> any lines that begin with the comment string. I think I'm leaning
> slightly towards the original solution but it is not clear to me that
> one option is much better that the other.
>
> Thanks
>
> Phillip
>

Now that we're planning to get rid of the 'auto' keyword from
commentChar [1], do you think it would be better if we just ignored
the keyword when we detect conflict comments? Also, how is it that a
user will end up having lines starting with the character being the
same as the conflict comment's character?

Thanks!
Ayush

[1]: https://lore.kernel.org/git/cover.1751983009.git.phillip.wood@dunelm.org.uk/

Re: [GSOC PATCH v3] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Phillip Wood]

Hi Ayush

On 08/07/2025 16:47, Ayush Chandekar wrote:
> 
> Now that we're planning to get rid of the 'auto' keyword from
> commentChar [1], do you think it would be better if we just ignored
> the keyword when we detect conflict comments? Also, how is it that a
> user will end up having lines starting with the character being the
> same as the conflict comment's character?

Let see if Junio agrees with depreciation and removing support for 
commentChar=auto first. If we do deprecate it then we should still 
support it until it is removed so I'm leaning towards fixing the config 
parsing to reset comment_line_char to '#' instead.

Thanks

Phillip

[GSOC PATCH 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages

[Ayush Chandekar]

Hey everyone,

The aim of this patch series is to improve the behaviour of core.commentChar=auto by the following patches:
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.
2/2 - Standardizes the behaviour of code by resetting the 'comment_line_str' to "#" when core.commentChar is set to auto. 

Thanks to Junio, Phillip and Kristoffer for reviewing the patches and also Christian for the reviews and mentoring me.

Ayush Chandekar (2):
  commit: avoid scanning trailing comments when 'core.commentChar' is
    "auto"
  config: set comment_line_str to "#" when core.commentChar=auto

 builtin/commit.c           |  6 +++++-
 config.c                   |  6 ++++--
 t/t3418-rebase-continue.sh | 13 +++++++++++++
 3 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.49.0

[GSOC PATCH 1/2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

When core.commentChar is set to "auto", Git selects a comment character
by scanning the commit message contents and avoiding any character
already present in the message.

If the message still contains old conflict comments (starting with a
comment character), Git assumes that character is in use and chooses a
different one. As a result, those existing comment lines are no longer
recognized as comments and end up being included in the final commit
message.

To avoid this, skip scanning the trailing comment block when selecting
the comment character. This allows Git to safely reuse the original
character when appropriate, keeping the commit message clean and free of
leftover conflict information.

Background:

The "auto" value for core.commentchar was introduced in the commit
84c9dc2c5a (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17) but did not exhibit this issue at that time.

The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
with conflicts, 2024-04-18) where Git started writing conflict comments
to the file at 'rebase_path_message()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/commit.c           |  6 +++++-
 t/t3418-rebase-continue.sh | 13 +++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index fba0dded64..63e7158e98 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	char candidates[] = "#;@!$%^&|:";
 	char *candidate;
 	const char *p;
+	size_t cutoff;
+
+	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
+	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
 
 	if (!memchr(sb->buf, candidates[0], sb->len)) {
 		free(comment_line_str_to_free);
@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	candidate = strchr(candidates, *p);
 	if (candidate)
 		*candidate = ' ';
-	for (p = sb->buf; *p; p++) {
+	for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
 		if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
 			candidate = strchr(candidates, p[1]);
 			if (candidate)
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 127216f722..b8a8dd77e7 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -328,6 +328,19 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
 	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
 '
 
+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
+	git checkout -b branch-a &&
+	test_commit A F1 &&
+	git checkout -b branch-b HEAD^ &&
+	test_commit B F1 &&
+	test_must_fail git rebase branch-a &&
+	printf "B\nA\n" >F1 &&
+	git add F1 &&
+	GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
+	# Check that "#" is still the comment character.
+	test_grep "^# Changes to be committed" actual
+'
+
 test_orig_head_helper () {
 	test_when_finished 'git rebase --abort &&
 		git checkout topic &&
-- 
2.49.0

[GSOC PATCH v4 1/2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

When core.commentChar is set to "auto", Git selects a comment character
by scanning the commit message contents and avoiding any character
already present in the message.

If the message still contains old conflict comments (starting with a
comment character), Git assumes that character is in use and chooses a
different one. As a result, those existing comment lines are no longer
recognized as comments and end up being included in the final commit
message.

To avoid this, skip scanning the trailing comment block when selecting
the comment character. This allows Git to safely reuse the original
character when appropriate, keeping the commit message clean and free of
leftover conflict information.

Background:

The "auto" value for core.commentchar was introduced in the commit
84c9dc2c5a (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17) but did not exhibit this issue at that time.

The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
with conflicts, 2024-04-18) where Git started writing conflict comments
to the file at 'rebase_path_message()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/commit.c           |  6 +++++-
 t/t3418-rebase-continue.sh | 13 +++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index fba0dded64..63e7158e98 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	char candidates[] = "#;@!$%^&|:";
 	char *candidate;
 	const char *p;
+	size_t cutoff;
+
+	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
+	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
 
 	if (!memchr(sb->buf, candidates[0], sb->len)) {
 		free(comment_line_str_to_free);
@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	candidate = strchr(candidates, *p);
 	if (candidate)
 		*candidate = ' ';
-	for (p = sb->buf; *p; p++) {
+	for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
 		if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
 			candidate = strchr(candidates, p[1]);
 			if (candidate)
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 127216f722..b8a8dd77e7 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -328,6 +328,19 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
 	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
 '
 
+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
+	git checkout -b branch-a &&
+	test_commit A F1 &&
+	git checkout -b branch-b HEAD^ &&
+	test_commit B F1 &&
+	test_must_fail git rebase branch-a &&
+	printf "B\nA\n" >F1 &&
+	git add F1 &&
+	GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
+	# Check that "#" is still the comment character.
+	test_grep "^# Changes to be committed" actual
+'
+
 test_orig_head_helper () {
 	test_when_finished 'git rebase --abort &&
 		git checkout topic &&
-- 
2.49.0

[GSOC PATCH 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Ayush Chandekar]

If conflict comments already use a comment character that isn't "#", and
core.commentChar is set "auto", Git will ignore these lines during the
scan using ignored_log_message_bytes() and pick a new comment character
based on the rest of the message. The newly chosen character may be
different from the one used in the conflict comments and therefore,
these are no longer treated as comments and end up in the final commit
message.

For example, during a rebase if the user previously set
core.commentChar=% and then encounters a conflict, conflict comments
like "% Conflicts:" are generated. If the user subsequently sets
core.commentChar=auto before running `rebase --continue`, Git parses the
"auto" setting and begins scanning. It first uses the existing
'comment_line_str' (which is '%') to detect and ignore conflict comments
via ignored_log_message_bytes().

Then, Git scans the rest of the message (excluding conflict comments),
sees that none of the remaining lines start with '#' and decides to set
comment_line_str to '#'. Since the final commit character differs from
the one used in the conflict comments, those lines are no longer
considered comments and get included in the final commit message.

Set 'comment_line_str' to '#' when core.commentChar is set to 'auto' to
reset any previously set value.

While this does not solve the issue of conflict comment inclusion and
the user visible behaviour stays tha same, it standardizes the behaviour
of the code by always resetting 'comment_line_str' to '#' when
core.commentChar=auto is parsed.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 config.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index eb60c293ab..bb75bdc65d 100644
--- a/config.c
+++ b/config.c
@@ -1537,9 +1537,11 @@ static int git_default_core_config(const char *var, const char *value,
 	    !strcmp(var, "core.commentstring")) {
 		if (!value)
 			return config_error_nonbool(var);
-		else if (!strcasecmp(value, "auto"))
+		else if (!strcasecmp(value, "auto")) {
 			auto_comment_line_char = 1;
-		else if (value[0]) {
+			FREE_AND_NULL(comment_line_str_to_free);
+			comment_line_str = "#";
+		} else if (value[0]) {
 			if (strchr(value, '\n'))
 				return error(_("%s cannot contain newline"), var);
 			comment_line_str = value;
-- 
2.49.0

[GSOC PATCH v4 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Ayush Chandekar]

If conflict comments already use a comment character that isn't "#", and
core.commentChar is set "auto", Git will ignore these lines during the
scan using ignored_log_message_bytes() and pick a new comment character
based on the rest of the message. The newly chosen character may be
different from the one used in the conflict comments and therefore,
these are no longer treated as comments and end up in the final commit
message.

For example, during a rebase if the user previously set
core.commentChar=% and then encounters a conflict, conflict comments
like "% Conflicts:" are generated. If the user subsequently sets
core.commentChar=auto before running `rebase --continue`, Git parses the
"auto" setting and begins scanning. It first uses the existing
'comment_line_str' (which is '%') to detect and ignore conflict comments
via ignored_log_message_bytes().

Then, Git scans the rest of the message (excluding conflict comments),
sees that none of the remaining lines start with '#' and decides to set
comment_line_str to '#'. Since the final commit character differs from
the one used in the conflict comments, those lines are no longer
considered comments and get included in the final commit message.

Set 'comment_line_str' to '#' when core.commentChar is set to 'auto' to
reset any previously set value.

While this does not solve the issue of conflict comment inclusion and
the user visible behaviour stays tha same, it standardizes the behaviour
of the code by always resetting 'comment_line_str' to '#' when
core.commentChar=auto is parsed.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 config.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index eb60c293ab..bb75bdc65d 100644
--- a/config.c
+++ b/config.c
@@ -1537,9 +1537,11 @@ static int git_default_core_config(const char *var, const char *value,
 	    !strcmp(var, "core.commentstring")) {
 		if (!value)
 			return config_error_nonbool(var);
-		else if (!strcasecmp(value, "auto"))
+		else if (!strcasecmp(value, "auto")) {
 			auto_comment_line_char = 1;
-		else if (value[0]) {
+			FREE_AND_NULL(comment_line_str_to_free);
+			comment_line_str = "#";
+		} else if (value[0]) {
 			if (strchr(value, '\n'))
 				return error(_("%s cannot contain newline"), var);
 			comment_line_str = value;
-- 
2.49.0

Re: [GSOC PATCH 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> If conflict comments already use a comment character that isn't "#", and
> core.commentChar is set "auto", Git will ignore these lines during the
> scan using ignored_log_message_bytes() and pick a new comment character
> based on the rest of the message. The newly chosen character may be
> different from the one used in the conflict comments and therefore,
> these are no longer treated as comments and end up in the final commit
> message.
>
> For example, during a rebase if the user previously set
> core.commentChar=% and then encounters a conflict, conflict comments
> like "% Conflicts:" are generated. If the user subsequently sets
> core.commentChar=auto before running `rebase --continue`, Git parses the
> "auto" setting and begins scanning. It first uses the existing
> 'comment_line_str' (which is '%') to detect and ignore conflict comments
> via ignored_log_message_bytes().
>
> Then, Git scans the rest of the message (excluding conflict comments),
> sees that none of the remaining lines start with '#' and decides to set
> comment_line_str to '#'. Since the final commit character differs from
> the one used in the conflict comments, those lines are no longer
> considered comments and get included in the final commit message.
>
> Set 'comment_line_str' to '#' when core.commentChar is set to 'auto' to
> reset any previously set value.
>
> While this does not solve the issue of conflict comment inclusion and
> the user visible behaviour stays tha same, it standardizes the behaviour
> of the code by always resetting 'comment_line_str' to '#' when
> core.commentChar=auto is parsed.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---
>  config.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
>
> diff --git a/config.c b/config.c
> index eb60c293ab..bb75bdc65d 100644
> --- a/config.c
> +++ b/config.c
> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char *var, const char *value,
>  	    !strcmp(var, "core.commentstring")) {
>  		if (!value)
>  			return config_error_nonbool(var);
> -		else if (!strcasecmp(value, "auto"))
> +		else if (!strcasecmp(value, "auto")) {
>  			auto_comment_line_char = 1;
> -		else if (value[0]) {
> +			FREE_AND_NULL(comment_line_str_to_free);
> +			comment_line_str = "#";
> +		} else if (value[0]) {
>  			if (strchr(value, '\n'))
>  				return error(_("%s cannot contain newline"), var);
>  			comment_line_str = value;

This patch is exactly what Phillip suggested in

https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/

isn't it?  Makes sense to me.

Re: [GSOC PATCH 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Ayush Chandekar]

Hi Junio,

On Wed, Jul 16, 2025 at 2:53 AM Junio C Hamano <gitster@pobox.com> wrote:
>
[snip]
>
> This patch is exactly what Phillip suggested in
>
> https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/
>
> isn't it?  Makes sense to me.
>

Yes, you're right. I should add the suggested-by trailer for this patch.

Thanks
Ayush

Re: [GSOC PATCH 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> On Wed, Jul 16, 2025 at 2:53 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
> [snip]
>>
>> This patch is exactly what Phillip suggested in
>>
>> https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/
>>
>> isn't it?  Makes sense to me.
>>
>
> Yes, you're right. I should add the suggested-by trailer for this patch.

I am not sure about that, though.  A verbatim copy is stronger than
implementing what was suggested by another person.  If I were in
your position, I'll probably say something like

	The patch text was taken from Phillip Wood's message [*URL*],
	with the commit log message written by me.

	Based-on-a-patch-by: Phillip Wood <...>
	Signed-off-by: Ayush Chandekar <...>

In any case, this overlaps both textually but also intent-wise with
Phillip's "let's mark core.commentchar=auto deprecated and remove
the support at 3.0 boundary", which is planned to be rerolled to
make it a failure when the user uses core.commentchar=auto.  It
would be a while before we tag Git 3.0, so the fix in this topic
will be necessary until then.

Thanks.

Re: [GSOC PATCH 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Ayush Chandekar]

On Wed, Jul 16, 2025 at 5:00 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>
> > On Wed, Jul 16, 2025 at 2:53 AM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> > [snip]
> >>
> >> This patch is exactly what Phillip suggested in
> >>
> >> https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/
> >>
> >> isn't it?  Makes sense to me.
> >>
> >
> > Yes, you're right. I should add the suggested-by trailer for this patch.
>
> I am not sure about that, though.  A verbatim copy is stronger than
> implementing what was suggested by another person.  If I were in
> your position, I'll probably say something like
>
>         The patch text was taken from Phillip Wood's message [*URL*],
>         with the commit log message written by me.
>
>         Based-on-a-patch-by: Phillip Wood <...>
>         Signed-off-by: Ayush Chandekar <...>
>
> In any case, this overlaps both textually but also intent-wise with
> Phillip's "let's mark core.commentchar=auto deprecated and remove
> the support at 3.0 boundary", which is planned to be rerolled to
> make it a failure when the user uses core.commentchar=auto.  It
> would be a while before we tag Git 3.0, so the fix in this topic
> will be necessary until then.
>
> Thanks.
>

Yeah, Phillip should actually get the primary credit for this patch
and Suggested-by does not do enough justice.
I will send a new version right away.

Thanks!
Ayush

Re: [GSOC PATCH 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> Yeah, Phillip should actually get the primary credit for this patch
> and Suggested-by does not do enough justice.
> I will send a new version right away.

Thanks.  Don't forget to ask him to sign-off.

Re: [GSOC PATCH 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Phillip Wood]

On 16/07/2025 16:21, Junio C Hamano wrote:
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
> 
>> Yeah, Phillip should actually get the primary credit for this patch
>> and Suggested-by does not do enough justice.
>> I will send a new version right away.
> 
> Thanks.  Don't forget to ask him to sign-off.

Here it is

Signed-off-by: <Phillip Wood phillip.wood@dunelm.org.uk>

Re: [GSOC PATCH 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 16/07/2025 16:21, Junio C Hamano wrote:
>> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>> 
>>> Yeah, Phillip should actually get the primary credit for this patch
>>> and Suggested-by does not do enough justice.
>>> I will send a new version right away.
>> Thanks.  Don't forget to ask him to sign-off.
>
> Here it is
>
> Signed-off-by: <Phillip Wood phillip.wood@dunelm.org.uk>

Thanks.

[GSOC PATCH v4 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages

[Ayush Chandekar]

**Please ignore the patch series sent before this, accidentally missed out on mentioning the verison 'v4' in the subject.**

Hey everyone,

The aim of this patch series is to improve the behaviour of core.commentChar=auto by the following patches:
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.
2/2 - Standardizes the behaviour of code by resetting the 'comment_line_str' to "#" when 'core.commentChar' is set to "auto". 

Thanks to Junio, Phillip and Kristoffer for reviewing the patches and also Christian for the reviews and mentoring me.

Ayush Chandekar (2):
  commit: avoid scanning trailing comments when 'core.commentChar' is
    "auto"
  config: set comment_line_str to "#" when core.commentChar=auto

 builtin/commit.c           |  6 +++++-
 config.c                   |  6 ++++--
 t/t3418-rebase-continue.sh | 13 +++++++++++++
 3 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.49.0

[GSOC PATCH v5 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages

[Ayush Chandekar]

Hey everyone,

The aim of this patch series is to improve the behaviour of core.commentChar=auto by the following patches:
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.
2/2 - Standardizes the behaviour of code by resetting the 'comment_line_str' to "#" when 'core.commentChar' is set to "auto". 

Thanks to Junio, Phillip and Kristoffer for reviewing the patches and also Christian for the reviews and mentoring me.

The only difference between this version (v5) and the previous one is that I've added credit to Phillip for patch (2/2).

Ayush Chandekar (2):
  commit: avoid scanning trailing comments when 'core.commentChar' is
    "auto"
  config: set comment_line_str to "#" when core.commentChar=auto

 builtin/commit.c           |  6 +++++-
 config.c                   |  6 ++++--
 t/t3418-rebase-continue.sh | 13 +++++++++++++
 3 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.49.0

[GSOC PATCH v5 1/2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"

[Ayush Chandekar]

When core.commentChar is set to "auto", Git selects a comment character
by scanning the commit message contents and avoiding any character
already present in the message.

If the message still contains old conflict comments (starting with a
comment character), Git assumes that character is in use and chooses a
different one. As a result, those existing comment lines are no longer
recognized as comments and end up being included in the final commit
message.

To avoid this, skip scanning the trailing comment block when selecting
the comment character. This allows Git to safely reuse the original
character when appropriate, keeping the commit message clean and free of
leftover conflict information.

Background:

The "auto" value for core.commentchar was introduced in the commit
84c9dc2c5a (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17) but did not exhibit this issue at that time.

The bug was introduced in commit a6c2654f83 (rebase -m: fix --signoff
with conflicts, 2024-04-18) where Git started writing conflict comments
to the file at 'rebase_path_message()'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/commit.c           |  6 +++++-
 t/t3418-rebase-continue.sh | 13 +++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index fba0dded64..63e7158e98 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -688,6 +688,10 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	char candidates[] = "#;@!$%^&|:";
 	char *candidate;
 	const char *p;
+	size_t cutoff;
+
+	/* Ignore comment chars in trailing comments (e.g., Conflicts:) */
+	cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len);
 
 	if (!memchr(sb->buf, candidates[0], sb->len)) {
 		free(comment_line_str_to_free);
@@ -700,7 +704,7 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	candidate = strchr(candidates, *p);
 	if (candidate)
 		*candidate = ' ';
-	for (p = sb->buf; *p; p++) {
+	for (p = sb->buf; p + 1 < sb->buf + cutoff; p++) {
 		if ((p[0] == '\n' || p[0] == '\r') && p[1]) {
 			candidate = strchr(candidates, p[1]);
 			if (candidate)
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 127216f722..b8a8dd77e7 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -328,6 +328,19 @@ test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebas
 	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
 '
 
+test_expect_success 'no change in comment character due to conflicts markers with core.commentChar=auto' '
+	git checkout -b branch-a &&
+	test_commit A F1 &&
+	git checkout -b branch-b HEAD^ &&
+	test_commit B F1 &&
+	test_must_fail git rebase branch-a &&
+	printf "B\nA\n" >F1 &&
+	git add F1 &&
+	GIT_EDITOR="cat >actual" git -c core.commentChar=auto rebase --continue &&
+	# Check that "#" is still the comment character.
+	test_grep "^# Changes to be committed" actual
+'
+
 test_orig_head_helper () {
 	test_when_finished 'git rebase --abort &&
 		git checkout topic &&
-- 
2.49.0

[GSOC PATCH v5 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Ayush Chandekar]

If conflict comments already use a comment character that isn't "#", and
core.commentChar is set "auto", Git will ignore these lines during the
scan using ignored_log_message_bytes() and pick a new comment character
based on the rest of the message. The newly chosen character may be
different from the one used in the conflict comments and therefore,
these are no longer treated as comments and end up in the final commit
message.

For example, during a rebase if the user previously set
core.commentChar=% and then encounters a conflict, conflict comments
like "% Conflicts:" are generated. If the user subsequently sets
core.commentChar=auto before running `rebase --continue`, Git parses the
"auto" setting and begins scanning. It first uses the existing
'comment_line_str' (which is '%') to detect and ignore conflict comments
via ignored_log_message_bytes().

Then, Git scans the rest of the message (excluding conflict comments),
sees that none of the remaining lines start with '#' and decides to set
comment_line_str to '#'. Since the final commit character differs from
the one used in the conflict comments, those lines are no longer
considered comments and get included in the final commit message.

Set 'comment_line_str' to '#' when core.commentChar is set to 'auto' to
reset any previously set value.

While this does not solve the issue of conflict comment inclusion and
the user visible behaviour stays tha same, it standardizes the behaviour
of the code by always resetting 'comment_line_str' to '#' when
core.commentChar=auto is parsed.

The patch text is based on Phillip Wood's message:
https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/
and the commit log message is wriiten by me.

Based-on-a-patch-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 config.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index eb60c293ab..bb75bdc65d 100644
--- a/config.c
+++ b/config.c
@@ -1537,9 +1537,11 @@ static int git_default_core_config(const char *var, const char *value,
 	    !strcmp(var, "core.commentstring")) {
 		if (!value)
 			return config_error_nonbool(var);
-		else if (!strcasecmp(value, "auto"))
+		else if (!strcasecmp(value, "auto")) {
 			auto_comment_line_char = 1;
-		else if (value[0]) {
+			FREE_AND_NULL(comment_line_str_to_free);
+			comment_line_str = "#";
+		} else if (value[0]) {
 			if (strchr(value, '\n'))
 				return error(_("%s cannot contain newline"), var);
 			comment_line_str = value;
-- 
2.49.0

Re: [GSOC PATCH v5 2/2] config: set comment_line_str to "#" when core.commentChar=auto

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> If conflict comments already use a comment character that isn't "#", and
> ...
> The patch text is based on Phillip Wood's message:
> https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/
> and the commit log message is wriiten by me.
>
> Based-on-a-patch-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---

Earlier in response to your "Phillip should actually get the primary
credit" I said to ask for his sign-off, because I took it as you are
actually making Phillip the author of the patch.  But it is fine
either way.  Phillip has given his permission to add a sign-off, so
we have everything to move this topic forward.

Thanks, all!


>  config.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index eb60c293ab..bb75bdc65d 100644
> --- a/config.c
> +++ b/config.c
> @@ -1537,9 +1537,11 @@ static int git_default_core_config(const char *var, const char *value,
>  	    !strcmp(var, "core.commentstring")) {
>  		if (!value)
>  			return config_error_nonbool(var);
> -		else if (!strcasecmp(value, "auto"))
> +		else if (!strcasecmp(value, "auto")) {
>  			auto_comment_line_char = 1;
> -		else if (value[0]) {
> +			FREE_AND_NULL(comment_line_str_to_free);
> +			comment_line_str = "#";
> +		} else if (value[0]) {
>  			if (strchr(value, '\n'))
>  				return error(_("%s cannot contain newline"), var);
>  			comment_line_str = value;

Re: [GSOC PATCH v5 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages

[Phillip Wood]

Hi Ayush

On 16/07/2025 12:43, Ayush Chandekar wrote:
> 
> Hey everyone,
> 
> The aim of this patch series is to improve the behaviour of core.commentChar=auto by the following patches:
> 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.
> 2/2 - Standardizes the behaviour of code by resetting the 'comment_line_str' to "#" when 'core.commentChar' is set to "auto".

This version looks good to me, thanks for working on it.

Junio - shall I rebase 'pw/3.0-commentchar-auto-deprecation' on top of 
this when I re-roll to avoid conflicts?

Thanks

Phillip

> Thanks to Junio, Phillip and Kristoffer for reviewing the patches and also Christian for the reviews and mentoring me.
> 
> The only difference between this version (v5) and the previous one is that I've added credit to Phillip for patch (2/2).
> 
> Ayush Chandekar (2):
>    commit: avoid scanning trailing comments when 'core.commentChar' is
>      "auto"
>    config: set comment_line_str to "#" when core.commentChar=auto
> 
>   builtin/commit.c           |  6 +++++-
>   config.c                   |  6 ++++--
>   t/t3418-rebase-continue.sh | 13 +++++++++++++
>   3 files changed, 22 insertions(+), 3 deletions(-)
> 
J

Re: [GSOC PATCH v5 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> This version looks good to me, thanks for working on it.
>
> Junio - shall I rebase 'pw/3.0-commentchar-auto-deprecation' on top of
> this when I re-roll to avoid conflicts?

Sounds sensible.  I can drop my merge-fix then.

Thanks.