[PATCH RFC3.5 00/12] Introduction to Decreasing send-email Entropy

[PATCH RFC3.5 00/12] Introduction to Decreasing send-email Entropy

[Michael Witten]

This patch series introduces some very basic refactorizations and
improvements to send-email. My goal was to keep each patch relatively
small, but it may be better to look at the results of the following
patches:

	[PATCH RFC3.5 06/12] send-email: Cleanup and streamline the SMTP code in send_message
	[PATCH RFC3.5 07/12] send-email: Cleanup send_message 'log' code
	[PATCH RFC3.5 09/12] Docs: send-email: Reorganize the CONFIGURATION section

The overall diffstat:

	Documentation/git-send-email.txt |  201 ++++++++++++++++++++----
	git-send-email.perl              |  317 +++++++++++++++++++++++++-------------
	2 files changed, 376 insertions(+), 142 deletions(-)

The over all patch series:

	[PATCH RFC3.5 01/12] send-email: Cleanup the usage text and docs a bit
	[PATCH RFC3.5 02/12] send-email: No longer repeatedly test if $smtp_server is a command
	[PATCH RFC3.5 03/12] send-email: Interpret --smtp-server "" as "use a default".
	[PATCH RFC3.5 04/12] send-email: Verification for --smtp-server and --smpt-server-port
	[PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code
	[PATCH RFC3.5 06/12] send-email: Cleanup and streamline the SMTP code in send_message
	[PATCH RFC3.5 07/12] send-email: Cleanup send_message 'log' code
	[PATCH RFC3.5 08/12] send-email: Move Subject sanitization from --compose code to send_message
	[PATCH RFC3.5 09/12] Docs: send-email: Reorganize the CONFIGURATION section
	[PATCH RFC3.5 10/12] Docs: Embolden the CONFIGURATION references
	[PATCH RFC3.5 11/12] Docs: send-email: Clarification of sendemail.<identity>
	[PATCH RFC3.5 12/12] Docs: send-email: git send-email -> 'send-email'

Sincerely,
Michael Witten

[PATCH RFC3.5 01/12] send-email: Cleanup the usage text and docs a bit

[Michael Witten]

--chain-reply-to doesn't take an argument.

The here-document quotation that defines the usage
text is now a single-quote form, so that no interpolation
takes place.

All usage text lines should be < 80 characters.

The usage text's option arguments match those of the docs.

The 'host:port' form of argument for --smtp-server was only
working for SSL connections, because the SSL connection code
was relying on undocumented behavior of Net::SMTP::SSL (really,
undocumented behavior of Net::SMTP's new method). Because the
main documentation for send-email doesn't even list it as valid,
<str:int> has been replaced with just <server> and <host | command>
in the usage text; this is only temporary and for purity.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/git-send-email.txt |   21 ++++++++-------
 git-send-email.perl              |   49 +++++++++++++++++++++-----------------
 2 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index f7e428e..071e9bf 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -74,8 +74,9 @@ See the CONFIGURATION section for 'sendemail.multiedit'.
 	the value GIT_COMMITTER_IDENT, as returned by "git var -l".
 	The user will still be prompted to confirm this entry.
 
---in-reply-to=<identifier>::
-	Specify the contents of the first In-Reply-To header.
+--in-reply-to=<message-id>::
+	Specify the contents of the first In-Reply-To header;
+	include the angle brackets `<` and `>`.
 	Subsequent emails will refer to the previous email
 	instead of this if --chain-reply-to is set (the default)
 	Only necessary if --compose is also set.  If --compose
@@ -106,7 +107,7 @@ Sending
 	the 'sendemail.envelopesender' configuration variable; if that is
 	unspecified, choosing the envelope sender is left to your MTA.
 
---smtp-encryption=<encryption>::
+--smtp-encryption=<type>::
 	Specify the encryption to use, either 'ssl' or 'tls'.  Any other
 	value reverts to plain SMTP.  Default is the value of
 	'sendemail.smtpencryption'.
@@ -123,20 +124,20 @@ or on the command line. If a username has been specified (with
 specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
 user is prompted for a password while the input is masked for privacy.
 
---smtp-server=<host>::
+--smtp-server=<server>::
 	If set, specifies the outgoing SMTP server to use (e.g.
 	`smtp.example.com` or a raw IP address).  Alternatively it can
 	specify a full pathname of a sendmail-like program instead;
 	the program must support the `-i` option.  Default value can
 	be specified by the 'sendemail.smtpserver' configuration
-	option; the built-in default is `/usr/sbin/sendmail` or
-	`/usr/lib/sendmail` if such program is available, or
+	variable; the built-in default is `/usr/sbin/sendmail` or
+	`/usr/lib/sendmail` if such a program is available, or
 	`localhost` otherwise.
 
 --smtp-server-port=<port>::
 	Specifies a port different from the default port (SMTP
 	servers typically listen to smtp port 25 and ssmtp port
-	465); symbolic port names (e.g. "submission" instead of 465)
+	465); symbolic service names (e.g. "submission" instead of 587)
 	are also accepted. The port can also be set with the
 	'sendemail.smtpserverport' configuration variable.
 
@@ -158,7 +159,7 @@ Automating
 	Output of this command must be single email address per line.
 	Default is the value of 'sendemail.cccmd' configuration value.
 
---[no-]chain-reply-to=<identifier>::
+--[no-]chain-reply-to::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
 	the first will be sent as replies to the first email sent.  When using
@@ -170,7 +171,7 @@ Automating
 	A configuration identity. When given, causes values in the
 	'sendemail.<identity>' subsection to take precedence over
 	values in the 'sendemail' section. The default identity is
-	the value of 'sendemail.identity'.
+	the value of the 'sendemail.identity' configuration variable.
 
 --[no-]signed-off-by-cc::
 	If this is set, add emails found in Signed-off-by: or Cc: lines to the
@@ -214,7 +215,7 @@ specified, as well as 'body' if --no-signed-off-cc is specified.
 Administering
 ~~~~~~~~~~~~~
 
---confirm=<mode>::
+--confirm=<when>::
 	Confirm just before sending:
 +
 --
diff --git a/git-send-email.perl b/git-send-email.perl
index 04267c5..e43342e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -42,46 +42,51 @@ package main;
 
 
 sub usage {
-	print <<EOT;
+
+	# All printed lines should be less than 80 characters.
+
+	print <<'EOT';
 git send-email [options] <file | directory | rev-list options >
 
   Composing:
-    --from                  <str>  * Email From:
-    --to                    <str>  * Email To:
-    --cc                    <str>  * Email Cc:
-    --bcc                   <str>  * Email Bcc:
-    --subject               <str>  * Email "Subject:"
-    --in-reply-to           <str>  * Email "In-Reply-To:"
-    --annotate                     * Review each patch that will be sent in an editor.
+    --from              <address>  * Email From:
+    --to                <address>  * Email To:
+    --cc                <address>  * Email Cc:
+    --bcc               <address>  * Email Bcc:
+    --subject            <string>  * Email "Subject:"
+    --in-reply-to    <message-id>  * Email "In-Reply-To:"; include '<' and '>'.
+    --annotate                     * Review each patch that will be sent in
+                                     an editor.
     --compose                      * Open an editor for introduction.
 
   Sending:
-    --envelope-sender       <str>  * Email envelope sender.
-    --smtp-server       <str:int>  * Outgoing SMTP server to use. The port
-                                     is optional. Default 'localhost'.
-    --smtp-server-port      <int>  * Outgoing SMTP server port.
-    --smtp-user             <str>  * Username for SMTP-AUTH.
-    --smtp-pass             <str>  * Password for SMTP-AUTH; not necessary.
-    --smtp-encryption       <str>  * tls or ssl; anything else disables.
+    --envelope-sender   <address>  * Email envelope sender.
+    --smtp-server        <server>  * Outgoing SMTP server. <host | command>
+    --smtp-server-port     <port>  * Outgoing SMTP server port; symbolic too.
+    --smtp-user        <username>  * Username for SMTP-AUTH.
+    --smtp-pass       [<password>] * Password for SMTP-AUTH; not necessary.
+    --smtp-encryption      <type>  * tls or ssl; anything else disables.
     --smtp-ssl                     * Deprecated. Use '--smtp-encryption ssl'.
 
   Automating:
-    --identity              <str>  * Use the sendemail.<id> options.
-    --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.
+    --identity         <identity>  * Use the sendemail.<identity> options.
+    --cc-cmd            <command>  * Email Cc: via `<command> $patch_path`
+    --suppress-cc      <category>  * author, self, sob, cc, cccmd, body,
+                                     bodycc, all.
+    --[no-]signed-off-by-cc        * Send to Signed-off-by: addresses.
+                                     Default on.
     --[no-]suppress-from           * Send to self. Default off.
     --[no-]chain-reply-to          * Chain In-Reply-To: fields. Default on.
     --[no-]thread                  * Use In-Reply-To: field. Default on.
 
   Administering:
-    --confirm               <str>  * Confirm recipients before sending;
+    --confirm              <when>  * Confirm recipients before sending;
                                      auto, cc, compose, always, or never.
     --quiet                        * Output one line of info per email.
     --dry-run                      * Don't actually send the emails.
     --[no-]validate                * Perform patch sanity checks. Default on.
-    --[no-]format-patch            * understand any non optional arguments as
-                                     `git format-patch` ones.
+    --[no-]format-patch            * Understand any non-optional arguments as
+                                     `git format-patch' arguments.
 
 EOT
 	exit(1);
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 02/12] send-email: No longer repeatedly test if $smtp_server is a command

[Michael Witten]

This is a minor change, but it's cleaner, and it sets up the
$smtp_server initialization code for future improvements.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 git-send-email.perl |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index e43342e..1a20b2c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -186,7 +186,8 @@ sub do_edit {
 
 # Variables with corresponding config settings
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
-my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);
+my ($smtp_server, $smtp_server_is_a_command);
+my ($smtp_server_port, $smtp_authuser, $smtp_encryption);
 my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
 my ($validate, $confirm);
 my (@suppress_cc);
@@ -674,14 +675,22 @@ if (defined $initial_reply_to) {
 	$initial_reply_to = "<$initial_reply_to>" if $initial_reply_to ne '';
 }
 
-if (!defined $smtp_server) {
+if (defined $smtp_server) {
+
+	$smtp_server_is_a_command = ($smtp_server =~ m{^/});
+
+} else { # use a default:
+
 	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
 		if (-x $_) {
 			$smtp_server = $_;
+			$smtp_server_is_a_command = 1;
 			last;
 		}
 	}
-	$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
+
+	$smtp_server = 'localhost'; # 127.0.0.1 is not compatible with IPv6
+		unless $smtp_server_is_a_command;
 }
 
 if ($compose && $compose > 0) {
@@ -882,7 +891,7 @@ X-Mailer: git-send-email $gitversion
 
 	if ($dry_run) {
 		# We don't want to send the email.
-	} elsif ($smtp_server =~ m#^/#) {
+	} elsif ($smtp_server_is_a_command) {
 		my $pid = open my $sm, '|-';
 		defined $pid or die $!;
 		if (!$pid) {
@@ -958,7 +967,7 @@ X-Mailer: git-send-email $gitversion
 		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
 	} else {
 		print (($dry_run ? "Dry-" : "")."OK. Log says:\n");
-		if ($smtp_server !~ m#^/#) {
+		unless ($smtp_server_is_a_command) {
 			print "Server: $smtp_server\n";
 			print "MAIL FROM:<$raw_from>\n";
 			print "RCPT TO:".join(',',(map { "<$_>" } @recipients))."\n";
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 03/12] send-email: Interpret --smtp-server "" as "use a default".

[Michael Witten]

Why not? It's at least useful for testing.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/git-send-email.txt |    4 +++-
 git-send-email.perl              |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 071e9bf..ae01632 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -132,7 +132,9 @@ user is prompted for a password while the input is masked for privacy.
 	be specified by the 'sendemail.smtpserver' configuration
 	variable; the built-in default is `/usr/sbin/sendmail` or
 	`/usr/lib/sendmail` if such a program is available, or
-	`localhost` otherwise.
+	`localhost` otherwise. Also, a built-in default is used if
+	`<host>` or 'sendemail.smtpserver' is the empty string (for
+	example, if '--smtp-server ""' is specified on the command line).
 
 --smtp-server-port=<port>::
 	Specifies a port different from the default port (SMTP
diff --git a/git-send-email.perl b/git-send-email.perl
index 1a20b2c..5e669c7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -675,7 +675,7 @@ if (defined $initial_reply_to) {
 	$initial_reply_to = "<$initial_reply_to>" if $initial_reply_to ne '';
 }
 
-if (defined $smtp_server) {
+if (defined $smtp_server && $smtp_server ne '') {
 
 	$smtp_server_is_a_command = ($smtp_server =~ m{^/});
 
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 04/12] send-email: Verification for --smtp-server and --smpt-server-port

[Michael Witten]

The server URI is verified according to RFCs (including IPv6 support).

The $smtp_server and $smtp_server_port setup code has been moved
higher in the file, so that send-email fails fast if they are bad.

Now, the 'host:port' server URI form is handled regardless of the
documentation deficiencies of Net::SMTP{,::SSL}.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/git-send-email.txt |   50 ++++++++++++-----
 git-send-email.perl              |  115 +++++++++++++++++++++++++++++++------
 2 files changed, 131 insertions(+), 34 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index ae01632..92985ee 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -125,23 +125,43 @@ specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
 user is prompted for a password while the input is masked for privacy.
 
 --smtp-server=<server>::
-	If set, specifies the outgoing SMTP server to use (e.g.
-	`smtp.example.com` or a raw IP address).  Alternatively it can
-	specify a full pathname of a sendmail-like program instead;
-	the program must support the `-i` option.  Default value can
-	be specified by the 'sendemail.smtpserver' configuration
-	variable; the built-in default is `/usr/sbin/sendmail` or
-	`/usr/lib/sendmail` if such a program is available, or
-	`localhost` otherwise. Also, a built-in default is used if
-	`<host>` or 'sendemail.smtpserver' is the empty string (for
-	example, if '--smtp-server ""' is specified on the command line).
+	Specifies the outgoing SMTP server to use. The server may be
+	given as a domain name (e.g. `smtp.example.com:587`), raw IP
+	address (e.g. `192.168.0.1`), or absolute path to a command
+	(e.g. `/usr/sbin/sendmail`).
++
+Usually a server is specified with its URI form:
++
+	host[:port]
++
+The optional port, which identifies a particular service at the
+given host, is normally provided as a non-negative integer that
+is representable in 16-bits; however, it is possible to use any
+string composed of some combination of alphanumeric characters, the
+underscore, the hyphen, and punctuation (special) characters, so
+that symbolic service names can be employed (as defined by, say,
+`/etc/services` on Unix systems).
++
+Alternatively the server can be specified as an absolute path to
+a sendmail-like program; in particular, the program must support
+`sendmail's` `-i` option.
++
+Default value can be specified by the 'sendemail.smtpserver'
+configuration variable; the built-in default is `/usr/sbin/sendmail`
+or `/usr/lib/sendmail` if such a program is available, or `localhost`
+otherwise. Also, a built-in default is used if `<server>` or
+'sendemail.smtpserver' is the empty string (for example, if
+'--smtp-server ""' is specified on the command line).
 
 --smtp-server-port=<port>::
-	Specifies a port different from the default port (SMTP
-	servers typically listen to smtp port 25 and ssmtp port
-	465); symbolic service names (e.g. "submission" instead of 587)
-	are also accepted. The port can also be set with the
-	'sendemail.smtpserverport' configuration variable.
+	Specifies a port different from the default port (SMTP servers
+	typically listen to port 25, 587, or even non-standard 465); symbolic
+	service names (e.g. "submission" instead of 587) are also accepted,
+	provided the underlying system handles mappings in something like
+	`/etc/services`; such service names may only be composed of some
+	combination of alphanumeric characters, the underscore, the hyphen,
+	and punctuation (special) characters. The port can also be set with
+	the 'sendemail.smtpserverport' configuration variable.
 
 --smtp-ssl::
 	Legacy alias for '--smtp-encryption ssl'.
diff --git a/git-send-email.perl b/git-send-email.perl
index 5e669c7..e2c7954 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -61,7 +61,7 @@ git send-email [options] <file | directory | rev-list options >
 
   Sending:
     --envelope-sender   <address>  * Email envelope sender.
-    --smtp-server        <server>  * Outgoing SMTP server. <host | command>
+    --smtp-server        <server>  * Outgoing SMTP server: <host[:port] | cmd>.
     --smtp-server-port     <port>  * Outgoing SMTP server port; symbolic too.
     --smtp-user        <username>  * Username for SMTP-AUTH.
     --smtp-pass       [<password>] * Password for SMTP-AUTH; not necessary.
@@ -328,6 +328,101 @@ foreach my $setting (values %config_bool_settings) {
 # 'default' encryption is none -- this only prevents a warning
 $smtp_encryption = '' unless (defined $smtp_encryption);
 
+# Define a function that verifies a server URI and returns
+# its host and port parts:
+
+sub parse_server_URI($) {
+
+	# These regular experssions were derived from:
+	#   * RFC 2373 (Appendix B) : IP Version 6 Addressing Architecture
+	#   * RFC 2732 (Section  3) : Format for Literal IPv6 Addresses in URL's
+	#   * RFC 2396 (Sec. 3.2.2) : Uniform Resource Identifiers (URI): Generic Syntax
+	# Also, ports are allowed to be symbolic, so that /etc/services mappings
+	# can be used.
+
+	# Define a port (RFC 2396 and extension):
+
+	my $port = qr/[\w[:punct:]]+/;
+
+	# Define an IPv4 address (RFC 2373):
+
+	my $dig3        = qr/\d{1,3}/;
+	my $IPv4address = qr/$dig3(?:\.$dig3){3}/;
+
+	# Define an IPv6 address (RFC 2373):
+
+	my $hex4        = qr/[[:xdigit:]]{1,4}/;
+	my $hexseq      = qr/$hex4(?::$hex4)*/;  		# RFC 2373 is really that loose.
+	my $hexpart     = qr/$hexseq|$hexseq?::$hexseq?/;
+	my $IPv6address = qr/$hexpart(?::$IPv4address)?/;
+
+	# Define an IPv6 literal (RFC 2732):
+
+	my $IPv6reference = qr/\[$IPv6address\]/;
+
+	# Define a server URI (RFC 2396 and RFC 2732):
+
+	my $toplabel    = qr/[[:alpha:]](?:(?:[[:alnum:]]|-)*[[:alpha:]])?/;
+	my $domainlabel = qr/[[:alnum:]](?:(?:[[:alnum:]]|-)*[[:alnum:]])?/;
+	my $hostname    = qr/(?:$domainlabel\.)*$toplabel\.?/;
+	my $host        = qr/$hostname|$IPv4address|$IPv6reference/;
+	my $hostport    = qr/^($host)(?::($port))?$/;
+
+	# Phew! Now parse
+
+	return shift =~ $hostport;
+}
+
+# Figure out how to contact the SMTP server.
+# After this code, $smtp_server_port is either
+# valid or undef:
+
+if (defined $smtp_server && $smtp_server ne '') {
+
+	if ($smtp_server_is_a_command = ($smtp_server =~ m{^/})) {
+
+		die "--smtp-server: The SMTP server command does not exist: $smtp_server\n"
+			unless -x $smtp_server;
+
+		print STDERR "--smtp-server: Using command '$smtp_server'; ignoring --smtp-server-port='$smtp_server_port'\n"
+			if defined $smtp_server_port;
+
+	} else {
+
+		($smtp_server, my $port) = parse_server_URI $smtp_server
+			or die "--smtp-server: Not a valid server URI: '$smtp_server'\n";
+
+		if (defined $smtp_server_port) {
+
+			$smtp_server_port =~ /[\w[:punct:]]+/ or die "--smtp-server-port: Invalid port: '$smtp_server_port'\n";
+
+			print STDERR "--smtp-server-port: Using port '$smtp_server_port'; ignoring --smtp-server's port '$port'\n"
+				if defined $port;
+
+		} else {
+
+			$smtp_server_port = $port;
+		}
+	}
+
+} else { # use a default:
+
+	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
+		if (-x $_) {
+			$smtp_server = $_;
+			$smtp_server_is_a_command = 1;
+
+			print STDERR "--smtp-server: Using command '$smtp_server'; ignoring --smtp-server-port='$smtp_server_port'\n"
+				if defined $smtp_server_port;
+
+			last;
+		}
+	}
+
+	$smtp_server = 'localhost'; # 127.0.0.1 is not compatible with IPv6
+		unless $smtp_server_is_a_command;
+}
+
 # Set CC suppressions
 my(%suppress_cc);
 if (@suppress_cc) {
@@ -675,24 +770,6 @@ if (defined $initial_reply_to) {
 	$initial_reply_to = "<$initial_reply_to>" if $initial_reply_to ne '';
 }
 
-if (defined $smtp_server && $smtp_server ne '') {
-
-	$smtp_server_is_a_command = ($smtp_server =~ m{^/});
-
-} else { # use a default:
-
-	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
-		if (-x $_) {
-			$smtp_server = $_;
-			$smtp_server_is_a_command = 1;
-			last;
-		}
-	}
-
-	$smtp_server = 'localhost'; # 127.0.0.1 is not compatible with IPv6
-		unless $smtp_server_is_a_command;
-}
-
 if ($compose && $compose > 0) {
 	@files = ($compose_filename . ".final", @files);
 }
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Michael Witten]

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 git-send-email.perl |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index e2c7954..2727c77 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -967,15 +967,22 @@ X-Mailer: git-send-email $gitversion
 	}
 
 	if ($dry_run) {
+
 		# We don't want to send the email.
+
 	} elsif ($smtp_server_is_a_command) {
-		my $pid = open my $sm, '|-';
-		defined $pid or die $!;
-		if (!$pid) {
-			exec($smtp_server, @sendmail_parameters) or die $!;
-		}
-		print $sm "$header\n$message";
-		close $sm or die $?;
+
+		(my $pid = open my $pipe, '|-')
+			// die "Could not fork to run '$smtp_server': $!\n";
+
+		$pid or exec($smtp_server, @sendmail_parameters)
+			or die "Could not run '$smtp_server': $!\n";
+
+		local $SIG{PIPE} = 'IGNORE';
+
+		print $pipe "$header\n$message";
+		close $pipe or die "'$smtp_server' exited with status $?: $!\n";
+
 	} else {
 
 		if (!defined $smtp_server) {
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 06/12] send-email: Cleanup and streamline the SMTP code in send_message

[Michael Witten]

Some of the code was never used or not necessary; it should
be easier to read now.

The code could even be simplified further, because Net::SMTP{,::SSL}
both take the PORT variable in their new methods (which, as of this
commit, are actually the same method). Moreover, both take a server
URI of the form 'host:port' that trumps any value passed to PORT.

Unfortunately, none of this is documented publicly, so it isn't
exploited out of purity.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 git-send-email.perl |   93 +++++++++++++++++++++++++++-----------------------
 1 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 2727c77..6e2ea2c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -985,67 +985,74 @@ X-Mailer: git-send-email $gitversion
 
 	} else {
 
-		if (!defined $smtp_server) {
-			die "The required SMTP server is not properly defined."
-		}
+		goto SEND_MAIL if $smtp;
+
+		if ($smtp_encryption =~ /ssl/i) {
+
+			use Net::SMTP::SSL;
+			$smtp = Net::SMTP::SSL->new($smtp_server, Port => $smtp_server_port // 465)
+				or die "Could not connect to SSL SMTP server '$smtp_server:$smtp_server_port'\n";
+
+		} else {
+
+			use Net::SMTP;
+
+			my $server_URI = (defined $smtp_server_port)
+						? "$smtp_server:$smtp_server_port"
+						:  $smtp_server;
+
+			$smtp = Net::SMTP->new($server_URI)
+				or die "Could not connect to SMTP server: '$server_URI'\n";
+
+			if ($smtp_encryption =~ /tls/i) {
 
-		if ($smtp_encryption eq 'ssl') {
-			$smtp_server_port ||= 465; # ssmtp
-			require Net::SMTP::SSL;
-			$smtp ||= Net::SMTP::SSL->new($smtp_server, Port => $smtp_server_port);
-		}
-		else {
-			require Net::SMTP;
-			$smtp ||= Net::SMTP->new((defined $smtp_server_port)
-						 ? "$smtp_server:$smtp_server_port"
-						 : $smtp_server);
-			if ($smtp_encryption eq 'tls') {
-				require Net::SMTP::SSL;
 				$smtp->command('STARTTLS');
-				$smtp->response();
-				if ($smtp->code == 220) {
-					$smtp = Net::SMTP::SSL->start_SSL($smtp)
-						or die "STARTTLS failed! ".$smtp->message;
-					$smtp_encryption = '';
-					# Send EHLO again to receive fresh
-					# supported commands
-					$smtp->hello();
-				} else {
-					die "Server does not support STARTTLS! ".$smtp->message;
-				}
-			}
-		}
+				$smtp->response(); # so $smtp->code works.
+
+				die "Server does not support STARTTLS: " . $smtp->message . "\n"
+					unless $smtp->code == 220;
+
+				use Net::SMTP::SSL;
+				Net::SMTP::SSL->start_SSL($smtp)
+					or die "STARTTLS failed! " . $smtp->message . "\n";
+
+				# Send EHLO again to receive fresh
+				# supported commands:
 
-		if (!$smtp) {
-			die "Unable to initialize SMTP properly.  Is there something wrong with your config?";
+				$smtp->hello();
+			}
 		}
 
 		if (defined $smtp_authuser) {
 
-			if (!defined $smtp_authpass) {
+			unless (defined $smtp_authpass) {
 
 				system "stty -echo";
 
-				do {
+				{
 					print "Password: ";
-					$_ = <STDIN>;
+					$smtp_authpass = <STDIN>;
 					print "\n";
-				} while (!defined $_);
-
-				chomp($smtp_authpass = $_);
+					redo unless defined $smtp_authpass;
+					chomp($smtp_authpass);
+				}
 
 				system "stty echo";
 			}
 
-			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
+			$smtp->auth($smtp_authuser, $smtp_authpass)
+				or die "Could not authenticate '$smtp_authuser': " . $smtp->message . "\n";
 		}
 
-		$smtp->mail( $raw_from ) or die $smtp->message;
-		$smtp->to( @recipients ) or die $smtp->message;
-		$smtp->data or die $smtp->message;
-		$smtp->datasend("$header\n$message") or die $smtp->message;
-		$smtp->dataend() or die $smtp->message;
-		$smtp->code =~ /250|200/ or die "Failed to send $subject\n".$smtp->message;
+		SEND_MAIL:
+
+		$smtp->mail($raw_from)               and
+		$smtp->to(@recipients)               and
+		$smtp->data                          and
+		$smtp->datasend("$header\n$message") and
+		$smtp->dataend                       or
+
+		die "Failed to send '$subject': " . $smtp->message . "\n";
 	}
 	if ($quiet) {
 		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 07/12] send-email: Cleanup send_message 'log' code

[Michael Witten]

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 git-send-email.perl |   33 ++++++++++++++++-----------------
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 6e2ea2c..f3e2ccd 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1054,24 +1054,23 @@ X-Mailer: git-send-email $gitversion
 
 		die "Failed to send '$subject': " . $smtp->message . "\n";
 	}
-	if ($quiet) {
-		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
+
+	print "\n" unless $quiet;
+	print 'Dry-' if $dry_run;
+	print("Sent $subject\n"), return 1 if $quiet;
+	print "OK. Log says:\n";
+
+	if ($smtp_server_is_a_command) {
+		print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
+		print "$header\n";
+		print "Result: OK\n";
 	} else {
-		print (($dry_run ? "Dry-" : "")."OK. Log says:\n");
-		unless ($smtp_server_is_a_command) {
-			print "Server: $smtp_server\n";
-			print "MAIL FROM:<$raw_from>\n";
-			print "RCPT TO:".join(',',(map { "<$_>" } @recipients))."\n";
-		} else {
-			print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
-		}
-		print $header, "\n";
-		if ($smtp) {
-			print "Result: ", $smtp->code, ' ',
-				($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
-		} else {
-			print "Result: OK\n";
-		}
+		print "Server: $smtp_server\n";
+		print "MAIL FROM:<$raw_from>\n";
+		print "RCPT TO:".join(',',(map { "<$_>" } @recipients))."\n";
+		print "$header\n";
+		print("Result: OK\n"), return 1 if $dry_run;
+		print "Result: ", $smtp->code, ' ', ($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
 	}
 
 	return 1;
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 08/12] send-email: Move Subject sanitization from --compose code to send_message

[Michael Witten]

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 git-send-email.perl |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index f3e2ccd..b9a6d42 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -676,12 +676,7 @@ EOT
 			$need_8bit_cte = 0;
 		} elsif (/^Subject:\s*(.+)\s*$/i) {
 			$initial_subject = $1;
-			my $subject = $initial_subject;
-			$_ = "Subject: " .
-				($subject =~ /[^[:ascii:]]/ ?
-				 quote_rfc2047($subject) :
-				 $subject) .
-				"\n";
+			next;
 		} elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
 			$initial_reply_to = $1;
 			next;
@@ -914,9 +909,11 @@ sub send_message
 	my $sanitized_sender = sanitize_address($sender);
 	make_message_id() unless defined($message_id);
 
+	my $sanitized_subject = ($subject =~ /[^[:ascii:]]/) ? quote_rfc2047($subject) : $subject;
+
 	my $header = "From: $sanitized_sender
 To: $to${ccline}
-Subject: $subject
+Subject: $sanitized_subject
 Date: $date
 Message-Id: $message_id
 X-Mailer: git-send-email $gitversion
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 09/12] Docs: send-email: Reorganize the CONFIGURATION section

[Michael Witten]

For each configuration variable, the reader is either prompted
to seek out the description of the corresponding command line option,
or a description is given if there is no corresponding command line
option.

The CONFIGURATION section has also been recast into the Composing,
Sending, Automating, and Administering sections, and configuration
variables are listed in alphabetical order within each section.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/git-send-email.txt |   83 ++++++++++++++++++++++++++++++++++----
 1 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 92985ee..6f770d0 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -282,6 +282,28 @@ default to '--validate'.
 CONFIGURATION
 -------------
 
+Composing
+~~~~~~~~~
+
+sendemail.bcc::
+	See '--bcc'
+
+sendemail.cc::
+	See '--cc'
+
+sendemail.multiedit::
+	If true (default), a single editor instance will be spawned to edit
+	files you have to edit (patches when '--annotate' is used, and the
+	summary when '--compose' is used). If false, files will be edited one
+	after the other, spawning a new editor each time.
+
+sendemail.to::
+	See '--from'
+
+
+Sending
+~~~~~~~
+
 sendemail.aliasesfile::
 	To avoid typing long email addresses, point this to one or more
 	email aliases files.  You must also supply 'sendemail.aliasfiletype'.
@@ -290,16 +312,61 @@ sendemail.aliasfiletype::
 	Format of the file(s) specified in sendemail.aliasesfile. Must be
 	one of 'mutt', 'mailrc', 'pine', or 'gnus'.
 
-sendemail.multiedit::
-	If true (default), a single editor instance will be spawned to edit
-	files you have to edit (patches when '--annotate' is used, and the
-	summary when '--compose' is used). If false, files will be edited one
-	after the other, spawning a new editor each time.
+sendemail.envelopesender::
+	See '--envelope-sender'
+
+sendemail.smtpencryption::
+	See '--smtp-encryption'
+
+sendemail.smtppass::
+	See '--smtp-pass'
+
+sendemail.smtpserver::
+	See '--smtp-server'
+
+sendemail.smtpserverport::
+	See '--smtp-server-port'
+
+sendemail.smtpuser::
+	See '--smtp-ssl'
+
+
+Automating
+~~~~~~~~~~
+
+sendemail.cccmd::
+	See '--cc-cmd'
+
+sendemail.chainreplyto::
+	See '--[no-]chain-reply-to'
+
+sendemail.identity::
+	See '--identity'
+
+sendemail.signedoffbycc::
+	See '--[no-]signed-off-by-cc'
+
+sendemail.sleep::
+	See '--sleep'
+
+sendemail.suppresscc::
+	See '--suppress-cc'
+
+sendemail.suppressfrom::
+	See '--[no-]suppress-from'
+
+sendemail.thread::
+	See '--[no-]thread'
+
+
+Administering
+~~~~~~~~~~~~~
 
 sendemail.confirm::
-	Sets the default for whether to confirm before sending. Must be
-	one of 'always', 'never', 'cc', 'compose', or 'auto'. See '--confirm'
-	in the previous section for the meaning of these values.
+	See '--confirm'
+
+sendemail.validate::
+	See '--dry-run'
 
 
 Author
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 10/12] Docs: Embolden the CONFIGURATION references

[Michael Witten]

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/git-send-email.txt |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 6f770d0..93c39e1 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -41,7 +41,7 @@ Composing
 
 --annotate::
 	Review and edit each patch you're about to send. See the
-	CONFIGURATION section for 'sendemail.multiedit'.
+	*CONFIGURATION* section for 'sendemail.multiedit'.
 
 --bcc=<address>::
 	Specify a "Bcc:" value for each email. Default is the value of
@@ -67,7 +67,7 @@ and In-Reply-To headers will be used unless they are removed.
 +
 Missing From or In-Reply-To headers will be prompted for.
 +
-See the CONFIGURATION section for 'sendemail.multiedit'.
+See the *CONFIGURATION* section for 'sendemail.multiedit'.
 
 --from=<address>::
 	Specify the sender of the emails.  This will default to
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 11/12] Docs: send-email: Clarification of sendemail.<identity>

[Michael Witten]

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/git-send-email.txt |   43 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 93c39e1..3b36ac7 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -194,6 +194,7 @@ Automating
 	'sendemail.<identity>' subsection to take precedence over
 	values in the 'sendemail' section. The default identity is
 	the value of the 'sendemail.identity' configuration variable.
+	See the *CONFIGURATION* section for more details.
 
 --[no-]signed-off-by-cc::
 	If this is set, add emails found in Signed-off-by: or Cc: lines to the
@@ -282,6 +283,48 @@ default to '--validate'.
 CONFIGURATION
 -------------
 
+Configuration subsections are very useful in combination with '--identity'.
+For instance, consider a configuration file that contains the following:
+
+	...
+
+	[sendemail]
+		smtpencryption = tls
+		smtpserver     = smtp.example.com
+		smtpuser       = user@example.com
+		suppresscc     = self
+		confirm        = always
+		identity       = test
+
+	[sendemail "test"]
+		to = user@example.com
+
+	[sendemail "git-rfc"]
+		to = git@vger.kernel.org
+
+	[sendemail "git-submit"]
+		to      = gitster@pobox.com
+		cc      = git@vger.kernel.org
+		confirm = never
+
+	...
+
+With this configuration, 'send-email' processes the variables in `[sendemail]`.
+Then, 'sendemail.identity' tells 'send-email' to process the variables in the
+`[sendemail "test"]` subsection. Thus:
+
+	git send-email <patch>
+
+would send `<patch>` to `user@example.com` as a "test". When the user
+decides that `<patch>` is ready to be sent for real, the user could
+email the 'git' mailing list to make a request for comments (RFC):
+
+	git send-email <patch> --identity git-rfc
+
+Then, for final submission:
+
+	git send-email <patch> --identity git-submit
+
 Composing
 ~~~~~~~~~
 
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5 12/12] Docs: send-email: git send-email -> 'send-email'

