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
