Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

8 messages, 6 authors, 2021-12-08 · open the first message on its own page

Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

From: Junio C Hamano <hidden>
Date: 2021-11-27 20:22:27

Eric Wong [off-list ref] writes:
Thorsten Leemhuis [off-list ref] wrote:
quoted
Just to be sure I'll do what you expect to be done: I assume you want to see
it changed like this?

-	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
...
The entire match should be case-insensitive[1], so I'd add `i'
at the end:

	perl -pi -e 's|^Message-ID:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|gi;' "$1"

Fwiw, every mail and HTTP/1.x header parser I've looked at works
case-insensitively.  Also, I'm not sure if `g' is needed, actually...
It is left anchored with "^" so it would be hard to match more than
once on the same line ;-)

I agree that it is the right solution to make the whole thing
case-insensitive by adding 'i' at the end.

FWIW, the RFC first says this:

    1.2.2. Syntactic notation

       This standard uses the Augmented Backus-Naur Form (ABNF) notation
       specified in [RFC2234] for the formal definitions of the syntax of
       messages.  Characters will be specified either by a decimal value
       (e.g., the value %d65 for uppercase A and %d97 for lowercase A) or by
       a case-insensitive literal value enclosed in quotation marks (e.g.,
       "A" for either uppercase or lowercase A).

and then goes on to define how message-id should look like.

    3.6.4. Identification fields

    message-id      =       "Message-ID:" msg-id CRLF


But if you go the "add /i at the end" route, you do not have to
upcase "d" to "D" and that may reduce the patch noise (it only
matters if the patch viewer highlights letter-by-letter changes for
your recipients).

HTH

Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

From: Jani Nikula <hidden>
Date: 2021-11-29 12:05:30

On Sat, 27 Nov 2021, Junio C Hamano [off-list ref] wrote:
Eric Wong [off-list ref] writes:
quoted
Thorsten Leemhuis [off-list ref] wrote:
quoted
Just to be sure I'll do what you expect to be done: I assume you want to see
it changed like this?

-	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
...
The entire match should be case-insensitive[1], so I'd add `i'
at the end:

	perl -pi -e 's|^Message-ID:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|gi;' "$1"

Fwiw, every mail and HTTP/1.x header parser I've looked at works
case-insensitively.  Also, I'm not sure if `g' is needed, actually...
It is left anchored with "^" so it would be hard to match more than
once on the same line ;-)

I agree that it is the right solution to make the whole thing
case-insensitive by adding 'i' at the end.

FWIW, the RFC first says this:

    1.2.2. Syntactic notation

       This standard uses the Augmented Backus-Naur Form (ABNF) notation
       specified in [RFC2234] for the formal definitions of the syntax of
       messages.  Characters will be specified either by a decimal value
       (e.g., the value %d65 for uppercase A and %d97 for lowercase A) or by
       a case-insensitive literal value enclosed in quotation marks (e.g.,
       "A" for either uppercase or lowercase A).

and then goes on to define how message-id should look like.

    3.6.4. Identification fields

    message-id      =       "Message-ID:" msg-id CRLF


But if you go the "add /i at the end" route, you do not have to
upcase "d" to "D" and that may reduce the patch noise (it only
matters if the patch viewer highlights letter-by-letter changes for
your recipients).
From the RFC nitpicking department, msg-id is allowed to contain CFWS
(comments and folding white space) outside the angle brackets, which
means you could have RFC compliant Message-ID header field:

Message-ID: 
  [off-list ref]

or

Message-ID: (comment) 
  [off-list ref]

or even worse, really.

The moral of the story is that you should always offload the header
parsing to some tool or library designed to do that.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

From: Eric Wong <hidden>
Date: 2021-11-29 17:28:25

Jani Nikula [off-list ref] wrote:
From the RFC nitpicking department, msg-id is allowed to contain CFWS
(comments and folding white space) outside the angle brackets, which
means you could have RFC compliant Message-ID header field:

Message-ID: 
  [off-list ref]

or

Message-ID: (comment) 
  [off-list ref]

or even worse, really.

The moral of the story is that you should always offload the header
parsing to some tool or library designed to do that.
It's a bit much for common cases with git-send-email and
reasonable MUAs, I think.  I don't know if formail is commonly
installed, nowadays...

Fwiw, the code running lore uses something like this:

	/^Message-ID:[ \t]*([^\n]*\r?\n # 1st line
			# continuation lines:
			(?:[^:\n]*?[ \t]+[^\n]*\r?\n)*)
			/ismx

I'm fine with this non-trivial regexp being included with
GPL-2.0 code; but it could be too big for a one-liner *shrug*

... And <([^>]+)>/s to extract Message-IDs, but ISTR the code
behind lore doesn't handle spaces inside <> properly, but I'm
not sure if there's enough valid, non-spam messages with them...

Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

From: Jani Nikula <hidden>
Date: 2021-11-29 19:27:07

