git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Ayush Chandekar <ayu.chandekar@gmail.com>, phillip.wood@dunelm.org.uk
Cc: Junio C Hamano <gitster@pobox.com>,
	christian.couder@gmail.com, git@vger.kernel.org,
	shyamthakkar001@gmail.com, kristofferhaugsbakk@fastmail.com
Subject: Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"
Date: Mon, 30 Jun 2025 09:59:33 +0100	[thread overview]
Message-ID: <d25bf6c5-e56c-48d4-95b6-c714ee14ab78@gmail.com> (raw)
In-Reply-To: <CAE7as+aUcd65vPwwRh_C89vQbMjKQh0Y6LF7WDq1Whyj6iYfLg@mail.gmail.com>

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


  reply	other threads:[~2025-06-30  8:59 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-26 13:22 [GSOC PATCH] commit: avoid scanning trailing comments when 'core.commentChar' is "auto" Ayush Chandekar
2025-06-26 14:33 ` Junio C Hamano
2025-06-26 21:30   ` Ayush Chandekar
2025-06-26 15:40 ` Kristoffer Haugsbakk
2025-06-26 21:28   ` Ayush Chandekar
2025-06-26 22:16 ` [GSOC PATCH v2] " Ayush Chandekar
2025-06-27  8:34   ` Phillip Wood
2025-06-27 14:52     ` Junio C Hamano
2025-06-28 10:37       ` Ayush Chandekar
2025-06-28 13:38       ` Phillip Wood
2025-06-28 14:33         ` Ayush Chandekar
2025-06-30  8:59           ` Phillip Wood [this message]
2025-06-30 17:34             ` Ayush Chandekar
2025-06-28 15:10         ` Phillip Wood
2025-06-30 14:11         ` Junio C Hamano
2025-06-28 10:18     ` Ayush Chandekar
2025-06-27  9:04   ` Christian Couder
2025-06-30 18:25 ` [GSOC PATCH v3] " Ayush Chandekar
2025-07-01 13:17   ` Phillip Wood
2025-07-01 18:33     ` Ayush Chandekar
2025-07-01 19:31       ` Phillip Wood
2025-07-02 23:46         ` Ayush Chandekar
2025-07-04  8:23           ` Phillip Wood
2025-07-08 15:47             ` Ayush Chandekar
2025-07-09 14:17               ` Phillip Wood
2025-07-15 18:51 ` [GSOC PATCH 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages Ayush Chandekar
2025-07-15 18:51   ` [GSOC PATCH 1/2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto" Ayush Chandekar
2025-07-15 18:57     ` [GSOC PATCH v4 " Ayush Chandekar
2025-07-15 18:51   ` [GSOC PATCH 2/2] config: set comment_line_str to "#" when core.commentChar=auto Ayush Chandekar
2025-07-15 18:57     ` [GSOC PATCH v4 " Ayush Chandekar
2025-07-15 21:23     ` [GSOC PATCH " Junio C Hamano
2025-07-15 22:15       ` Ayush Chandekar
2025-07-15 23:30         ` Junio C Hamano
2025-07-16 11:04           ` Ayush Chandekar
2025-07-16 15:21             ` Junio C Hamano
2025-07-16 15:24               ` Phillip Wood
2025-07-16 15:29                 ` Junio C Hamano
2025-07-15 18:56   ` [GSOC PATCH v4 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages Ayush Chandekar
2025-07-16 11:43 ` [GSOC PATCH v5 " Ayush Chandekar
2025-07-16 11:43   ` [GSOC PATCH v5 1/2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto" Ayush Chandekar
2025-07-16 11:43   ` [GSOC PATCH v5 2/2] config: set comment_line_str to "#" when core.commentChar=auto Ayush Chandekar
2025-07-16 15:28     ` Junio C Hamano
2025-07-16 14:28   ` [GSOC PATCH v5 0/2] commit: improve behaviour of core.commentChar=auto for comments in commit messages Phillip Wood
2025-07-16 15:29     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d25bf6c5-e56c-48d4-95b6-c714ee14ab78@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=ayu.chandekar@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kristofferhaugsbakk@fastmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=shyamthakkar001@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).