[Michael Witten]

For the sake of consistency.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 Documentation/git-send-email.txt |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 3b36ac7..11965eb 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -59,7 +59,7 @@ The --cc option must be repeated for each user you want on the cc list.
 	Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an
 	introductory message for the patch series.
 +
-When '--compose' is used, git send-email will use the From, Subject, and
+When '--compose' is used, 'send-email' will use the From, Subject, and
 In-Reply-To headers specified in the message. If the body of the message
 (what you type after the headers and a blank line) only contains blank
 (or GIT: prefixed) lines the summary won't be sent, but From, Subject,
@@ -244,7 +244,7 @@ Administering
 --
 - 'always' will always confirm before sending
 - 'never' will never confirm before sending
-- 'cc' will confirm before sending when send-email has automatically
+- 'cc' will confirm before sending when 'send-email' has automatically
   added addresses from the patch to the Cc list
 - 'compose' will confirm before sending the first message when using --compose.
 - 'auto' is equivalent to 'cc' + 'compose'
@@ -261,10 +261,10 @@ have been specified, in which case default to 'compose'.
 	When an argument may be understood either as a reference or as a file name,
 	choose to understand it as a format-patch argument ('--format-patch')
 	or as a file name ('--no-format-patch'). By default, when such a conflict
-	occurs, git send-email will fail.
+	occurs, 'send-email' will fail.
 
 --quiet::
