Re: [GSOC PATCH v2] commit: avoid scanning trailing comments when 'core.commentChar' is "auto"
- From
Ayush Chandekar <ayu.chandekar@gmail.com>
- Date
- Jun 28, 2025, 14:33 UTC
- Message-ID
- <CAE7as+aUcd65vPwwRh_C89vQbMjKQh0Y6LF7WDq1Whyj6iYfLg@mail.gmail.com>
- In-Reply-To
- <f39a3285-574a-45c6-9646-04eb175f4770@gmail.com>
On Sat, Jun 28, 2025 at 7:08 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 52 quoted lines
> > On 27/06/2025 15:52, Junio C Hamano wrote: > > Phillip Wood <phillip.wood123@gmail.com> writes: > > > >>> + size_t cutoff; > >>> + > >>> + /* Ignore comment chars in trailing comments (e.g., Conflicts:) */ > >>> + cutoff = sb->len - ignored_log_message_bytes(sb->buf, sb->len); > >> > >> This finds the "Conflicts:" line. I was surprised to see that the > >> string it looks for is hard coded and not translated, however the > >> sequencer (also surprisingly) does not translate that message either > >> so it should work. > > > > There is a funny chicken-and-egg problem, though. It limits the > > search for "Conflicts" by using wt_status_locate_end() based on the > > current value of comment_line_str. When core.commentstring is set > > to "auto", the code that reads the configuration does not touch the > > comment_line_str variable, which is initialized to '#'. So > > > > [core] > > commentstring = '%' > > commentstring = auto > > > > would have '%' in comment_line_str upon entering this codepath, let > > wt_status_locate_end() use '%' as the comment string to find the end > > of the log message, and then looks for "Conflicts:" in the result. > > > > Which may or may not be what you want. > > Oh, good point - I'd not looked at the config parsing. So we'd create > conflict comments that look like > > % Conflicts: > % some-file.c > > but so long as the commit message did not contain a '#' character [1] > "git commit" would select '#' as the automatic comment char and our > conflicts lines would not be treated as a comment. > > Should we be resetting comment_line_str to '#' when core.commentString > is set to "auto"? That wont help if the commit message contains a '#' > but at least it would be consistently broken. > > We could move adjust_comment_line_char() into libgit.a, use that to > select the comment character used in append_conflicts_hint() and set > core.commentString to that character when we run "git commit". I think > doing that would mean that appending conflict comments would always work > with core.commentString=auto but it is a more complex solution as we > would need to remember the comment character and then pass it to git > commit once the user had fixed the conflicts. >
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.
Show 14 quoted lines
> One final note - although the commit message mentions a change to "git > rebase" I think this problem already affected cherry-pick, revert and > merge before that change. In practice I suspect it is only cherry-pick > where one is likely to see this problem because the template messages > for the other two commands are unlikely to contain a '#'. > > Thanks > > Phillip > > [1] For some reason adjust_comment_line_char() will not select '#' as > the comment char if it occurs anywhere in the message but the other > candidates are selected so long as they are not the first character on > any line.
Thanks:)
[2] : https://github.com/ayu-ch/git/commits/comment-line-str-4 [3] : https://ayu-ch.github.io/2025/06/09/gsoc-week-1.html