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

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:

quoted

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 [off-list ref]
    [bhelgaas: squash fixes:
    https://lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com (local)
    https://lore.kernel.org/r/20260108013956.14351-3-bagasdotme@gmail.com (local)]
    Signed-off-by: Bjorn Helgaas [off-list ref]
    Reviewed-by: Ilpo Järvinen [off-list ref]
    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/ (local)

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

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]

quoted

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!