-	Make git-send-email less verbose.  One line per email should be
+	Make 'send-email' less verbose.  One line per email should be
 	all that is output.
 
 --[no-]validate::
-- 
1.6.2.2.479.g2aec

Re: [PATCH RFC3.5 03/12] send-email: Interpret --smtp-server "" as "use a default".

[Wesley J. Landaker]

On Saturday 18 April 2009 11:01:59 Michael Witten wrote:
>  	`/usr/lib/sendmail` if such a program is available, or
> -	`localhost` otherwise.
> +	`localhost` otherwise. Also, a built-in default is used if

I think you may have meant:
+	`localhost` otherwise. Also, the built-in default is used if

> +	`<host>` or 'sendemail.smtpserver' is the empty string (for
> +	example, if '--smtp-server ""' is specified on the command line).

Re: [PATCH RFC3.5 03/12] send-email: Interpret --smtp-server "" as "use a default".

[Michael Witten]

On Sat, Apr 18, 2009 at 18:35, Wesley J. Landaker <wjl@icecavern.net> wrote:
> On Saturday 18 April 2009 11:01:59 Michael Witten wrote:
>>       `/usr/lib/sendmail` if such a program is available, or
>> -     `localhost` otherwise.
>> +     `localhost` otherwise. Also, a built-in default is used if
>
> I think you may have meant:
> +       `localhost` otherwise. Also, the built-in default is used if

