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

["Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>]

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