On Mon, 29 Nov 2021, Eric Wong [off-list ref] wrote:
Jani Nikula [off-list ref] wrote:
quoted
The moral of the story is that you should always offload the header
parsing to some tool or library designed to do that.
It's a bit much for common cases with git-send-email and
reasonable MUAs, I think.
I think you can have unreasonable MDAs in between, though!

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

From: Steven Rostedt <rostedt@goodmis.org>
Date: 2021-11-29 20:34:45

On Mon, 29 Nov 2021 14:03:09 +0200
Jani Nikula [off-list ref] wrote:
quoted
From the RFC nitpicking department, msg-id is allowed to contain CFWS  
(comments and folding white space) outside the angle brackets, which
means you could have RFC compliant Message-ID header field:

Message-ID: 
  [off-list ref]
My scripts have already been hit by this. (I've been lazy and not fixed it,
but instead, just edit the file that it is parsing manually, to be on one
line :-p)

-- Steve

Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-11-30 08:24:29

Hi Eric,

On Mon, Nov 29, 2021 at 11:29 PM Eric Wong [off-list ref] wrote:
It's a bit much for common cases with git-send-email and
reasonable MUAs, I think.  I don't know if formail is commonly
installed, nowadays...
Of course ;-) You need it to run checkpatch on patch series obtained
through "b4 am", before you apply them to your tree:

$ cat *mbx | formail -s scripts/checkpatch.pl

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

From: Thorsten Leemhuis <linux@leemhuis.info>
Date: 2021-12-08 13:41:53

Hi Eric!

On 30.11.21 09:24, Geert Uytterhoeven wrote:
On Mon, Nov 29, 2021 at 11:29 PM Eric Wong [off-list ref] wrote:
quoted
It's a bit much for common cases with git-send-email and
reasonable MUAs, I think.  I don't know if formail is commonly
installed, nowadays...
Well, after your earlier suggestion I considered to go with this:

-	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link:
https://lore.kernel.org/r/$1|g;' "$1"
+	perl -pi -e 's|^Message-ID:\s*<?([^>]+)>?$|Link:
https://lore.kernel.org/r/$1|i;' "$1"

But...
Of course ;-) You need it to run checkpatch on patch series obtained
through "b4 am", before you apply them to your tree:

$ cat *mbx | formail -s scripts/checkpatch.pl
...this made me wonder if formail would be the better solution. I came
up with this:

formail -A "Link: https://lore.kernel.org/r/`formail -c -x Message-ID <
"${1}" | sed 's!.*<\(.*\)>!\1!'`" < "${1}" | sponge "${1}"

Downsides: instead of perl it requires sed and sponge (part of
moreutils, which I guess not everyone has installed; but I tried to
avoid a big here document or moving files around).

Is that worth it? Or is there a way to realize this in a more elegant
fashion with tools everyone has installed?

Ciao, Thorsten

Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

From: Eric Wong <hidden>
Date: 2021-12-08 17:02:31

Thorsten Leemhuis [off-list ref] wrote:
On 30.11.21 09:24, Geert Uytterhoeven wrote:
quoted
On Mon, Nov 29, 2021 at 11:29 PM Eric Wong [off-list ref] wrote:
quoted
It's a bit much for common cases with git-send-email and
reasonable MUAs, I think.  I don't know if formail is commonly
installed, nowadays...
Well, after your earlier suggestion I considered to go with this:

-	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link:
https://lore.kernel.org/r/$1|g;' "$1"
+	perl -pi -e 's|^Message-ID:\s*<?([^>]+)>?$|Link:
https://lore.kernel.org/r/$1|i;' "$1"

But...
quoted
Of course ;-) You need it to run checkpatch on patch series obtained
through "b4 am", before you apply them to your tree:

$ cat *mbx | formail -s scripts/checkpatch.pl
...this made me wonder if formail would be the better solution. I came
up with this:

formail -A "Link: https://lore.kernel.org/r/`formail -c -x Message-ID <
"${1}" | sed 's!.*<\(.*\)>!\1!'`" < "${1}" | sponge "${1}"

Downsides: instead of perl it requires sed and sponge (part of
moreutils, which I guess not everyone has installed; but I tried to
avoid a big here document or moving files around).
As Geert noted, formail is probably reasonable, but I certainly
don't have moreutils across all the systems I'm using right now.
Is that worth it? Or is there a way to realize this in a more elegant
fashion with tools everyone has installed?
*shrug*  Since newlines after ':' are a concern and it's (probably :P)
safe to slurp entire contents of emails into memory nowadays;
some minor tweaks to the original perl invocation should work:

* use `$/ = undef' to force Perl to operate on the entire input at once
* use `m' RE modifier to ensure `^' and `$' still match SOL/EOL
  ($/ is only the input record separator, it doesn't change
   Perl's definition of "lines" for `^' and `$')

perl -i -p -e 'BEGIN{$/=undef};s|^Message-ID:\s*<?([^>]+)>?$|Link:
 https://lore.kernel.org/r/$1|im;'
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help