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

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

[Joe Perches]

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(-)

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);

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

[Andreas Schwab]

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>$/

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."

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

[Joe Perches]

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.

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

[Pete Harlan]

On 11/20/2010 01:01 PM, Joe Perches wrote:
> 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.

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

[Andreas Schwab]

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."

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

[Joe Perches]

On Sat, 2010-11-20 at 23:57 +0100, Andreas Schwab wrote:
> 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...

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

[Joe Perches]

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(-)

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);

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

[Junio C Hamano]

Joe Perches <joe@perches.com> writes:

> 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.

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

[Joe Perches]

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.

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

[Junio C Hamano]

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.

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

[Joe Perches]

On Fri, 2010-11-26 at 16:25 -0800, Junio C Hamano wrote:
> 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.

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

[Joe Perches]

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(-)

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'};
 

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

[Junio C Hamano]

Joe Perches <joe@perches.com> writes:

> +- '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.

> +--
> ++
> +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.

> @@ -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);

> @@ -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.