Good eye! However, I deliberately chose 'a built-in', because there
are multiple built-in values. However, 'the built-in' is used in the
previous sentence, now that I take a closer look myself.

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Jay Soffian]

On Sat, Apr 18, 2009 at 1:02 PM, Michael Witten <mfwitten@gmail.com> wrote:
> diff --git a/git-send-email.perl b/git-send-email.perl
> index e2c7954..2727c77 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -967,15 +967,22 @@ X-Mailer: git-send-email $gitversion
>        }
>
>        if ($dry_run) {
> +
>                # We don't want to send the email.
> +
>        } elsif ($smtp_server_is_a_command) {
> -               my $pid = open my $sm, '|-';
> -               defined $pid or die $!;
> -               if (!$pid) {
> -                       exec($smtp_server, @sendmail_parameters) or die $!;
> -               }
> -               print $sm "$header\n$message";
> -               close $sm or die $?;
> +
> +               (my $pid = open my $pipe, '|-')
> +                       // die "Could not fork to run '$smtp_server': $!\n";

What is '//' about?

j.

Re: [PATCH RFC3.5 08/12] send-email: Move Subject sanitization from --compose code to send_message

[Jay Soffian]

On Sat, Apr 18, 2009 at 1:02 PM, Michael Witten <mfwitten@gmail.com> wrote:
> +       my $sanitized_subject = ($subject =~ /[^[:ascii:]]/) ? quote_rfc2047($subject) : $subject;

I wonder if it would be clearer to always call quote_rfc2047, then
have that function just return its input unaltered if quoting is not
needed.

j.

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Michael Witten]

On Sat, Apr 18, 2009 at 20:51, Jay Soffian <jaysoffian@gmail.com> wrote:
> What is '//' about?

It's called the Logical Defined-OR:

    http://perldoc.perl.org/perlop.html#C-style-Logical-Defined-Or

> Although it has no direct equivalent in C, Perl's // operator is related to its C-style or. In fact, it's exactly the same as ||, except that it tests the left hand side's definedness instead of its truth. Thus, $a // $b is similar to defined($a) || $b  (except that it returns the value of $a rather than the value of defined($a)) and is exactly equivalent to defined($a) ? $a : $b . This is very useful for providing default values for variables. If you actually want to test if at least one of $a  and $b  is defined, use defined($a // $b) ...

However, I wonder if your comment is a veiled quip at my "Improve
redability" claim (which is also ironically unreadable). :-)

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Thomas Adam]

2009/4/19 Michael Witten <mfwitten@gmail.com>:
> However, I wonder if your comment is a veiled quip at my "Improve
> redability" claim (which is also ironically unreadable). :-)

More concerning is that it's a perl 5.10ism -- you cannot assume that
perl 5.10 is installed on all platforms.  I really wouldn't use this
construct.

-- Thomas Adam

Re: [PATCH RFC3.5 08/12] send-email: Move Subject sanitization from --compose code to send_message

[Michael Witten]

On Sat, Apr 18, 2009 at 20:54, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Sat, Apr 18, 2009 at 1:02 PM, Michael Witten <mfwitten@gmail.com> wrote:
>> +       my $sanitized_subject = ($subject =~ /[^[:ascii:]]/) ? quote_rfc2047($subject) : $subject;
>
> I wonder if it would be clearer to always call quote_rfc2047, then
> have that function just return its input unaltered if quoting is not
> needed.

It actually ALWAYS changes the input. This code:

	sub quote_rfc2047 {
		local $_ = shift;
		my $encoding = shift || 'utf-8';
		s/([^-a-zA-Z0-9!*+\/])/sprintf("=%02X", ord($1))/eg;
		s/(.*)/=\?$encoding\?q\?$1\?=/;
		return $_;
	}

	print quote_rfc2047("Yiarg #&@$! This output is messy!") . "\n"

gives this output:

	=?utf-8?q?Yiarg=20=23=26!=20This=20output=20is=20messy!?=

Therfore the /[^[:ascii:]]/ check actually saves us from corrupting
already encoded subjects or from encoding ones that shouldn't be. In
fact, I'm not entirely sure the original code is correct to make that
check, because some of the characters that are replaced are ascii
characters. This is all rather strange.

Thanks! I should have tested it more; I'm constantly amazed by my
inability to see the problems I introduce ;-)

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Michael Witten]

On Sat, Apr 18, 2009 at 21:17, Thomas Adam <thomas.adam22@gmail.com> wrote:
> More concerning is that it's a perl 5.10ism -- you cannot assume that
> perl 5.10 is installed on all platforms.  I really wouldn't use this
> construct.

See that's the thing: How am I supposed to know it's a perl 5.10ism?
The Perl docs give absolutely no clue; Perl[5] is based way too much
on practice rather than theory, because only people that have been
using it since day 1 know what's going on. A couple of weeks ago, I
went to the perldoc website and just read each website one after the
other---that is my total knowledge of Perl, and already I've caught
flack a number of times for having used 'new-fangled technologies'; I
really wish the docs would specify when a feature became available.

... add that to the list of perldoc shortcomings.

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> On Sat, Apr 18, 2009 at 21:17, Thomas Adam <thomas.adam22@gmail.com> wrote:
>> More concerning is that it's a perl 5.10ism -- you cannot assume that
>> perl 5.10 is installed on all platforms.  I really wouldn't use this
>> construct.
>
> See that's the thing: How am I supposed to know it's a perl 5.10ism?

Read perldelta.pod  They come with your Perl distribution.

Debian based distro installs them in

    /usr/share/perl/5.10.0/pod/perl*delta.pod

and Fedora seems to have them in

    /usr/lib/perl5/5.10.0/pod/perl*delta.pod

I think we stick to something like early 5.8 to support platforms with
infrequent updates to Perl.

[PATCH RFC3.5.1 05/12] send-email: Improve readability and error-handling in send_message's sendmail code

