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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jun 30, 2025, 08:59 UTC
Message-ID: <d25bf6c5-e56c-48d4-95b6-c714ee14ab78@gmail.com>
In-Reply-To: <CAE7as+aUcd65vPwwRh_C89vQbMjKQh0Y6LF7WDq1Whyj6iYfLg@mail.gmail.com>

Hi Ayush

On 28/06/2025 15:33, Ayush Chandekar wrote:

Show 12 quoted lines

> 
> 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