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

From: Phillip Wood <[email-masked]>
Date: Mon, 30 Jun 2025 09:59:33 +0100

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