[Michael Witten]

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 git-send-email.perl |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 559e17b..3a140cd 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -967,15 +967,22 @@ X-Mailer: git-send-email $gitversion
 	}
 
 	if ($dry_run) {
+
 		# We don't want to send the email.
+
 	} elsif ($smtp_server_is_a_command) {
-		my $pid = open my $sm, '|-';
-		defined $pid or die $!;
-		if (!$pid) {
-			exec($smtp_server, @sendmail_parameters) or die $!;
-		}
-		print $sm "$header\n$message";
-		close $sm or die $?;
+
+		defined (my $pid = open my $pipe, '|-')
+			or die "Could not fork to run '$smtp_server': $!\n";
+
+		$pid or exec($smtp_server, @sendmail_parameters)
+			or die "Could not run '$smtp_server': $!\n";
+
+		local $SIG{PIPE} = 'IGNORE';
+
+		print $pipe "$header\n$message";
+		close $pipe or die "'$smtp_server' exited with status $?: $!\n";
+
 	} else {
 
 		if (!defined $smtp_server) {
-- 
1.6.2.2.479.g2aec

Re: [PATCH RFC3.5 08/12] send-email: Move Subject sanitization from --compose code to send_message

[Jay Soffian]

On Sat, Apr 18, 2009 at 10:37 PM, Michael Witten <mfwitten@gmail.com> wrote:
> On Sat, Apr 18, 2009 at 20:54, Jay Soffian <jaysoffian@gmail.com> wrote:
>> On Sat, Apr 18, 2009 at 1:02 PM, Michael Witten <mfwitten@gmail.com> wrote:
>>> +       my $sanitized_subject = ($subject =~ /[^[:ascii:]]/) ? quote_rfc2047($subject) : $subject;
>>
>> I wonder if it would be clearer to always call quote_rfc2047, then
>> have that function just return its input unaltered if quoting is not
>> needed.
>
> It actually ALWAYS changes the input. This code:

I think I was not clear. My suggestion was to move the /[^[:ascii:]]/
check to the inside of quote_rfc2047 exactly so that it doesn't always
change its input. i.e.

>        sub quote_rfc2047 {
>                local $_ = shift;

Add this:
                 return $_ unless /[^[:ascii:]]/;

>                my $encoding = shift || 'utf-8';
>                s/([^-a-zA-Z0-9!*+\/])/sprintf("=%02X", ord($1))/eg;
>                s/(.*)/=\?$encoding\?q\?$1\?=/;
>                return $_;
>        }

This simplifies things for the function caller I think.

j.

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Jay Soffian]

On Sat, Apr 18, 2009 at 10:17 PM, Thomas Adam <thomas.adam22@gmail.com> wrote:
> 2009/4/19 Michael Witten <mfwitten@gmail.com>:
>> However, I wonder if your comment is a veiled quip at my "Improve
>> redability" claim (which is also ironically unreadable). :-)
>
> More concerning is that it's a perl 5.10ism -- you cannot assume that
> perl 5.10 is installed on all platforms.  I really wouldn't use this
> construct.

Exactly. I started with perl4 (no really...), then moved away from
hard-core perl coding around 2000 and have been just a perl dabbler
since then. So I didn't recognize //, though I suspected it was a
newer construct. :-)

j.

[PATCH RFC3.5.1 03/12] send-email: Interpret --smtp-server "" as "use a default".

[Michael Witten]

Why not? It's at least useful for testing.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---

		NOTE: This also resulted in a similar change to:
		[PATCH RFC3.5 04/12] send-email: Verification...

 Documentation/git-send-email.txt |    4 +++-
 git-send-email.perl              |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 071e9bf..0937dd0 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -132,7 +132,9 @@ user is prompted for a password while the input is masked for privacy.
 	be specified by the 'sendemail.smtpserver' configuration
 	variable; the built-in default is `/usr/sbin/sendmail` or
 	`/usr/lib/sendmail` if such a program is available, or
