threads / patch / 25795

patchgit-send-email.perl: Deduplicate "to:" and "cc:" entries with names

Subject: [PATCH] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

## tl;dr

13 messages between Nov 20, 2010 and Feb 6, 2011. Diffs are folded; open one to read it.

replies: 12people: 4as markdown or json

Joe Perches· Nov 20, 2010, 17:06 UTC · lore

If an email address in the "to:" list is in the style "First Last <email@domain.tld>", ie: not just a bare address like "email@domain.tld", and the same named entry style exists in the "cc:" list, the current logic will not remove the entry from the "cc:" list.

Add logic to better deduplicate the "cc:" list by also matching the email address with angle brackets.

Signed-off-by: Joe Perches <joe@perches.com>
---
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to git-send-email.perl +1 −1
diff --git a/git-send-email.perl b/git-send-email.perl
index f68ed5a..1ae5fbf 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -960,7 +960,7 @@ sub maildomain {
 sub send_message {
 	my @recipients = unique_email_list(@to);
 	@cc = (grep { my $cc = extract_valid_address($_);
-		      not grep { $cc eq $_ } @recipients
+		      not grep { $cc eq $_ || $_ =~ /<${cc}>$/ } @recipients
 		    }
 	       map { sanitize_address($_) }
 	       @cc);
Andreas Schwab· Nov 20, 2010, 20:15 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

Joe Perches <joe@perches.com> writes:
Show 10 quoted lines
> diff --git a/git-send-email.perl b/git-send-email.perl
> index f68ed5a..1ae5fbf 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -960,7 +960,7 @@ sub maildomain {
>  sub send_message {
>  	my @recipients = unique_email_list(@to);
>  	@cc = (grep { my $cc = extract_valid_address($_);
> -		      not grep { $cc eq $_ } @recipients
> +		      not grep { $cc eq $_ || $_ =~ /<${cc}>$/ } @recipients
                                                    /<\Q${cc}\E>$/
Andreas.
-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Joe Perches· Nov 20, 2010, 21:01 UTC · re: Andreas Schwab · lore

Re: [PATCH] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

On Sat, 2010-11-20 at 21:15 +0100, Andreas Schwab wrote:
Show 12 quoted lines
> Joe Perches <joe@perches.com> writes:
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > index f68ed5a..1ae5fbf 100755
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -960,7 +960,7 @@ sub maildomain {
> >  sub send_message {
> >  	my @recipients = unique_email_list(@to);
> >  	@cc = (grep { my $cc = extract_valid_address($_);
> > -		      not grep { $cc eq $_ } @recipients
> > +		      not grep { $cc eq $_ || $_ =~ /<${cc}>$/ } @recipients
>                                                     /<\Q${cc}\E>$/

Why are \Q and \E useful here? extract_valid_address provides an unadorned email address. I've now tested with and without, both seem to work properly.

Pete Harlan· Nov 20, 2010, 22:47 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

On 11/20/2010 01:01 PM, Joe Perches wrote:
Show 17 quoted lines
> On Sat, 2010-11-20 at 21:15 +0100, Andreas Schwab wrote:
>> Joe Perches <joe@perches.com> writes:
>> > diff --git a/git-send-email.perl b/git-send-email.perl
>> > index f68ed5a..1ae5fbf 100755
>> > --- a/git-send-email.perl
>> > +++ b/git-send-email.perl
>> > @@ -960,7 +960,7 @@ sub maildomain {
>> >  sub send_message {
>> >  	my @recipients = unique_email_list(@to);
>> >  	@cc = (grep { my $cc = extract_valid_address($_);
>> > -		      not grep { $cc eq $_ } @recipients
>> > +		      not grep { $cc eq $_ || $_ =~ /<${cc}>$/ } @recipients
>>                                                     /<\Q${cc}\E>$/
> 
> Why are \Q and \E useful here?
> extract_valid_address provides an unadorned email address.
> I've now tested with and without, both seem to work properly.
Because email addresses can contain characters (e.g., '+') that you don't want the regular expression to treat specially.
Joe Perches· Nov 20, 2010, 23:06 UTC · re: Pete Harlan · lore

[PATCH V2] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

If an email address in the "to:" list is in the style "First Last <email@domain.tld>", ie: not just a bare address like "email@domain.tld", and the same named entry style exists in the "cc:" list, the current logic will not remove the entry from the "cc:" list.

Add logic to better deduplicate the "cc:" list by also matching the email address with angle brackets.

Signed-off-by: Joe Perches <joe@perches.com>
---
V2: Added quote escaping suggested by Andreas Schwab
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to git-send-email.perl +1 −1
diff --git a/git-send-email.perl b/git-send-email.perl
index f68ed5a..1ae5fbf 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -960,7 +960,7 @@ sub maildomain {
 sub send_message {
 	my @recipients = unique_email_list(@to);
 	@cc = (grep { my $cc = extract_valid_address($_);
-		      not grep { $cc eq $_ } @recipients
+		      not grep { $cc eq $_ || $_ =~ /<\Q${cc}\E>$/ } @recipients
 		    }
 	       map { sanitize_address($_) }
 	       @cc);
Junio C Hamano· Nov 26, 2010, 18:34 UTC · re: Joe Perches · lore

Re: [PATCH V2] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

Joe Perches <joe@perches.com> writes:
Show 8 quoted lines
> If an email address in the "to:" list is in the style
> "First Last <email@domain.tld>", ie: not just a bare
> address like "email@domain.tld", and the same named
> entry style exists in the "cc:" list, the current
> logic will not remove the entry from the "cc:" list.
>
> Add logic to better deduplicate the "cc:" list by also
> matching the email address with angle brackets.

Thanks; this is an improvement from the current behaviour. We do cull duplicates if you have the same address listed on @cc twice, but we don't do the similar culling between To:/Cc: headers.

I wonder if we should remove addresses from To: and Cc: if the same address appears in @bcclist, by the way, but that is a separate topic.

Thanks.
Joe Perches· Nov 26, 2010, 21:34 UTC · re: Junio C Hamano · lore

Re: [PATCH V2] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

On Fri, 2010-11-26 at 10:34 -0800, Junio C Hamano wrote:
> I wonder if we should remove addresses from To: and Cc: if the same
> address appears in @bcclist, by the way, but that is a separate topic.
Most likely it should be culled from the bcclist.
Junio C Hamano· Nov 27, 2010, 00:25 UTC · re: Joe Perches · lore

Re: [PATCH V2] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

Joe Perches <joe@perches.com> writes:
Show 5 quoted lines
> On Fri, 2010-11-26 at 10:34 -0800, Junio C Hamano wrote:
>> I wonder if we should remove addresses from To: and Cc: if the same
>> address appears in @bcclist, by the way, but that is a separate topic.
>
> Most likely it should be culled from the bcclist.

That means that the current behaviour is Ok, no? We do make the "RCPT TO" list of addresses we feed to smtp unique after concatenating To/Cc/Bcc, if I am not mistaken.

Joe Perches· Nov 27, 2010, 00:39 UTC · re: Junio C Hamano · lore

Re: [PATCH V2] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

On Fri, 2010-11-26 at 16:25 -0800, Junio C Hamano wrote:
Show 8 quoted lines
> Joe Perches <joe@perches.com> writes:
> > On Fri, 2010-11-26 at 10:34 -0800, Junio C Hamano wrote:
> >> I wonder if we should remove addresses from To: and Cc: if the same
> >> address appears in @bcclist, by the way, but that is a separate topic.
> > Most likely it should be culled from the bcclist.
> That means that the current behaviour is Ok, no?  We do make the "RCPT TO"
> list of addresses we feed to smtp unique after concatenating To/Cc/Bcc, if
> I am not mistaken.
I believe it's correct as is.
Joe Perches· Feb 5, 2011, 23:40 UTC · re: Junio C Hamano · lore

[PATCH] git-send-email.perl: Add --suppress-to

Add an equivalent command line option to suppress-cc.
Signed-off-by: Joe Perches <joe@perches.com>
---
 Documentation/git-send-email.txt |   17 +++++++++++++++++
 git-send-email.perl              |   33 +++++++++++++++++++++++++++++++--
 2 files changed, 48 insertions(+), 2 deletions(-)
Show changes to 2 files +48 −2

Documentation/git-send-email.txt, git-send-email.perl

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 7ec9dab..69e03e8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -232,6 +232,23 @@ Automating
 	cc list. Default is the value of 'sendemail.signedoffbycc' configuration
 	value; if that is unspecified, default to --signed-off-by-cc.
 
+--suppress-to=<category>::
+	Specify an additional category of recipients to suppress the
+	auto-to of:
++
+--
+- 'author' will avoid including the patch author
+- 'self' will avoid including the sender
+- 'tocmd' will avoid running the --to-cmd
+- 'bodyto' will avoid including anyone mentioned in To lines in the
+   patch body (commit message) except for self (use 'self' for that)
+- 'all' will suppress all auto to values.
+--
++
+Default is the value of 'sendemail.suppressto' configuration value; if
+that is unspecified, default to 'self' if --suppress-from is
+specified.
+
 --suppress-cc=<category>::
 	Specify an additional category of recipients to suppress the
 	auto-cc of:
diff --git a/git-send-email.perl b/git-send-email.perl
index 76565de..0365c29 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -74,6 +74,7 @@ git send-email [options] <file | directory | rev-list options >
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
+    --suppress-to           <str>  * author, self, tocmd, bodyto, all.
     --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
     --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, all.
     --[no-]signed-off-by-cc        * Send to Signed-off-by: addresses. Default on.
@@ -196,6 +197,7 @@ my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
 my ($validate, $confirm);
+my (@suppress_to);
 my (@suppress_cc);
 my ($auto_8bit_encoding);
 
@@ -226,6 +228,7 @@ my %config_settings = (
     "aliasfiletype" => \$aliasfiletype,
     "bcc" => \@bcclist,
     "aliasesfile" => \@alias_files,
+    "suppressto" => \@suppress_to,
     "suppresscc" => \@suppress_cc,
     "envelopesender" => \$envelope_sender,
     "multiedit" => \$multiedit,
@@ -301,6 +304,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
 		    "suppress-from!" => \$suppress_from,
+		    "suppress-to=s" => \@suppress_to,
 		    "suppress-cc=s" => \@suppress_cc,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
 		    "confirm=s" => \$confirm,
@@ -369,6 +373,24 @@ foreach my $setting (values %config_bool_settings) {
 # 'default' encryption is none -- this only prevents a warning
 $smtp_encryption = '' unless (defined $smtp_encryption);
 
+# Set TO suppressions
+my(%suppress_to);
+if (@suppress_to) {
+	foreach my $entry (@suppress_to) {
+		die "Unknown --suppress-to field: '$entry'\n"
+			unless $entry =~ /^(?:all|author|self|tocmd|bodyto)$/;
+		$suppress_to{$entry} = 1;
+	}
+}
+
+if ($suppress_to{'all'}) {
+	foreach my $entry (qw (author self tocmd bodyto)) {
+		$suppress_to{$entry} = 1;
+	}
+	delete $suppress_to{'all'};
+}
+$suppress_to{'self'} = $suppress_from if defined $suppress_from;
+
 # Set CC suppressions
 my(%suppress_cc);
 if (@suppress_cc) {
@@ -407,7 +429,11 @@ die "Unknown --confirm setting: '$confirm'\n"
 
 # Debugging, print out the suppressions.
 if (0) {
-	print "suppressions:\n";
+	print "To suppressions:\n";
+	foreach my $entry (keys %suppress_to) {
+		printf "  %-5s -> $suppress_to{$entry}\n", $entry;
+	}
+	print "Cc suppressions:\n";
 	foreach my $entry (keys %suppress_cc) {
 		printf "  %-5s -> $suppress_cc{$entry}\n", $entry;
 	}
@@ -1201,6 +1227,9 @@ foreach my $t (@files) {
 			}
 			elsif (/^To:\s+(.*)$/) {
 				foreach my $addr (parse_address_line($1)) {
+
+				    next if $suppress_to{'author'};
+				    next if $suppress_to{'self'} and $author eq $sender;
 					printf("(mbox) Adding to: %s from line '%s'\n",
 						$addr, $_) unless $quiet;
 					push @to, sanitize_address($addr);
@@ -1269,7 +1298,7 @@ foreach my $t (@files) {
 	close $fh;
 
 	push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t)
-		if defined $to_cmd;
+		if defined $to_cmd && !$suppress_to{'tocmd'};
 	push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t)
 		if defined $cc_cmd && !$suppress_cc{'cccmd'};
 
Junio C Hamano· Feb 6, 2011, 20:28 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email.perl: Add --suppress-to

Joe Perches <joe@perches.com> writes:
Show 6 quoted lines
> +- 'author' will avoid including the patch author
> +- 'self' will avoid including the sender
> +- 'tocmd' will avoid running the --to-cmd
> +- 'bodyto' will avoid including anyone mentioned in To lines in the
> +   patch body (commit message) except for self (use 'self' for that)
> +- 'all' will suppress all auto to values.

Is there a definition of what an "auto to value" is somewhere? "auto cc values" was sort of understandable as there is no word "cc" (other than 1/1000 litre ;-) and what it meant was guessable from context, but it took me a few seconds to realize you meant "To:" with this.

Show 9 quoted lines
> +--
> ++
> +Default is the value of 'sendemail.suppressto' configuration value; if
> +that is unspecified, default to 'self' if --suppress-from is
> +specified.
> +
>  --suppress-cc=<category>::
>  	Specify an additional category of recipients to suppress the
>  	auto-cc of:
Hmmm, from a cursory look I don't see how bodyto is handled and where.
Show 6 quoted lines
> @@ -1201,6 +1227,9 @@ foreach my $t (@files) {
>  			}
>  			elsif (/^To:\s+(.*)$/) {
>  				foreach my $addr (parse_address_line($1)) {
> +
> +				    next if $suppress_to{'author'};
Is "To: somebody" in the output guaranteed to name the author and nobody else?
> +				    next if $suppress_to{'self'} and $author eq $sender;
>  					printf("(mbox) Adding to: %s from line '%s'\n",
>  						$addr, $_) unless $quiet;
>  					push @to, sanitize_address($addr);
Show 8 quoted lines
> @@ -1269,7 +1298,7 @@ foreach my $t (@files) {
>  	close $fh;
>  
>  	push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t)
> -		if defined $to_cmd;
> +		if defined $to_cmd && !$suppress_to{'tocmd'};
>  	push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t)
>  		if defined $cc_cmd && !$suppress_cc{'cccmd'};

I think it is about time to make this a bit more readable by explicitly using if statement, not statement modifiers.

Andreas Schwab· Nov 20, 2010, 22:57 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

Joe Perches <joe@perches.com> writes:
> extract_valid_address provides an unadorned email address.
An email address is not a regexp.  Thus it may not match itself.
> I've now tested with and without, both seem to work properly.
Did you test with all possible email addresses?
Andreas.
-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Joe Perches· Nov 20, 2010, 23:00 UTC · re: Andreas Schwab · lore

Re: [PATCH] git-send-email.perl: Deduplicate "to:" and "cc:" entries with names

On Sat, 2010-11-20 at 23:57 +0100, Andreas Schwab wrote:
Show 5 quoted lines
> Joe Perches <joe@perches.com> writes:
> > extract_valid_address provides an unadorned email address.
> An email address is not a regexp.  Thus it may not match itself.
> > I've now tested with and without, both seem to work properly.
> Did you test with all possible email addresses?
:)  That'd take forever...

← back to recent threads