On Mon, Jun 30, 2025 at 2:29 PM Phillip Wood <phillip.wood123@xxxxxxxxx> 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:)
