Re: [PATCH] trailers: stop recognizing URLs as trailers

["Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>]

On Mon, Aug 3, 2026, at 17:20, Jeff King wrote:
> On Sun, Aug 02, 2026 at 09:57:17PM +0200,
> kristofferhaugsbakk@fastmail.com wrote:
>
>> There are commits which contain intended non-trailer lines which start
>> with URLs. These are comments. Example with just the trailers:[2]
>>
>>     Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>>     [bhelgaas: squash fixes:
>>     https://lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com
>>     https://lore.kernel.org/r/20260108013956.14351-3-bagasdotme@gmail.com]
>>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>     Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>     Link: https://patch.msgid.link/20251210132907.58799-4-xueshuai@linux.alibaba.com
>>
>> Those `[]` pairs delimit the “squash fixes” comment.
>
> This example makes me wonder if we ought to be smarter about brackets.
> I.e., could/should we realize that the opening bracket is a comment and
> then ignore everything up to the closing one? That would help this case
> and other weird cases like: [snip]

I think that it makes a lot of sense to special-case brackets as
delimiting non-trailer runs.

(for other readers) This support for non-trailer lines grew out of Linux
Kernel practices. At least according to this thread:

https://lore.kernel.org/git/CA+55aFzN4SnenchxPScn61_apzitGAPtoYEd49iLZPxgK0KQGw@mail.gmail.com/

(See also in particular: https://lore.kernel.org/git/20150905000745.GC11443@sigill.intra.peff.net/ )

And part of the back-and-forth in that thread is an inherent tension:
the trailer format is loose. All you need is a some
alphanumerics/hyphens and a colon. So it is simple to accidentally slip
in a *real* trailer line along with all the cruft. Like Peff’s example
shows:

>
>   Signed-off-by: whomever
>   [peff: there's a really interesting thing going on
>   here: the comment is free-form text that happens to
>   use a colon in a sentence, but we'll interpret it
>   as a trailer with key "here"]
>   Signed-off-by: another unlucky soul

Imagine you had internalized the trailer parsing rules (which you
shouldn’t have to but anyway); it would still be easy to accidentally
write something like the above.

But with an additional `[]` rule you don’t have to worry:

• A run of non-trailer lines starts with regex `^[`
• And ends 0 or more lines later with regex `]$`
• In addition to the existing rules

And `[]` are illegal in trailer keys anyway.

This is a very Linux (and Git project) specific additional rule, but the
non-trailer lines rules were always like that.

>
> That said, I think there are cases without brackets that are also
> confusing. Like:
>
>   Let me finish this commit message by telling you all about this
>   amazing url:
>
>   https://example.com

Yeah exactly.

>[snip]
>> diff --git a/trailer.c b/trailer.c
>> index 6d8ec7fa8d8..971ae459596 100644
>> --- a/trailer.c
>> +++ b/trailer.c
>> @@ -635,8 +635,13 @@ static ssize_t find_separator(const char *line, const char *separators)
>>  	int whitespace_found = 0;
>>  	const char *c;
>>  	for (c = line; *c; c++) {
>> -		if (strchr(separators, *c))
>> +		if (strchr(separators, *c)) {
>> +			/* avoid accidental URL matches (://) */
>> +			if (*c == ':' && c[1] == '/' && c[2] == '/' &&
>> +			    !whitespace_found)
>> +				return -1;
>>  			return c - line;
>> +		}
>
> As discussed elsewhere, we are free to match with short-circuiting
> because of the NUL termination. But that also means we could write this
> as:
>
>   if (starts_with(c, "://") && !whitespace_found)
>
> which is perhaps a little more readable.

Oh for sure, much more readable.

Thanks!