-	`localhost` otherwise.
+	`localhost` otherwise. Also, the built-in default is used if
+	`<host>` or 'sendemail.smtpserver' is the empty string (for
+	example, if '--smtp-server ""' is specified on the command line).
 
 --smtp-server-port=<port>::
 	Specifies a port different from the default port (SMTP
diff --git a/git-send-email.perl b/git-send-email.perl
index fed3554..be6d171 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -675,7 +675,7 @@ if (defined $initial_reply_to) {
 	$initial_reply_to = "<$initial_reply_to>" if $initial_reply_to ne '';
 }
 
-if (defined $smtp_server) {
+if (defined $smtp_server && $smtp_server ne '') {
 
 	$smtp_server_is_a_command = ($smtp_server =~ m{^/});
 
-- 
1.6.2.2.479.g2aec

[PATCH RFC3.5.1 04/12] send-email: Verification for --smtp-server and --smpt-server-port

[Michael Witten]

The server URI is verified according to RFCs (including IPv6 support).

The $smtp_server and $smtp_server_port setup code has been moved
higher in the file, so that send-email fails fast if they are bad.

Now, the 'host:port' server URI form is handled regardless of the
documentation deficiencies of Net::SMTP{,::SSL}.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---

		NOTE: This changed due to the change in:
		[PATCH RFC3.5.1 03/12] send-email: Interpret...
		Basically, only "a built-in" was changed to
		"the built-in".

		Is it ever useful to submit patches for patches
		rather than sending the whole patch gain? Or would
		that be even more trouble?

 Documentation/git-send-email.txt |   50 ++++++++++++-----
 git-send-email.perl              |  115 +++++++++++++++++++++++++++++++------
 2 files changed, 131 insertions(+), 34 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 0937dd0..f306e88 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -125,23 +125,43 @@ specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
 user is prompted for a password while the input is masked for privacy.
 
 --smtp-server=<server>::
-	If set, specifies the outgoing SMTP server to use (e.g.
-	`smtp.example.com` or a raw IP address).  Alternatively it can
-	specify a full pathname of a sendmail-like program instead;
-	the program must support the `-i` option.  Default value can
-	be specified by the 'sendemail.smtpserver' configuration
-	variable; the built-in default is `/usr/sbin/sendmail` or
-	`/usr/lib/sendmail` if such a program is available, or
-	`localhost` otherwise. Also, the built-in default is used if
-	`<host>` or 'sendemail.smtpserver' is the empty string (for
-	example, if '--smtp-server ""' is specified on the command line).
+	Specifies the outgoing SMTP server to use. The server may be
+	given as a domain name (e.g. `smtp.example.com:587`), raw IP
+	address (e.g. `192.168.0.1`), or absolute path to a command
+	(e.g. `/usr/sbin/sendmail`).
++
+Usually a server is specified with its URI form:
++
+	host[:port]
++
+The optional port, which identifies a particular service at the
+given host, is normally provided as a non-negative integer that
+is representable in 16-bits; however, it is possible to use any
+string composed of some combination of alphanumeric characters, the
+underscore, the hyphen, and punctuation (special) characters, so
+that symbolic service names can be employed (as defined by, say,
+`/etc/services` on Unix systems).
++
+Alternatively the server can be specified as an absolute path to
+a sendmail-like program; in particular, the program must support
+`sendmail's` `-i` option.
++
+Default value can be specified by the 'sendemail.smtpserver'
+configuration variable; the built-in default is `/usr/sbin/sendmail`
+or `/usr/lib/sendmail` if such a program is available, or `localhost`
+otherwise. Also, the built-in default is used if `<server>` or
+'sendemail.smtpserver' is the empty string (for example, if
+'--smtp-server ""' is specified on the command line).
 
 --smtp-server-port=<port>::
-	Specifies a port different from the default port (SMTP
-	servers typically listen to smtp port 25 and ssmtp port
-	465); symbolic service names (e.g. "submission" instead of 587)
-	are also accepted. The port can also be set with the
-	'sendemail.smtpserverport' configuration variable.
+	Specifies a port different from the default port (SMTP servers
+	typically listen to port 25, 587, or even non-standard 465); symbolic
+	service names (e.g. "submission" instead of 587) are also accepted,
+	provided the underlying system handles mappings in something like
+	`/etc/services`; such service names may only be composed of some
+	combination of alphanumeric characters, the underscore, the hyphen,
+	and punctuation (special) characters. The port can also be set with
+	the 'sendemail.smtpserverport' configuration variable.
 
 --smtp-ssl::
 	Legacy alias for '--smtp-encryption ssl'.
diff --git a/git-send-email.perl b/git-send-email.perl
index be6d171..559e17b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -61,7 +61,7 @@ git send-email [options] <file | directory | rev-list options >
 
   Sending:
     --envelope-sender   <address>  * Email envelope sender.
-    --smtp-server        <server>  * Outgoing SMTP server. <host | command>
+    --smtp-server        <server>  * Outgoing SMTP server: <host[:port] | cmd>.
     --smtp-server-port     <port>  * Outgoing SMTP server port; symbolic too.
     --smtp-user        <username>  * Username for SMTP-AUTH.
     --smtp-pass       [<password>] * Password for SMTP-AUTH; not necessary.
@@ -328,6 +328,101 @@ foreach my $setting (values %config_bool_settings) {
 # 'default' encryption is none -- this only prevents a warning
 $smtp_encryption = '' unless (defined $smtp_encryption);
 
+# Define a function that verifies a server URI and returns
+# its host and port parts:
+
+sub parse_server_URI($) {
+
+	# These regular experssions were derived from:
+	#   * RFC 2373 (Appendix B) : IP Version 6 Addressing Architecture
+	#   * RFC 2732 (Section  3) : Format for Literal IPv6 Addresses in URL's
+	#   * RFC 2396 (Sec. 3.2.2) : Uniform Resource Identifiers (URI): Generic Syntax
+	# Also, ports are allowed to be symbolic, so that /etc/services mappings
+	# can be used.
+
+	# Define a port (RFC 2396 and extension):
+
+	my $port = qr/[\w[:punct:]]+/;
+
+	# Define an IPv4 address (RFC 2373):
+
+	my $dig3        = qr/\d{1,3}/;
+	my $IPv4address = qr/$dig3(?:\.$dig3){3}/;
+
+	# Define an IPv6 address (RFC 2373):
+
+	my $hex4        = qr/[[:xdigit:]]{1,4}/;
+	my $hexseq      = qr/$hex4(?::$hex4)*/;  		# RFC 2373 is really that loose.
+	my $hexpart     = qr/$hexseq|$hexseq?::$hexseq?/;
+	my $IPv6address = qr/$hexpart(?::$IPv4address)?/;
+
+	# Define an IPv6 literal (RFC 2732):
+
+	my $IPv6reference = qr/\[$IPv6address\]/;
+
+	# Define a server URI (RFC 2396 and RFC 2732):
+
+	my $toplabel    = qr/[[:alpha:]](?:(?:[[:alnum:]]|-)*[[:alpha:]])?/;
+	my $domainlabel = qr/[[:alnum:]](?:(?:[[:alnum:]]|-)*[[:alnum:]])?/;
+	my $hostname    = qr/(?:$domainlabel\.)*$toplabel\.?/;
+	my $host        = qr/$hostname|$IPv4address|$IPv6reference/;
+	my $hostport    = qr/^($host)(?::($port))?$/;
+
+	# Phew! Now parse
+
+	return shift =~ $hostport;
+}
+
+# Figure out how to contact the SMTP server.
+# After this code, $smtp_server_port is either
+# valid or undef:
+
+if (defined $smtp_server && $smtp_server ne '') {
+
+	if ($smtp_server_is_a_command = ($smtp_server =~ m{^/})) {
+
+		die "--smtp-server: The SMTP server command does not exist: $smtp_server\n"
+			unless -x $smtp_server;
+
+		print STDERR "--smtp-server: Using command '$smtp_server'; ignoring --smtp-server-port='$smtp_server_port'\n"
+			if defined $smtp_server_port;
+
+	} else {
+
+		($smtp_server, my $port) = parse_server_URI $smtp_server
+			or die "--smtp-server: Not a valid server URI: '$smtp_server'\n";
+
+		if (defined $smtp_server_port) {
+
+			$smtp_server_port =~ /[\w[:punct:]]+/ or die "--smtp-server-port: Invalid port: '$smtp_server_port'\n";
+
+			print STDERR "--smtp-server-port: Using port '$smtp_server_port'; ignoring --smtp-server's port '$port'\n"
+				if defined $port;
+
+		} else {
+
+			$smtp_server_port = $port;
+		}
+	}
+
+} else { # use a default:
+
+	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
+		if (-x $_) {
+			$smtp_server = $_;
+			$smtp_server_is_a_command = 1;
+
+			print STDERR "--smtp-server: Using command '$smtp_server'; ignoring --smtp-server-port='$smtp_server_port'\n"
+				if defined $smtp_server_port;
+
+			last;
+		}
+	}
+
+	$smtp_server = 'localhost' # 127.0.0.1 is not compatible with IPv6
+		unless $smtp_server_is_a_command;
+}
+
 # Set CC suppressions
 my(%suppress_cc);
 if (@suppress_cc) {
@@ -675,24 +770,6 @@ if (defined $initial_reply_to) {
 	$initial_reply_to = "<$initial_reply_to>" if $initial_reply_to ne '';
 }
 
-if (defined $smtp_server && $smtp_server ne '') {
-
-	$smtp_server_is_a_command = ($smtp_server =~ m{^/});
-
-} else { # use a default:
-
-	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
-		if (-x $_) {
-			$smtp_server = $_;
-			$smtp_server_is_a_command = 1;
-			last;
-		}
-	}
-
-	$smtp_server = 'localhost' # 127.0.0.1 is not compatible with IPv6
-		unless $smtp_server_is_a_command;
-}
-
 if ($compose && $compose > 0) {
 	@files = ($compose_filename . ".final", @files);
 }
-- 
1.6.2.2.479.g2aec

Re: [PATCH RFC3.5 08/12] send-email: Move Subject sanitization from --compose code to send_message

[Michael Witten]

On Sun, Apr 19, 2009 at 09:13, Jay Soffian <jaysoffian@gmail.com> wrote:
> I think I was not clear. My suggestion was to move the /[^[:ascii:]]/
> check to the inside of quote_rfc2047 exactly so that it doesn't always
> change its input. i.e.

Ah. However, there is still the question of whether the actual email
headers are present to declare the right encoding. I don't know enough
to comment on this, though; before this patch, this quoting was
performed by code that new to right the correct "Content-Type" and
"Content-Transfer-Encoding" headers. I suppose I'll have to read the
RFC.

>
>>        sub quote_rfc2047 {
>>                local $_ = shift;
>
> Add this:
>                 return $_ unless /[^[:ascii:]]/;
>
>>                my $encoding = shift || 'utf-8';
>>                s/([^-a-zA-Z0-9!*+\/])/sprintf("=%02X", ord($1))/eg;
>>                s/(.*)/=\?$encoding\?q\?$1\?=/;
>>                return $_;
>>        }
>
> This simplifies things for the function caller I think.

I'm morally opposed to this kind of thing. The caller should be
required to test whether quote_rfc2047() is required, as it's not the
job of quote_rfc2047 to validate. Suppose that quote_rfc2047 were
actually part of a library of useful functions that my program
imports. Perhaps my program knows that it must always quote some piece
of text. Why, then, should my program be forced to waste the cycles to
perform a useless test?

IMnsHO, verification should always be done by the caller with one
exception: Interactive (human) input should always be verified,
because humans represent an unreliable component in the system (in
terms of digital systems, their asynchronous input must be
synchronized with the clocked system). WIth this model, there's are
fewer wasted cycles, because you can reuse verification across similar
functions, and the code (particularly library code) is easier to
understand.

Re: [PATCH RFC3.5 08/12] send-email: Move Subject sanitization from --compose code to send_message

[Michael Witten]

On Sun, Apr 19, 2009 at 09:39, Michael Witten <mfwitten@gmail.com> wrote:
> I'm morally opposed to this kind of thing. The caller should be
> required to test whether quote_rfc2047() is required, as it's not the
> job of quote_rfc2047 to validate. Suppose that quote_rfc2047 were
> actually part of a library of useful functions that my program
> imports. Perhaps my program knows that it must always quote some piece
> of text. Why, then, should my program be forced to waste the cycles to
> perform a useless test?
>
> IMnsHO, verification should always be done by the caller with one
> exception: Interactive (human) input should always be verified,
> because humans represent an unreliable component in the system (in
> terms of digital systems, their asynchronous input must be
> synchronized with the clocked system). WIth this model, there's are
> fewer wasted cycles, because you can reuse verification across similar
> functions, and the code (particularly library code) is easier to
> understand.

I should add, though, that making the logic of the program clear is a
good idea. In that sense, your approach makes sense. Since we 'own'
quote_rfc2047(), I'd say we could take your approach, but rename the
function to something like quote_rfc2047_if_necessary(). If
quote_rfc2047() were part of a library, I think the only moral
solution would be to insist that callers wrap it in another function
named quote_rfc2047_if_necessary().

[PATCH RFC3.5.1 08/12] send-email: Simplify --compose subject sanitation

[Michael Witten]

We might as well use the global variables while they exist; there's
no reason to print the result to a file and then read it back in.

Also, the entire file is already read and checked for:

	/[^[:ascii:]]/

by:

	my $need_8bit_cte = file_has_nonascii($compose_filename);

so we might as well use $need_8bit_cte until something less egregiously
inefficient is implemented.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 git-send-email.perl |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 3894a93..1f815d7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -675,13 +675,8 @@ EOT
 		} elsif (/^MIME-Version:/i) {
 			$need_8bit_cte = 0;
 		} elsif (/^Subject:\s*(.+)\s*$/i) {
-			$initial_subject = $1;
-			my $subject = $initial_subject;
-			$_ = "Subject: " .
-				($subject =~ /[^[:ascii:]]/ ?
-				 quote_rfc2047($subject) :
-				 $subject) .
-				"\n";
+			$initial_subject = $need_8bit_cte ? quote_rfc2047($1) : $1;
+			next;
 		} elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
 			$initial_reply_to = $1;
 			next;
-- 
1.6.2.2.479.g2aec

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> +
> +		(my $pid = open my $pipe, '|-')
> +			// die "Could not fork to run '$smtp_server': $!\n";

Have I already rejected this "5.10 or later" construct in the previous
round?  If I haven't, please consider now I have.

Re: [PATCH RFC3.5 02/12] send-email: No longer repeatedly test if $smtp_server is a command

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> This is a minor change, but it's cleaner, and it sets up the
> $smtp_server initialization code for future improvements.
> ...
> -if (!defined $smtp_server) {
> +if (defined $smtp_server) {
> +
> +	$smtp_server_is_a_command = ($smtp_server =~ m{^/});
> +
> +} else { # use a default:
> +
>  	foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
>  		if (-x $_) {
>  			$smtp_server = $_;
> +			$smtp_server_is_a_command = 1;
>  			last;
>  		}
>  	}
> -	$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
> +
> +	$smtp_server = 'localhost'; # 127.0.0.1 is not compatible with IPv6
> +		unless $smtp_server_is_a_command;

Nobody suggests to use 127.0.0.1 anymore with this change, so why not just
get rid of that comment?

Also the new statement looks wrong.

 (1) you have ';' after assignment before the statement modifier "unless";
     I do not think you meant it.  I generally *dis*like statement
     modifiers, but if you use it, at least please use it correctly.

 (2) earlier, when $smtp_server is defined (say, the name of your smtp
     host) but is not a command, we did not set smtp_server to
     'localhost', but kept the value given by the user.  Now you seem to
     kill the user's wish with this change.

I think a genuine improvement would be something like:

	if (!defined $smtp_server) {
        	$smtp_server = 'localhost';
	}

Of course if you are writing for a project that is "5.8.1 or later only",
you could say:

	$smtp_server //= 'localhost';

Re: [PATCH RFC3.5 03/12] send-email: Interpret --smtp-server "" as "use a default".

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> Why not? It's at least useful for testing.

Why so?  "Use a default" as opposed to using what?

It is unclear what "a default" is in this context.  Do configured values
count as "a default"?  I suspect not.

I think you meant "allow overriding the configured values and use the
default", but then you should spell what the defaults are (an available
local MTA binary, or SMTP port on localhost, I think).  That is much more
informative than your "Why not?..."

In any case, I find it counterintuitive to trigger "use the default" with
an option.  Something like "--ignore-config=smtp-server,smtp-port" might
make sense, though.

Re: [PATCH RFC3.5 04/12] send-email: Verification for --smtp-server and --smpt-server-port

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> The server URI is verified according to RFCs (including IPv6 support).
>
> The $smtp_server and $smtp_server_port setup code has been moved
> higher in the file, so that send-email fails fast if they are bad.
>
> Now, the 'host:port' server URI form is handled regardless of the
> documentation deficiencies of Net::SMTP{,::SSL}.

You said that in 01/12, too but I do not think there is any problem with
Perl documentation.

My installed copy of /usr/share/perl/5.10.0/Net/SMTP.pm has this:

    B<Host> - SMTP host to connect to. It may be a single scalar, as defined for
    the C<PeerAddr> option in L<IO::Socket::INET>, or a reference to
    an array with hosts to try in turn. The L</host> method will return the value
    which was used to connect to the host.

and of course PeerAddr allows host:port.

Please stop placing a false blame on others.  I think it is you who lack
ability to read the documentation correctly in this particular case.

Re: [PATCH RFC3.5 06/12] send-email: Cleanup and streamline the SMTP code in send_message

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> +				die "Server does not support STARTTLS: " . $smtp->message . "\n"
> +					unless $smtp->code == 220;

Statement modifiers merely make things even less readable, especially when
the conditional is the unlikely case.  Please do not add more of them.

	do this;
	do that;
	do something
        	if some condition that holds true most of the time;
	do some other thing;

is already hard to follow, but it is probably excusable in some cases,
because your thought can flow "ah, Ok, these four things are done in
sequence" when you are quickly scanning the code to understand the overall
structure, letting your eyes ignore the "true most of the time" part.

But the following, which is equivalent to what you did, is inexcuable.

	do this;
	do that;
	do something unusual
        	if some condition that rarely holds true;
	do some other thing;

When your eyes and brain are coasting over this segment of code, your
thought process needs to stumble and hiccup at the statment that does
something unusual, and then need to realize that it is qualified with a
statement modifier that says "this is only for rare case".

Written without statement modifier:

	do this;
	do that;
	if (some consition that rarely holds true) {
		do something unusual
        }
	do some other thing;

it is much easier to coast over; you can tell "Ah, after doing this and
that, in the normal case we do some other thing" and do not have to even
look at the details of "something unusual" part.

> -		$smtp->mail( $raw_from ) or die $smtp->message;
> -		$smtp->to( @recipients ) or die $smtp->message;
> -		$smtp->data or die $smtp->message;
> -		$smtp->datasend("$header\n$message") or die $smtp->message;
> -		$smtp->dataend() or die $smtp->message;
> -		$smtp->code =~ /250|200/ or die "Failed to send $subject\n".$smtp->message;
> +		SEND_MAIL:
> +
> +		$smtp->mail($raw_from)               and
> +		$smtp->to(@recipients)               and
> +		$smtp->data                          and
> +		$smtp->datasend("$header\n$message") and
> +		$smtp->dataend                       or
> +
> +		die "Failed to send '$subject': " . $smtp->message . "\n";

These do make things more pleasant to read.

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Michael Witten <mfwitten@gmail.com> writes:
>
>> +
>> +		(my $pid = open my $pipe, '|-')
>> +			// die "Could not fork to run '$smtp_server': $!\n";
>
> Have I already rejected this "5.10 or later" construct in the previous
> round?  If I haven't, please consider now I have.

Sorry, I should have checked myself.  defined-or "//" is 5.8.1 or later.

Now the real question was if we still support anything older, and if so
what is the bottom version?

I certainly can go with "5.8.1 or later", but I vaguely recall during the
gitweb discussion we said anything without the utf-8 support is unusable
for gitweb, but I think we also said that the rest of the git codebase
should support running with something older (5.6.1, perhaps).

Anybody?

Re: [PATCH RFC3.5 02/12] send-email: No longer repeatedly test if $smtp_server is a command

[Michael Witten]

On Sun, Apr 19, 2009 at 20:41, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> This is a minor change, but it's cleaner, and it sets up the
>> $smtp_server initialization code for future improvements.
>> ...
>> -if (!defined $smtp_server) {
>> +if (defined $smtp_server) {
>> +
>> +     $smtp_server_is_a_command = ($smtp_server =~ m{^/});
>> +
>> +} else { # use a default:
>> +
>>       foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
>>               if (-x $_) {
>>                       $smtp_server = $_;
>> +                     $smtp_server_is_a_command = 1;
>>                       last;
>>               }
>>       }
>> -     $smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
>> +
>> +     $smtp_server = 'localhost'; # 127.0.0.1 is not compatible with IPv6
>> +             unless $smtp_server_is_a_command;
>
> Nobody suggests to use 127.0.0.1 anymore with this change, so why not just
> get rid of that comment?

Fine with me.

> Also the new statement looks wrong.
>
>  (1) you have ';' after assignment before the statement modifier "unless";
>     I do not think you meant it.  I generally *dis*like statement
>     modifiers, but if you use it, at least please use it correctly.

How embarassing. That's actually been fixed on my end since I sent
that patch; for some reason, I forget to send the update; sorry for
wasting your time.

>  (2) earlier, when $smtp_server is defined (say, the name of your smtp
>     host) but is not a command, we did not set smtp_server to
>     'localhost', but kept the value given by the user.  Now you seem to
>     kill the user's wish with this change.

I think you misread the code (with the exception of the error on my
part). The code could be read:

	if $smtp_server is already defined {
		determine whether it is a command;
	} else {
		find a suitable default for it;
	}

> I think a genuine improvement would be something like:
>
>        if (!defined $smtp_server) {
>                $smtp_server = 'localhost';
>        }

You don't care to search for a possible sendmail?

Re: [PATCH RFC3.5 04/12] send-email: Verification for --smtp-server and --smpt-server-port

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Please stop placing a false blame on others.  I think it is you who lack
> ability to read the documentation correctly in this particular case.

Sorry, this came out stronger than I intended.  Your ability has never
been an issue (otherwise there wouldn't have been this patch series).  I
think you just did not read the documentation carefully enough in this
case.

Re: [PATCH RFC3.5 03/12] send-email: Interpret --smtp-server "" as "use a default".

[Michael Witten]

On Sun, Apr 19, 2009 at 20:41, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> Why not? It's at least useful for testing.
>
> Why so?  "Use a default" as opposed to using what?
>

Use a default as opposed to using the nonsensical empty string.

> It is unclear what "a default" is in this context.  Do configured values
> count as "a default"?  I suspect not.
>
> I think you meant "allow overriding the configured values and use the
> default", but then you should spell what the defaults are (an available
> local MTA binary, or SMTP port on localhost, I think).  That is much more
> informative than your "Why not?..."

I see your irritation is derived from my carefree (careless?) commit
message. However, the patch doesn't do anything but trigger the
already present default-selecting code; there are already docs that
specify what those defaults are.

> In any case, I find it counterintuitive to trigger "use the default" with
> an option.  Something like "--ignore-config=smtp-server,smtp-port" might
> make sense, though.

I can get behind that kind of solution; however, I see no problem with
the shorthand --smtp-server "" either (other than the fact that it
requires an extra test in order to work).

Re: [PATCH RFC3.5 06/12] send-email: Cleanup and streamline the SMTP code in send_message

[Michael Witten]

On Sun, Apr 19, 2009 at 20:42, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
> ...
>> Now, the 'host:port' server URI form is handled regardless of the
>> documentation deficiencies of Net::SMTP{,::SSL}.
>
> You said that in 01/12, too but I do not think there is any problem with
> Perl documentation.
>
> My installed copy of /usr/share/perl/5.10.0/Net/SMTP.pm has this:
>
>    B<Host> - SMTP host to connect to. It may be a single scalar, as defined for
>    the C<PeerAddr> option in L<IO::Socket::INET>, or a reference to
>    an array with hosts to try in turn. The L</host> method will return the value
>    which was used to connect to the host.
>
> and of course PeerAddr allows host:port.
>
> Please stop placing a false blame on others.  I think it is you who lack
> ability to read the documentation correctly in this particular case.

Take a look again at my commit message for 01/12:

	The 'host:port' form of argument for --smtp-server was only
	working for SSL connections, because the SSL connection code
	was relying on undocumented behavior of Net::SMTP::SSL (really,
	undocumented behavior of Net::SMTP's new method)...

Clearly my beef is with Net::SMTP--->::SSL<---

I touch upon this in the commit message for 06/12:

	The code could even be simplified further, because Net::SMTP{,::SSL}
	both take the PORT variable in their new methods (which, as of this
	commit, are actually the same method). Moreover, both take a server
	URI of the form 'host:port' that trumps any value passed to PORT.

	Unfortunately, none of this is documented publicly, so it isn't
	exploited out of purity.

Net::SMTP doesn't document the PORT key, even though it's the one that
implements the constructor for both Net::SMTP and Net::SMTP::SSL. Also,
Net::SMTP:SSL doesn't document whether PORT shadows the ':port' in any
'host:port' input.

So... I remain resolved in my stance: The documentation is poor and
"--smtp-server host:port" only worked because the code relies on the
undocumented behavior of the ':port' taking precedence of over any
PORT specification. 

> I think you just did not read the documentation carefully enough in this
> case.

Actually, I think I read it too carefully and thought about it too much.

On Sun, Apr 19, 2009 at 21:38, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Please stop placing a false blame on others.  I think it is you who lack
>> ability to read the documentation correctly in this particular case.
>
> Sorry, this came out stronger than I intended.

That's OK. I tend to sound harsher than I want as well.

> Your ability has never been an issue (otherwise there wouldn't have been
> this patch series).

I appreciate that remark; I'm admittedly not important, but I hope that
I'm at least useful.

Now let's get back to work!

Sincerely,
Michael Witten

Re: [PATCH RFC3.5 04/12] send-email: Verification for --smtp-server and --smpt-server-port

[Michael Witten]

On Sun, Apr 19, 2009 at 20:42, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
> ...
>> Now, the 'host:port' server URI form is handled regardless of the
>> documentation deficiencies of Net::SMTP{,::SSL}.
>
> You said that in 01/12, too but I do not think there is any problem with
> Perl documentation.
>
> My installed copy of /usr/share/perl/5.10.0/Net/SMTP.pm has this:
>
>    B<Host> - SMTP host to connect to. It may be a single scalar, as defined for
>    the C<PeerAddr> option in L<IO::Socket::INET>, or a reference to
>    an array with hosts to try in turn. The L</host> method will return the value
>    which was used to connect to the host.
>
> and of course PeerAddr allows host:port.
>
> Please stop placing a false blame on others.  I think it is you who lack
> ability to read the documentation correctly in this particular case.

Take a look again at my commit message for 01/12:

	The 'host:port' form of argument for --smtp-server was only
	working for SSL connections, because the SSL connection code
	was relying on undocumented behavior of Net::SMTP::SSL (really,
	undocumented behavior of Net::SMTP's new method)...

Clearly my beef is with Net::SMTP--->::SSL<---

I touch upon this in the commit message for 06/12:

	The code could even be simplified further, because Net::SMTP{,::SSL}
	both take the PORT variable in their new methods (which, as of this
	commit, are actually the same method). Moreover, both take a server
	URI of the form 'host:port' that trumps any value passed to PORT.

	Unfortunately, none of this is documented publicly, so it isn't
	exploited out of purity.

Net::SMTP doesn't document the PORT key, even though it's the one that
implements the constructor for both Net::SMTP and Net::SMTP::SSL. Also,
Net::SMTP:SSL doesn't document whether PORT shadows the ':port' in any
'host:port' input.

So... I remain resolved in my stance: The documentation is poor and
"--smtp-server host:port" only worked because the code relies on the
undocumented behavior of the ':port' taking precedence of over any
PORT specification. 

> I think you just did not read the documentation carefully enough in this
> case.

Actually, I think I read it too carefully and thought about it too much.

On Sun, Apr 19, 2009 at 21:38, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Please stop placing a false blame on others.  I think it is you who lack
>> ability to read the documentation correctly in this particular case.
>
> Sorry, this came out stronger than I intended.

That's OK. I tend to sound harsher than I want as well.

> Your ability has never been an issue (otherwise there wouldn't have been
> this patch series).

I appreciate that remark; I'm admittedly not important, but I hope that
I'm at least useful.

Now let's get back to work!

Sincerely,
Michael Witten

Re: [PATCH RFC3.5 02/12] send-email: No longer repeatedly test if $smtp_server is a command

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> On Sun, Apr 19, 2009 at 20:41, Junio C Hamano <gitster@pobox.com> wrote:
>> Michael Witten <mfwitten@gmail.com> writes:
>>
>>> This is a minor change, but it's cleaner, and it sets up the
>>> $smtp_server initialization code for future improvements.
>>> ...
>>> -if (!defined $smtp_server) {
>>> +if (defined $smtp_server) {
>>> +
>>> +     $smtp_server_is_a_command = ($smtp_server =~ m{^/});
>>> +
>>> +} else { # use a default:
>>> +
>>>       foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
>>>               if (-x $_) {
>>>                       $smtp_server = $_;
>>> +                     $smtp_server_is_a_command = 1;
>>>                       last;
>>>               }
>>>       }
>>> -     $smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
>>> +
>>> +     $smtp_server = 'localhost'; # 127.0.0.1 is not compatible with IPv6
>>> +             unless $smtp_server_is_a_command;
> ...
>> I think a genuine improvement would be something like:
>>
>>        if (!defined $smtp_server) {
>>                $smtp_server = 'localhost';
>>        }
>
> You don't care to search for a possible sendmail?

That's something you already did before setting smtp_server
unconditionally to localhost, right?  You do (in the above):

	if (user gave $smtp_server) {
        	use it, notice and note if it is a command;
	} else {
                if (standard binary avaiable) {
                	use it, note it is a command;
		}
                # otherwise it still is undef
	}
	if (!defined $smtp_server) {
        	set it to localhost;
	}

But I would probably write it this way:

	if (user didn't give us $smtp_server) {
                if (standard binary avaiable) {
                	use it, note it is a command;
		} else {
                	use localhost;
		}
	}
	if ($smtp_server looks like a command) {
        	$smtp_server_is_a_command = true;
	}

Subject: Re: [PATCH RFC3.5 02/12] send-email: No longer repeatedly test if $smtp_server is a command

[Michael Witten]

On Sun, Apr 19, 2009 at 23:21, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> ...
>>> I think a genuine improvement would be something like:
>>>
>>> 	if (!defined $smtp_server) {
>>> 		$smtp_server = 'localhost';
>>> 	}
>>
>> You don't care to search for a possible sendmail?
>
> That's something you already did before setting smtp_server
> unconditionally to localhost, right?  You do (in the above):
>
> 	if (user gave $smtp_server) {
> 		use it, notice and note if it is a command;
> 	} else {
> 		if (standard binary avaiable) {
> 			use it, note it is a command;
> 		}
> 		# otherwise it still is undef
> 	}
> 	if (!defined $smtp_server) {
> 		set it to localhost;
> 	}
>
> But I would probably write it this way:
>
> 	if (user didn't give us $smtp_server) {
> 		if (standard binary avaiable) {
> 			use it, note it is a command;
> 		} else {
> 			use localhost;
> 		}
> 	}
> 	if ($smtp_server looks like a command) {
> 		$smtp_server_is_a_command = true;
> 	}
>
>

I suppose it's hard to tell from the patch, but it's actually
a combination of those two:

	if (user gave $smtp_server) {
		use it, notice and note if it is a command;
	} else { # use a default:
		if (standard binary avaiable) {
			use it, note it is a command;
		} else {
			use localhost;
			# automatically noted as a command
			# without doing anything (this would
			# cause warnings if $smtp_is_a_command
			# is used in places other than the bool
			# context, because it will be undef);
			# thus, my choice is a bad choice in
			# the long run, but I'm sticking to it.
		}
	}

The actual code:

	if (defined $smtp_server) {

		$smtp_server_is_a_command = ($smtp_server =~ m{^/});

	} else { # use a default:

		foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
			if (-x $_) {
				$smtp_server = $_;
				$smtp_server_is_a_command = 1;
				last;
			}
		}

		$smtp_server = 'localhost' # 127.0.0.1 is not compatible with IPv6
			unless $smtp_server_is_a_command;
	}

(I'll remove the "127.0.0.1" comment).

There's a minimum of checking and assigning; it's beautiful.
Plus, this organization fits in well with the server/port
verification (if I recall).

P.S.

I also like:

	$smtp_server_is_a_command or ($smtp_server = 'localhost');

or, maybe:

	$smtp_server_is_a_command or $smtp_server = 'localhost;

However, I wasn't sure if that is acceptable to others; more
importantly (:-D), I'm not sure that perl is smart enough to
optimize away the unnecessary comparison of the values, so I
figured that the modifier 'unless' is morally superior, because
it probably has the advantage of fewer cycles than the 'or' form,
and it has greater readability than the curly-braced conditional.
I absolutely loathe curly braces around a body of one line:

	if ($you_loath_this) {
		print "Clap Your Hands!\n";
	}

If only the curly-braces weren't there. I don't know why, they
just bug me terribly.

Re: [PATCH RFC3.5 06/12] send-email: Cleanup and streamline the SMTP code in send_message

[Michael Witten]

On Sun, Apr 19, 2009 at 20:42, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> +                             die "Server does not support STARTTLS: " . $smtp->message . "\n"
>> +                                     unless $smtp->code == 220;
>
> ...
> But the following, which is equivalent to what you did, is inexcuable.
>
>        do this;
>        do that;
>        do something unusual
>                if some condition that rarely holds true;
>        do some other thing;
>
> When your eyes and brain are coasting over this segment of code, your
> thought process needs to stumble and hiccup at the statment that does
> something unusual, and then need to realize that it is qualified with a
> statement modifier that says "this is only for rare case".

I mostly agree, and I frequently consider[ed] exactly those points.
However, there are 2 things that played a role in my decision:

    * For most conditional cases, I personally
      loathe curly braces around one statement.

    * The flow is actually:

            do this;
            do that;

            DIE "whisper some curses with the last breath"
                UNLESS some condition that holds mostly true;

            do some other thing;

      The "die" and thoughtful spacing should be pretty good clues.
      However, the "unless" can be strange to think with (at first);
      I figured Perlers would be happy with it.

In any case, I also like:

    condition and/or (do something);

or:

    condition and/or do something;

The only thing keeping me from using that more often is that I assume
other people would be less comfortable with it and that it may
introduce an unnecessary comparison of the return value of "do
something"; also, it might make the line a little long, which some
people get really angry about.

> Written without statement modifier:
>
>        do this;
>        do that;
>        if (some consition that rarely holds true) {
>                do something unusual
>        }
>        do some other thing;

I just have a hard time stomaching those curly braces. I really wish
perl didn't enforce them when there's only one statement. Also, I
would use some whitespace:

    do this;
    do that;

    if (some consition that rarely holds true) {
        do something unusual
    }

    do some other thing;

>> +             $smtp->mail($raw_from)               and
>> +             $smtp->to(@recipients)               and
>> +             $smtp->data                          and
>> +             $smtp->datasend("$header\n$message") and
>> +             $smtp->dataend                       or
>> +
>> +             die "Failed to send '$subject': " . $smtp->message . "\n";
>
> These do make things more pleasant to read.

Thanks!

P.S.

Sorry if the formatting of this email is bad; I'm in the middle of a
large move between systems, and currently I'm stuck with gmail's
webmail, which insists on reformatting my text and refusing to render
in fixed-width font (though I bet I could hack firefox's css to get
that one working.... hmmm.....), and firefox doesn't make it easy to
input tabs.

So, I've actually been writing and sending some emails with a combination of:

    * vim
    * date +'%a, %e %b %Y %T %z'
    * uuidgen (though I've found gmail makes a Message-ID for me)
    * cat path/to/email.txt | perl -pe 's/\n/\r\n/; END {print
"\r\n"}' | msmtp -t

This email was written in the webmail in firefox; I actually counted
spaces for indentation in the hope that things line up. ;-)

Re: [PATCH RFC3.5 06/12] send-email: Cleanup and streamline the SMTP code in send_message

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> In any case, I also like:
>
>     condition and/or (do something);
>
> or:
>
>     condition and/or do something;

I do not have problem with (condition and/or do this).

What I complained about was statement modifier that interrupts your
thought process and forces you to read things twice.  You see "DIE", and
you have to stop "Huh?".  You only realize that the author did not mean to
say "always die here" after seeing the modifier that changes the meaning
of "die" to "only under this rare condition--sorry, I did not say this
upfront, but I am checking for errors."

I just do not want to hear "sorry, I did not say this upfront" part.  IOW,
I have much less problem with this:

	do this;
	do that;
	condition that usually holds true
        	or die ...
	do some other thing;

Re: [PATCH RFC3.5.1 04/12] send-email: Verification for --smtp-server and --smpt-server-port

[Michael Witten]

On Sun, Apr 19, 2009 at 09:19, Michael Witten <mfwitten@gmail.com> wrote:
> +               ($smtp_server, my $port) = parse_server_URI $smtp_server
> +                       or die "--smtp-server: Not a valid server URI: '$smtp_server'\n";

Whoops! I used to have:

    my ($host, $port) = ...

but I tried to be clever and replaced it with the above.
Unfortunately, that means that an incorrect server 'URI' sets
$smtp_server to undef, which perl complains about when it interpolates
the die string.

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Jeff King]

On Sun, Apr 19, 2009 at 06:58:57PM -0700, Junio C Hamano wrote:

> >> +		(my $pid = open my $pipe, '|-')
> >> +			// die "Could not fork to run '$smtp_server': $!\n";
> >
> > Have I already rejected this "5.10 or later" construct in the previous
> > round?  If I haven't, please consider now I have.
> 
> Sorry, I should have checked myself.  defined-or "//" is 5.8.1 or later.

I don't think that's right. perl581delta lists it in the "Future
Directions" section and says

  A new operator "//" (defined-or) _will_ be available.

(emphasis mine). I don't think it was in a stable version until 5.10
(where it is listed as "now this is available" in perl5100delta.
Which puts it squarely in the "too new to use" list.

-Peff

Re: [PATCH RFC3.5.1 08/12] send-email: Simplify --compose subject sanitation

[Jeff King]

On Sun, Apr 19, 2009 at 11:43:41AM -0500, Michael Witten wrote:

>  		} elsif (/^Subject:\s*(.+)\s*$/i) {
> -			$initial_subject = $1;
> -			my $subject = $initial_subject;
> -			$_ = "Subject: " .
> -				($subject =~ /[^[:ascii:]]/ ?
> -				 quote_rfc2047($subject) :
> -				 $subject) .
> -				"\n";
> +			$initial_subject = $need_8bit_cte ? quote_rfc2047($1) : $1;
> +			next;

I don't think this is a good idea. need_8bit_cte is about the _whole_
message, including all headers, and this is just about the subject.
Which means that we end up rfc2047-encoding the subject unnecessarily
quite a bit (since at least in git itself, most of the time the
non-ascii bits are in people's names).

This makes the subject unnecessarily ugly for readers which don't do
rfc2047 decoding. And while I expect that most real MUAs these days
handle the decoding, it also makes life harder for people looking
directly at message, or doing "grep -i ^subject: foo.mbox". Yes, I know
that doesn't even remotely follow the standards (e.g., it won't handle
line-wrapped headers), but I don't see any need to make it worse.

All of that being said, even if we decided that it _is_ OK to quote
even when it wasn't unnecessary, your patch still isn't right.
need_8bit_cte is not "does this message need an 8-bit cte at all?" but
rather "does _we_ need to add an 8-bit cte?". A few lines above the ones
you changed, notice that when we see the message already has a
MIME-Version header, we turn set $need_8bit_cte to 0. But in that case,
we still may need to encode the subject if it has non-ascii characters.

-Peff

Re: [PATCH RFC3.5 05/12] send-email: Improve redability and error-handling in send_message's sendmail code

[Jeff King]

On Mon, Apr 20, 2009 at 10:00:21PM -0400, Jeff King wrote:

> I don't think that's right. perl581delta lists it in the "Future
> Directions" section and says
> 
>   A new operator "//" (defined-or) _will_ be available.

Bah, sorry for the noise. I didn't realize this was described with much
more detail in another thread (a pox on thread-breakers!):

  http://article.gmane.org/gmane.comp.version-control.git/117006

-Peff

Re: [PATCH RFC3.5.1 08/12] send-email: Simplify --compose subject sanitation

[Michael Witten]

On Mon, Apr 20, 2009 at 21:34, Jeff King <peff@peff.net> wrote:
> A few lines above the ones
> you changed, notice that when we see the message already has a
> MIME-Version header, we turn set $need_8bit_cte to 0. But in that case,
> we still may need to encode the subject if it has non-ascii characters.

I think this is the only problem with the patch, and I'm glad you
caught it. However, I can't make further comment until I read the RFC,
so I'm going to withdraw his patch from consideration. Thanks!