Re: [PATCH V2] git-send-email: Add ability to cc: any "bylines" in the commit message

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 31, 2016, 19:34 UTC
Message-ID: <xmqq4m6020nh.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <aa4c9e0b37e70423baa69bc27cc9adda0f152efe.1472669418.git.joe@perches.com>

Joe Perches <joe@perches.com> writes:

> Many commits have various forms of bylines similar to

A missing blank line (I can tweak while queuing).

Show 12 quoted lines

>      "Acked-by: Name <address>" and "Reported-by: Name <address>"
>
> Add the ability to cc: bylines (e.g. Acked-by:) when using git send-email.
>
> This can be suppressed with --suppress-cc=bylines.
> ...
> @@ -307,8 +308,10 @@ Automating
>    patch body (commit message) except for self (use 'self' for that).
>  - 'sob' will avoid including anyone mentioned in Signed-off-by lines except
>     for self (use 'self' for that).
> +- 'bylines' will avoid including anyone mentioned in any "<foo>-by:" lines
> +  in the patch header except for Signed-off-by.

<foo> feels a bit too informal but I don't think of a better alternative, perhaps other than "*-by:".

Show 6 quoted lines

> @@ -1545,7 +1545,7 @@ foreach my $t (@files) {
>  	# Now parse the message body
>  	while(<$fh>) {
>  		$message .=  $_;
> -		if (/^(Signed-off-by|Cc): (.*)$/i) {
> +		if (/^(Signed-off-by|Cc|[^\s]+[\w-]by): (.*)$/i) {

I thought you wanted

		if (/^(Signed-off-by|Cc|[\w-]+-by): (.*)$/i) {

instead to avoid "O_=:;fooby: Joe Perches <joe@...>"

Show 8 quoted lines

>  			chomp;
>  			my ($what, $c) = ($1, $2);
>  			chomp $c;
> @@ -1555,6 +1555,12 @@ foreach my $t (@files) {
>  			} else {
>  				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
>  				next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
> +				next if $suppress_cc{'bylines'} and $what !~ /Signed-off-by/i and $what =~ /by$/i;

Having to keep this /by$/i in sync with whatever definition of bylines is will be error prone. How about doing it in this way?

	# Now parse the message body
+	my $bypat = r/[\w-]+-by/;
	while (<$fh>) {
        	...
                if (/^(Signed-off-by|Cc|$bypat): (.*)$/i) {
                	...
                        	next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
+				next if $suppress_cc{'bylines'} and
+					$what !~ /^Signed-off-by/i and
+					$what =~ /^$bypat/i;

Other than that, looking good.