[PATCH RFC3 INTRO] I hope this will do it!

[PATCH RFC3 INTRO] I hope this will do it!

[Michael Witten]

Here is the list of notable improvements:

  send-email: --compose takes optional argument to existing file
    * Update docs about what editor is chosen.
    * Better variable name for --compose paths and filehandles.
    * Abort on empty summary.
    * Note (in commit message) that editing the final version no longer is used.

  send-email: Minor cleanup of $smtp_server usage and send_message()
    * Back to localhost.
    * Add note about empty string if it's not already there.

  send-email: Add --sleep for email throttling
    * Put docs in alphabetical order.

  send-email: Remove horrible mix of tabs and spaces
    * Less verbose whitespace comment.

  send-email: References: should only reference what is actually sent
    * s/of/if/ (comment above send_message()).

  send-email: Handle "GIT:" rather than "GIT: " during --compose
    * Undo refactoring of get_patch_subject().

  send-email: --smtp-server-port should take an integer
    * Update documents to suggest that symbolic ports can be used.
    * Put this commit with the docs.

  send-email: Cleanup the usage text and docs a bit
    * Add note about symbolic ports

  Docs: send-email: Remove superfluous information in CONFIGURATION
    * Remove comment block?

  Docs: send-email: Put options back into alphabetical order
    * Remove comment block?

The overall diffstat:

  Documentation/git-send-email.txt |  129 ++++++---
  git-send-email.perl              |  641 +++++++++++++++++++++++++-------------
  2 files changed, 511 insertions(+), 259 deletions(-)

The overall patch series:

    [PATCH RFC3 01/13] Docs: send-email: Put options back into alphabetical order
new [PATCH RFC3 02/13] Docs: send-email: Refer to CONFIGURATION section for sendemail.multiedit
    [PATCH RFC3 03/13] Docs: send-email: Remove superfluous information in CONFIGURATION
    [PATCH RFC3 04/13] Docs: send-email: --smtp-server-port can take symbolic ports
    [PATCH RFC3 05/13] send-email: Cleanup the usage text and docs a bit
    [PATCH RFC3 06/13] send-email: Handle "GIT:" rather than "GIT: " during --compose
    [PATCH RFC3 07/13] send-email: 'References:' should only reference what is sent
new [PATCH RFC3 08/13] send-email: Remove superfluous `my $editor = ...'
    [PATCH RFC3 09/13] send-email: Remove horrible mix of tabs and spaces
    [PATCH RFC3 10/13] send-email: Add --sleep for email throttling
    [PATCH RFC3 11/13] send-email: Minor cleanup of $smtp_server usage and send_message()
    [PATCH RFC3 12/13] send-email: --compose takes optional argument to existing file
new [PATCH RFC3 13/13] send-email: --compose always includes a 'GIT: ' prefixed list of patch subjects

Sincerely,
Michael Witten

[PATCH RFC3 01/13] Docs: send-email: Put options back into alphabetical order

[Michael Witten]

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 0b1f183..4db5a09 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -39,6 +39,11 @@ OPTIONS
 Composing
 ~~~~~~~~~
 
+--annotate::
+	Review each patch you're about to send in an editor. The setting
+	'sendemail.multiedit' defines if this will spawn one editor per patch
+	or one for all of them at once.
+
 --bcc=<address>::
 	Specify a "Bcc:" value for each email. Default is the value of
 	'sendemail.bcc'.
@@ -51,11 +56,6 @@ The --bcc option must be repeated for each user you want on the bcc list.
 +
 The --cc option must be repeated for each user you want on the cc list.
 
---annotate::
-	Review each patch you're about to send in an editor. The setting
-	'sendemail.multiedit' defines if this will spawn one editor per patch
-	or one for all of them at once.
-
 --compose::
 	Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an
 	introductory message for the patch series.
@@ -230,6 +230,12 @@ have been specified, in which case default to 'compose'.
 --dry-run::
 	Do everything except actually send the emails.
 
+--[no-]format-patch::
+	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.
+
 --quiet::
 	Make git-send-email less verbose.  One line per email should be
 	all that is output.
@@ -246,12 +252,6 @@ have been specified, in which case default to 'compose'.
 Default is the value of 'sendemail.validate'; if this is not set,
 default to '--validate'.
 
---[no-]format-patch::
-	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.
-
 
 CONFIGURATION
 -------------
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 02/13] Docs: send-email: Refer to CONFIGURATION section for sendemail.multiedit

[Michael Witten]

Replace description of sendemail.multiedit in --annotate docs
with a reference to the CONFIGURATION section.

Add such a reference to the --compose documentation.

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 4db5a09..7b87d6e 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -40,9 +40,8 @@ Composing
 ~~~~~~~~~
 
 --annotate::
-	Review each patch you're about to send in an editor. The setting
-	'sendemail.multiedit' defines if this will spawn one editor per patch
-	or one for all of them at once.
+	Review and edit each patch you're about to send. See the
+	CONFIGURATION section for 'sendemail.multiedit'.
 
 --bcc=<address>::
 	Specify a "Bcc:" value for each email. Default is the value of
@@ -67,6 +66,8 @@ In-Reply-To headers specified in the message. If the body of the message
 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'.
 
 --from=<address>::
 	Specify the sender of the emails.  This will default to
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 03/13] Docs: send-email: Remove superfluous information in CONFIGURATION

[Michael Witten]

In particular, sendemail.confirm was removed, because it's already
described along with its corresponding option.

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 7b87d6e..b58b433 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -271,12 +271,6 @@ sendemail.multiedit::
 	summary when '--compose' is used). If false, files will be edited one
 	after the other, spawning a new editor each time.
 
-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.
-
-
 Author
 ------
 Written by Ryan Anderson <ryan@michonline.com>
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 04/13] Docs: send-email: --smtp-server-port can take symbolic ports

[Michael Witten]

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b58b433..e8a385b 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -136,7 +136,9 @@ user is prompted for a password while the input is masked for privacy.
 --smtp-server-port=<port>::
 	Specifies a port different from the default port (SMTP
 	servers typically listen to smtp port 25 and ssmtp port
-	465). This can be set with 'sendemail.smtpserverport'.
+	465); symbolic port names are allowed. The port can also
+	be set with the 'sendemail.smtpserverport' configuration
+	variable.
 
 --smtp-ssl::
 	Legacy alias for '--smtp-encryption ssl'.
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 05/13] send-email: Cleanup the usage text and docs a bit

[Michael Witten]

All usage text lines should be < 80 characters.

A port number in --smtp-server is no longer handled,
so the suggestion has been removed.

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

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index e8a385b..5f7d640 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'.
@@ -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
@@ -166,11 +167,11 @@ Automating
 	entire patch series. Default is the value of the 'sendemail.chainreplyto'
 	configuration value; if that is unspecified, default to --chain-reply-to.
 
---identity=<identity>::
+--identity=<id>::
 	A configuration identity. When given, causes values in the
-	'sendemail.<identity>' subsection to take precedence over
+	'sendemail.<id>' 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 172b53c..5f01ad2 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          <host>  * Outgoing SMTP server.
+    --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               <id>  * Use the sendemail.<id> 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 06/13] send-email: Handle "GIT:" rather than "GIT: " during --compose

[Michael Witten]

This should make things a little more robust in terms of user input;
before, even the program got it wrong by outputting a line with only
"GIT:", which was left in place as a header, because there would be
no following space character.

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

diff --git a/git-send-email.perl b/git-send-email.perl
index 5f01ad2..5bd818e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -534,7 +534,7 @@ if ($compose) {
 
 	print C <<EOT;
 From $tpl_sender # This line is ignored.
-GIT: Lines beginning in "GIT: " will be removed.
+GIT: Lines beginning in "GIT:" will be removed.
 GIT: Consider including an overall diffstat or table of contents
 GIT: for the patch you are writing.
 GIT:
@@ -567,7 +567,7 @@ EOT
 	my $in_body = 0;
 	my $summary_empty = 1;
 	while(<C>) {
-		next if m/^GIT: /;
+		next if m/^GIT:/;
 		if ($in_body) {
 			$summary_empty = 0 unless (/^\n$/);
 		} elsif (/^\n$/) {
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 07/13] send-email: 'References:' should only reference what is sent

[Michael Witten]

If someone responded with a negative (n|no) to the confirmation,
then the Message-ID of the discarded email is no longer used
in the References: header of subsequent emails.

Consequently, send_message() now returns 1 if the message was
sent and 0 otherwise.

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

diff --git a/git-send-email.perl b/git-send-email.perl
index 5bd818e..2b0ff80 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -801,6 +801,10 @@ sub sanitize_address
 
 }
 
+# Returns 1 if the message was sent, and 0 otherwise.
+# In actuality, the whole program dies when a there
+# is an error sending a message.
+
 sub send_message
 {
 	my @recipients = unique_email_list(@to);
@@ -869,7 +873,7 @@ X-Mailer: git-send-email $gitversion
 		         default => $ask_default);
 		die "Send this email reply required" unless defined $_;
 		if (/^n/i) {
-			return;
+			return 0;
 		} elsif (/^q/i) {
 			cleanup_compose_files();
 			exit(0);
@@ -950,7 +954,7 @@ X-Mailer: git-send-email $gitversion
 		$smtp->data or die $smtp->message;
 		$smtp->datasend("$header\n$message") or die $smtp->message;
 		$smtp->dataend() or die $smtp->message;
-		$smtp->ok or die "Failed to send $subject\n".$smtp->message;
+		$smtp->code =~ /250|200/ or die "Failed to send $subject\n".$smtp->message;
 	}
 	if ($quiet) {
 		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
@@ -971,6 +975,8 @@ X-Mailer: git-send-email $gitversion
 			print "Result: OK\n";
 		}
 	}
+
+	return 1;
 }
 
 $reply_to = $initial_reply_to;
@@ -1131,10 +1137,10 @@ foreach my $t (@files) {
 
 	@cc = (@initial_cc, @cc);
 
-	send_message();
+	my $message_was_sent = send_message();
 
 	# set up for the next message
-	if ($chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
+	if ($message_was_sent and $chain_reply_to || not defined $reply_to || length($reply_to) == 0) {
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 08/13] send-email: Remove superfluous `my $editor = ...'

[Michael Witten]

Not only was it a repeat, but it also had no effect.

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

diff --git a/git-send-email.perl b/git-send-email.perl
index 2b0ff80..6b4b257 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -549,8 +549,6 @@ EOT
 	}
 	close(C);
 
-	my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
-
 	if ($annotate) {
 		do_edit($compose_filename, @files);
 	} else {
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 09/13] send-email: Remove horrible mix of tabs and spaces

[Michael Witten]

For the most part, I ran a search for all the lines
that match:

    ^[\t]*[ ]+

and then I manually replaced the offending text with
an appropriate number of tabs.

While scanning through the file, I also tried to format
some of the code so as to obviate future mixing; I also
fixed one horrendously egregious section of code, where
someone was trying to be unnecessarily compact.

Currently, no lines match the following:

    [\t]+[ ]+
    [ ]+[\t]+

So, it should be reasonably clean.

The whole file is still horrendous.

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

diff --git a/git-send-email.perl b/git-send-email.perl
index 6b4b257..0ff72f6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -16,6 +16,9 @@
 #    and second line is the subject of the message.
 #
 
+# Please do not mix tabs spaces across lines that share a relationship
+# in terms of layout. Currently, all indentation is expected to use tabs.
+
 use strict;
 use warnings;
 use Term::ReadLine;
@@ -116,19 +119,20 @@ sub format_2822_time {
 		die ("local time offset greater than or equal to 24 hours\n");
 	}
 
-	return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
-		       qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],
-		       $localtm[3],
-		       qw(Jan Feb Mar Apr May Jun
-			  Jul Aug Sep Oct Nov Dec)[$localtm[4]],
-		       $localtm[5]+1900,
-		       $localtm[2],
-		       $localtm[1],
-		       $localtm[0],
-		       ($offset >= 0) ? '+' : '-',
-		       abs($offhour),
-		       $offmin,
-		       );
+	return sprintf(
+		"%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
+		qw(Sun Mon Tue Wed Thu Fri Sat)[$localtm[6]],
+		$localtm[3],
+		qw(Jan Feb Mar Apr May Jun
+		Jul Aug Sep Oct Nov Dec)[$localtm[4]],
+		$localtm[5]+1900,
+		$localtm[2],
+		$localtm[1],
+		$localtm[0],
+		($offset >= 0) ? '+' : '-',
+		abs($offhour),
+		$offmin,
+	);
 }
 
 my $have_email_valid = eval { require Email::Valid; 1 };
@@ -192,29 +196,29 @@ my ($validate, $confirm);
 my (@suppress_cc);
 
 my %config_bool_settings = (
-    "thread" => [\$thread, 1],
-    "chainreplyto" => [\$chain_reply_to, 1],
-    "suppressfrom" => [\$suppress_from, undef],
-    "signedoffbycc" => [\$signed_off_by_cc, undef],
-    "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
-    "validate" => [\$validate, 1],
+	"thread" => [\$thread, 1],
+	"chainreplyto" => [\$chain_reply_to, 1],
+	"suppressfrom" => [\$suppress_from, undef],
+	"signedoffbycc" => [\$signed_off_by_cc, undef],
+	"signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
+	"validate" => [\$validate, 1],
 );
 
 my %config_settings = (
-    "smtpserver" => \$smtp_server,
-    "smtpserverport" => \$smtp_server_port,
-    "smtpuser" => \$smtp_authuser,
-    "smtppass" => \$smtp_authpass,
-    "to" => \@to,
-    "cc" => \@initial_cc,
-    "cccmd" => \$cc_cmd,
-    "aliasfiletype" => \$aliasfiletype,
-    "bcc" => \@bcclist,
-    "aliasesfile" => \@alias_files,
-    "suppresscc" => \@suppress_cc,
-    "envelopesender" => \$envelope_sender,
-    "multiedit" => \$multiedit,
-    "confirm"   => \$confirm,
+	"smtpserver" => \$smtp_server,
+	"smtpserverport" => \$smtp_server_port,
+	"smtpuser" => \$smtp_authuser,
+	"smtppass" => \$smtp_authpass,
+	"to" => \@to,
+	"cc" => \@initial_cc,
+	"cccmd" => \$cc_cmd,
+	"aliasfiletype" => \$aliasfiletype,
+	"bcc" => \@bcclist,
+	"aliasesfile" => \@alias_files,
+	"suppresscc" => \@suppress_cc,
+	"envelopesender" => \$envelope_sender,
+	"multiedit" => \$multiedit,
+	"confirm"   => \$confirm,
 );
 
 # Handle Uncouth Termination
@@ -245,37 +249,38 @@ $SIG{INT}  = \&signal_handler;
 # Begin by accumulating all the variables (defined above), that we will end up
 # needing, first, from the command line:
 
-my $rc = GetOptions("sender|from=s" => \$sender,
-                    "in-reply-to=s" => \$initial_reply_to,
-		    "subject=s" => \$initial_subject,
-		    "to=s" => \@to,
-		    "cc=s" => \@initial_cc,
-		    "bcc=s" => \@bcclist,
-		    "chain-reply-to!" => \$chain_reply_to,
-		    "smtp-server=s" => \$smtp_server,
-		    "smtp-server-port=s" => \$smtp_server_port,
-		    "smtp-user=s" => \$smtp_authuser,
-		    "smtp-pass:s" => \$smtp_authpass,
-		    "smtp-ssl" => sub { $smtp_encryption = 'ssl' },
-		    "smtp-encryption=s" => \$smtp_encryption,
-		    "identity=s" => \$identity,
-		    "annotate" => \$annotate,
-		    "compose" => \$compose,
-		    "quiet" => \$quiet,
-		    "cc-cmd=s" => \$cc_cmd,
-		    "suppress-from!" => \$suppress_from,
-		    "suppress-cc=s" => \@suppress_cc,
-		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
-		    "confirm=s" => \$confirm,
-		    "dry-run" => \$dry_run,
-		    "envelope-sender=s" => \$envelope_sender,
-		    "thread!" => \$thread,
-		    "validate!" => \$validate,
-		    "format-patch!" => \$format_patch,
-	 );
+my $rc = GetOptions(
+	"sender|from=s" => \$sender,
+	"in-reply-to=s" => \$initial_reply_to,
+	"subject=s" => \$initial_subject,
+	"to=s" => \@to,
+	"cc=s" => \@initial_cc,
+	"bcc=s" => \@bcclist,
+	"chain-reply-to!" => \$chain_reply_to,
+	"smtp-server=s" => \$smtp_server,
+	"smtp-server-port=s" => \$smtp_server_port,
+	"smtp-user=s" => \$smtp_authuser,
+	"smtp-pass:s" => \$smtp_authpass,
+	"smtp-ssl" => sub { $smtp_encryption = 'ssl' },
+	"smtp-encryption=s" => \$smtp_encryption,
+	"identity=s" => \$identity,
+	"annotate" => \$annotate,
+	"compose" => \$compose,
+	"quiet" => \$quiet,
+	"cc-cmd=s" => \$cc_cmd,
+	"suppress-from!" => \$suppress_from,
+	"suppress-cc=s" => \@suppress_cc,
+	"signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
+	"confirm=s" => \$confirm,
+	"dry-run" => \$dry_run,
+	"envelope-sender=s" => \$envelope_sender,
+	"thread!" => \$thread,
+	"validate!" => \$validate,
+	"format-patch!" => \$format_patch,
+);
 
 unless ($rc) {
-    usage();
+	usage();
 }
 
 die "Cannot run git format-patch from outside a repository\n"
@@ -404,29 +409,55 @@ sub split_addrs {
 my %aliases;
 my %parse_alias = (
 	# multiline formats can be supported in the future
-	mutt => sub { my $fh = shift; while (<$fh>) {
-		if (/^\s*alias\s+(\S+)\s+(.*)$/) {
-			my ($alias, $addr) = ($1, $2);
-			$addr =~ s/#.*$//; # mutt allows # comments
-			 # commas delimit multiple addresses
-			$aliases{$alias} = [ split_addrs($addr) ];
-		}}},
-	mailrc => sub { my $fh = shift; while (<$fh>) {
-		if (/^alias\s+(\S+)\s+(.*)$/) {
-			# spaces delimit multiple addresses
-			$aliases{$1} = [ split(/\s+/, $2) ];
-		}}},
-	pine => sub { my $fh = shift; my $f='\t[^\t]*';
-	        for (my $x = ''; defined($x); $x = $_) {
+	mutt => sub {
+
+		my $fh = shift;
+
+		while (<$fh>) {
+			if (/^\s*alias\s+(\S+)\s+(.*)$/) {
+				my ($alias, $addr) = ($1, $2);
+				$addr =~ s/#.*$//; # mutt allows # comments
+				# commas delimit multiple addresses
+				$aliases{$alias} = [ split_addrs($addr) ];
+			}
+		}
+	},
+
+	mailrc => sub {
+
+		my $fh = shift;
+
+		while (<$fh>) {
+			if (/^alias\s+(\S+)\s+(.*)$/) {
+				# spaces delimit multiple addresses
+				$aliases{$1} = [ split(/\s+/, $2) ];
+			}
+		}
+	},
+
+	pine => sub {
+
+		my $fh = shift;
+		my $f='\t[^\t]*';
+
+		for (my $x = ''; defined($x); $x = $_) {
 			chomp $x;
-		        $x .= $1 while(defined($_ = <$fh>) && /^ +(.*)$/);
+			$x .= $1 while(defined($_ = <$fh>) && /^ +(.*)$/);
 			$x =~ /^(\S+)$f\t\(?([^\t]+?)\)?(:?$f){0,2}$/ or next;
 			$aliases{$1} = [ split_addrs($2) ];
-		}},
-	gnus => sub { my $fh = shift; while (<$fh>) {
-		if (/\(define-mail-alias\s+"(\S+?)"\s+"(\S+?)"\)/) {
-			$aliases{$1} = [ $2 ];
-		}}}
+		}
+	},
+
+	gnus => sub {
+
+		my $fh = shift;
+
+		while (<$fh>) {
+			if (/\(define-mail-alias\s+"(\S+?)"\s+"(\S+?)"\)/) {
+				$aliases{$1} = [ $2 ];
+			}
+		}
+	}
 );
 
 if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {
@@ -571,10 +602,11 @@ EOT
 		} elsif (/^\n$/) {
 			$in_body = 1;
 			if ($need_8bit_cte) {
-				print C2 "MIME-Version: 1.0\n",
-					 "Content-Type: text/plain; ",
-					   "charset=utf-8\n",
-					 "Content-Transfer-Encoding: 8bit\n";
+				print C2
+					"MIME-Version: 1.0\n",
+					"Content-Type: text/plain; ",
+					"charset=utf-8\n",
+					"Content-Transfer-Encoding: 8bit\n";
 			}
 		} elsif (/^MIME-Version:/i) {
 			$need_8bit_cte = 0;
@@ -583,8 +615,8 @@ EOT
 			my $subject = $initial_subject;
 			$_ = "Subject: " .
 				($subject =~ /[^[:ascii:]]/ ?
-				 quote_rfc2047($subject) :
-				 $subject) .
+				quote_rfc2047($subject) :
+				$subject) .
 				"\n";
 		} elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
 			$initial_reply_to = $1;
@@ -616,8 +648,10 @@ sub ask {
 	my $resp;
 	my $i = 0;
 	return defined $default ? $default : undef
-		unless defined $term->IN and defined fileno($term->IN) and
-		       defined $term->OUT and defined fileno($term->OUT);
+		unless defined $term->IN
+			and defined fileno($term->IN)
+			and defined $term->OUT
+			and defined fileno($term->OUT);
 	while ($i++ < 10) {
 		$resp = $term->readline($prompt);
 		if (!defined $resp) { # EOF
@@ -637,8 +671,12 @@ sub ask {
 my $prompting = 0;
 if (!defined $sender) {
 	$sender = $repoauthor || $repocommitter || '';
-	$sender = ask("Who should the emails appear to be from? [$sender] ",
-	              default => $sender);
+
+	$sender = ask(
+		"Who should the emails appear to be from? [$sender] ",
+		default => $sender
+	);
+
 	print "Emails will be sent from: ", $sender, "\n";
 	$prompting++;
 }
@@ -666,7 +704,8 @@ sub expand_aliases {
 
 if ($thread && !defined $initial_reply_to && $prompting) {
 	$initial_reply_to = ask(
-		"Message-ID to be used as In-Reply-To for the first email? ");
+		"Message-ID to be used as In-Reply-To for the first email? "
+	);
 }
 if (defined $initial_reply_to) {
 	$initial_reply_to =~ s/^\s*<?//;
@@ -806,18 +845,22 @@ sub sanitize_address
 sub send_message
 {
 	my @recipients = unique_email_list(@to);
-	@cc = (grep { my $cc = extract_valid_address($_);
-		      not grep { $cc eq $_ } @recipients
-		    }
-	       map { sanitize_address($_) }
-	       @cc);
+
+	@cc = (grep
+		{
+			my $cc = extract_valid_address($_);
+			not grep { $cc eq $_ } @recipients
+		}
+		map { sanitize_address($_) } @cc
+	);
+
 	my $to = join (",\n\t", @recipients);
 	@recipients = unique_email_list(@recipients,@cc,@bcclist);
 	@recipients = (map { extract_valid_address($_) } @recipients);
 	my $date = format_2822_time($time++);
 	my $gitversion = '@@GIT_VERSION@@';
 	if ($gitversion =~ m/..GIT_VERSION../) {
-	    $gitversion = Git::version();
+		$gitversion = Git::version();
 	}
 
 	my $cc = join(", ", unique_email_list(@cc));
@@ -866,9 +909,13 @@ X-Mailer: git-send-email $gitversion
 			print "    To retain the current behavior, but squelch this message,\n";
 			print "    run 'git config --global sendemail.confirm auto'.\n\n";
 		}
-		$_ = ask("Send this email? ([y]es|[n]o|[q]uit|[a]ll): ",
-		         valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,
-		         default => $ask_default);
+
+		$_ = ask(
+			"Send this email? ([y]es|[n]o|[q]uit|[a]ll): ",
+			valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,
+			default => $ask_default
+		);
+
 		die "Send this email reply required" unless defined $_;
 		if (/^n/i) {
 			return 0;
@@ -903,9 +950,13 @@ X-Mailer: git-send-email $gitversion
 		}
 		else {
 			require Net::SMTP;
-			$smtp ||= Net::SMTP->new((defined $smtp_server_port)
-						 ? "$smtp_server:$smtp_server_port"
-						 : $smtp_server);
+
+			$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');
@@ -1002,7 +1053,7 @@ foreach my $t (@files) {
 			chomp($header[$#header]);
 			s/^\s+/ /;
 			$header[$#header] .= $_;
-	    } else {
+		} else {
 			push(@header, $_);
 		}
 	}
@@ -1120,9 +1171,9 @@ foreach my $t (@files) {
 			}
 			else {
 				push @xh,
-				  'MIME-Version: 1.0',
-				  "Content-Type: text/plain; charset=$author_encoding",
-				  'Content-Transfer-Encoding: 8bit';
+					'MIME-Version: 1.0',
+					"Content-Type: text/plain; charset=$author_encoding",
+					'Content-Transfer-Encoding: 8bit';
 			}
 		}
 	}
@@ -1130,7 +1181,9 @@ foreach my $t (@files) {
 	$needs_confirm = (
 		$confirm eq "always" or
 		($confirm =~ /^(?:auto|cc)$/ && @cc) or
-		($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));
+		($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1)
+	);
+
 	$needs_confirm = "inform" if ($needs_confirm && $confirm_unconfigured && @cc);
 
 	@cc = (@initial_cc, @cc);
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 10/13] send-email: Add --sleep for email throttling

[Michael Witten]

The --sleep option provides a means for specifying that there
should be a certain number of seconds of delay after sending
a certain number of emails; see Documentation/git-send-email.txt

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 5f7d640..236e578 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -178,6 +178,36 @@ Automating
 	cc list. Default is the value of 'sendemail.signedoffbycc' configuration
 	value; if that is unspecified, default to --signed-off-by-cc.
 
+--sleep=<seconds>[,<burst>]::
+	This option specfies that send-email should sleep for <seconds>
+	after sending <burst> messages as quickly as possible; <seconds>
+	should be an integer >= 0 and <burst> should be an integer >= 1.
+	This mode of operation attacks 2 problems: email throttling and
+	arrival disorder. Default is the value of the 'sendemail.sleep'
+	configuration variable, or '0' if that does not exist.
++
+By default, send-email tries to send one patch per email as quickly as
+possible. Unfortunately, some email services restrict a user by refusing
+to send more than some maximum number of email messages, M, in a given
+period of seconds, S. This can be troublesome if the patch series has
+more than M patches, because the server will ultimately refuse to send
+some of them. In this case, simply pass '--sleep=S,M' or '--sleep S,M'
+or set sendemail.sleep to 'S,M'.
++
+Moreover, the emails often arrive at the final destination out of order;
+though send-email manipulates the date fields and usually chains subsequent
+emails via the In-Reply-To headers, some mail viewers nevertheless insist
+on presenting them by order of arrival. This may be mitigated by using
+something like '--sleep 60' (the equivalent of '--sleep 60,1'), so that
+there is a 60 second delay between sending any two messages.
++
+*Note*: Because of varying routes and batching schemes, there is no delay
+that can guarantee the correct arrival order. Obviously, one solution is to
+choose an obscenely large number, so be prepared to run send-email in the
+background. Of course, spreading emails across time makes it more likely
+that unrelated email messages arrive between patches. Therefore, send-email
+warns you if both --sleep and --no-chain-reply-to are used.
+
 --suppress-cc=<category>::
 	Specify an additional category of recipients to suppress the
 	auto-cc of:
diff --git a/git-send-email.perl b/git-send-email.perl
index 0ff72f6..a394663 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -72,6 +72,7 @@ git send-email [options] <file | directory | rev-list options >
     --smtp-ssl                     * Deprecated. Use '--smtp-encryption ssl'.
 
   Automating:
+    --sleep      <secs>[,<burst>]  * Delay <secs> every <burst> email(s).
     --identity               <id>  * Use the sendemail.<id> options.
     --cc-cmd            <command>  * Email Cc: via `<command> $patch_path`
     --suppress-cc      <category>  * author, self, sob, cc, cccmd, body,
@@ -189,7 +190,7 @@ sub do_edit {
 }
 
 # Variables with corresponding config settings
-my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
+my ($sleep, $thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
 my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);
 my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
 my ($validate, $confirm);
@@ -205,6 +206,7 @@ my %config_bool_settings = (
 );
 
 my %config_settings = (
+	"sleep" => \$sleep,
 	"smtpserver" => \$smtp_server,
 	"smtpserverport" => \$smtp_server_port,
 	"smtpuser" => \$smtp_authuser,
@@ -257,6 +259,7 @@ my $rc = GetOptions(
 	"cc=s" => \@initial_cc,
 	"bcc=s" => \@bcclist,
 	"chain-reply-to!" => \$chain_reply_to,
+	"sleep=s" => \$sleep,
 	"smtp-server=s" => \$smtp_server,
 	"smtp-server-port=s" => \$smtp_server_port,
 	"smtp-user=s" => \$smtp_authuser,
@@ -329,6 +332,43 @@ foreach my $setting (values %config_bool_settings) {
 	${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]}));
 }
 
+#### Parse input
+
+my ($sleep_seconds, $sleep_burst);
+
+if (defined $sleep) {{
+
+	unless ($chain_reply_to) {
+
+		print "Both --sleep and --no-chain-reply-to are in effect.\n";
+		print "Therefore, it is much more likely that unrelated\n";
+		print "email messages will appear between any 2 of your\n";
+		print "patches.\n\n";
+
+		$_ = ask(
+			"How to proceed? ([q]uit | --[s]leep | --no-[c]hain | [n]either | [b]oth): ",
+			valid_re => qr/^(?:b|s|c|n|q)/i,
+			default => 'b'
+		);
+
+		/^b/                                               or
+		/^s/ and $chain_reply_to = 1                       or
+		/^c/ and $sleep = undef, last                      or
+		/^n/ and $chain_reply_to = 1, $sleep = undef, last or
+		/^q/ and exit;
+	}
+
+	$sleep =~ /^(\d+)(?:,(\d+))?$/
+		or  print "Should be '--sleep=<seconds>[,<burst>]', but got '--sleep=\"$sleep\"'\n"
+		and exit;
+
+	# Explicitly convert to integers to avoid repeated conversion:
+	# (<burst> = 0 is not defined, but let's be nice and absorb it)
+
+	$sleep_seconds = 0+$1;
+	$sleep_burst   = $2 ? 0+$2 : 1;
+}}
+
 # 'default' encryption is none -- this only prevents a warning
 $smtp_encryption = '' unless (defined $smtp_encryption);
 
@@ -1033,8 +1073,12 @@ $references = $initial_reply_to || '';
 $subject = $initial_subject;
 $message_num = 0;
 
-foreach my $t (@files) {
-	open(F,"<",$t) or die "can't open file $t";
+my $burst_count = $sleep_burst;
+my $time_of_last_message;
+
+for (my $index = 0; $index < @files; $index++) {
+	my $file = $files[$index];
+	open(F,"<",$file) or die "can't open file $file";
 
 	my $author = undef;
 	my $author_encoding;
@@ -1143,7 +1187,7 @@ foreach my $t (@files) {
 	close F;
 
 	if (defined $cc_cmd && !$suppress_cc{'cccmd'}) {
-		open(F, "$cc_cmd $t |")
+		open(F, "$cc_cmd $file |")
 			or die "(cc-cmd) Could not execute '$cc_cmd'";
 		while(<F>) {
 			my $c = $_;
@@ -1190,7 +1234,27 @@ foreach my $t (@files) {
 
 	my $message_was_sent = send_message();
 
-	# set up for the next message
+	# Throttle the outgoing rate:
+
+	if ($sleep_seconds && $message_was_sent) {
+
+		$time_of_last_message = time;
+
+		unless (--$burst_count) {  # unless we can send more
+
+			$burst_count = $sleep_burst;
+
+			my $already_elapsed = time - $time_of_last_message;
+
+			if ($already_elapsed < $sleep_seconds && $index < $#files) {
+				my $this_long = $sleep_seconds - $already_elapsed;
+				while (($this_long -= sleep $this_long) > 0) {}
+			}
+		}
+	}
+
+	# set up for the next message:
+
 	if ($message_was_sent and $chain_reply_to || not defined $reply_to || length($reply_to) == 0) {
 		$reply_to = $message_id;
 		if (length $references > 0) {
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 11/13] send-email: Minor cleanup of $smtp_server usage and send_message()

[Michael Witten]

Note that the output has been changed; the server return code
now appears after the `(Sendmail|Server):' line, rather than
after the headers.

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 236e578..93cfb34 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -130,9 +130,11 @@ user is prompted for a password while the input is masked for privacy.
 	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
-	`localhost` otherwise.
+	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> 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 a394663..8ce9d3b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -191,7 +191,8 @@ sub do_edit {
 
 # Variables with corresponding config settings
 my ($sleep, $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);
@@ -753,14 +754,20 @@ 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{^/});
+} else {
+
 	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
+	$smtp_server_is_a_command = 0;
 }
 
 if ($compose && $compose > 0) {
@@ -969,7 +976,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) {
@@ -1045,24 +1052,28 @@ X-Mailer: git-send-email $gitversion
 		$smtp->dataend() or die $smtp->message;
 		$smtp->code =~ /250|200/ or die "Failed to send $subject\n".$smtp->message;
 	}
+
+	print 'Dry-' if $dry_run;
+
 	if ($quiet) {
-		printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
+		print "Sent $subject\n";
 	} else {
-		print (($dry_run ? "Dry-" : "")."OK. Log says:\n");
-		if ($smtp_server !~ m#^/#) {
+		print "OK. Log says:\n";
+
+		if ($smtp_server_is_a_command) {
+			print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
+			print "Result: OK\n";
+		} else {
 			print "Server: $smtp_server\n";
+
+			$dry_run and print "Result: OK\n" or
+			print "Result: ", $smtp->code, ' ', ($smtp->message =~ /\n([^\n]+\n)$/s), "\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";
-		}
 	}
 
 	return 1;
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 12/13] send-email: --compose takes optional argument to existing file

[Michael Witten]

Now, a user may specify an existing (in-progress) file to use as
the introductory/summary email.

The file is opened for any additional editing as usual, but it
is not deleted upon normal termination.

There are also a number of fixes to how the internals and
temporaries are handled.

Also, it is no longer possible to pass --compose multiple times
in order to edit the transformed version as well.

Also, send-email now aborts upon discovering an empty message.

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 93cfb34..5ef8b12 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -55,17 +55,39 @@ The --bcc option must be repeated for each user you want on the bcc list.
 +
 The --cc option must be repeated for each user you want on the cc list.
 
---compose::
-	Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an
-	introductory message for the patch series.
+--compose[=<path>]::
+	Create an introductory message for the patch series; the
+	message is opened for editing with (in order of preference)
+	`$GIT_EDITOR`, 'core.editor', `$VISUAL`, `$EDITOR`, or `vi`.
 +
-When '--compose' is used, git 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,
-and In-Reply-To headers will be used unless they are removed.
+A path for the composition may be given. If the path exists, then
+the file at the path is opened for editing as-is. If the path
+doesn't exist, a file with default content is created at the path
+and opened for editing. If no path is specified, a new temporary
+file with default content is created and opened for editing.
 +
-Missing From or In-Reply-To headers will be prompted for.
+Upon the successful completion of send-email, all temporary files
+are automatically unlinked (deleted). However, if send-email is
+terminated by a trappable signal, then this temporary file is not
+unlinked, and the user is informed of its path.
++
+The user actually composes what will become a valid email message;
+therefore, the message must have the following form (as described in
+`RFC 822`):
++
+	<headers>
+	<blank line>
+	<body>
++
+In particular, `<headers>` must contain the "`Subject`" header. Once
+the user saves the message and quits the editor, this intermediate
+message is transformed into the final email message by removing all
+lines that begin with "`GIT:`". If the `<body>` of the final version
+is empty, then send-email aborts without sending anything.
++
+The "`From`", "`Subject`", and "`In-Reply-To`" headers are taken
+directly from the message; missing "`From`" or "`In-Reply-To`"
+headers will be prompted for.
 +
 See the CONFIGURATION section for 'sendemail.multiedit'.
 
diff --git a/git-send-email.perl b/git-send-email.perl
index 8ce9d3b..2ab76c6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -58,10 +58,9 @@ git send-email [options] <file | directory | rev-list options >
     --bcc               <address>  * Email Bcc:
     --subject            <string>  * Email "Subject:"
     --in-reply-to    <message-id>  * Email "In-Reply-To:"; include '<' and '>'.
+    --compose             [<path>] * Open an editor for introduction.
     --annotate                     * Review each patch that will be sent in
                                      an editor.
-    --compose                      * Open an editor for introduction.
-
   Sending:
     --envelope-sender   <address>  * Email envelope sender.
     --smtp-server          <host>  * Outgoing SMTP server.
@@ -168,7 +167,7 @@ if ($@) {
 # Behavior modification variables
 my ($quiet, $dry_run) = (0, 0);
 my $format_patch;
-my $compose_filename;
+my ($compose_path, $compose_final_path);
 
 # Handle interactive edition of files.
 my $multiedit;
@@ -234,16 +233,15 @@ sub signal_handler {
 	system "stty echo";
 
 	# tmp files from --compose
-	if (defined $compose_filename) {
-		if (-e $compose_filename) {
-			print "'$compose_filename' contains an intermediate version of the email you were composing.\n";
-		}
-		if (-e ($compose_filename . ".final")) {
-			print "'$compose_filename.final' contains the composed email.\n"
-		}
+	if (defined $compose_path and -f $compose_path) {
+		print "'$compose_path' contains an intermediate version of the email you were composing.\n";
+	}
+
+	if (defined $compose_final_path) {
+		unlink $compose_final_path if defined $compose_final_path;
 	}
 
-	exit;
+	exit 1;
 };
 
 $SIG{TERM} = \&signal_handler;
@@ -268,8 +266,8 @@ my $rc = GetOptions(
 	"smtp-ssl" => sub { $smtp_encryption = 'ssl' },
 	"smtp-encryption=s" => \$smtp_encryption,
 	"identity=s" => \$identity,
+	"compose:s" => \$compose,
 	"annotate" => \$annotate,
-	"compose" => \$compose,
 	"quiet" => \$quiet,
 	"cc-cmd=s" => \$cc_cmd,
 	"suppress-from!" => \$suppress_from,
@@ -590,60 +588,108 @@ sub get_patch_subject($) {
 	die "No subject line in $fn ?";
 }
 
-if ($compose) {
-	# Note that this does not need to be secure, but we will make a small
-	# effort to have it be unique
-	$compose_filename = ($repo ?
-		tempfile(".gitsendemail.msg.XXXXXX", DIR => $repo->repo_path()) :
-		tempfile(".gitsendemail.msg.XXXXXX", DIR => "."))[1];
-	open(C,">",$compose_filename)
-		or die "Failed to open for writing $compose_filename: $!";
-
-
-	my $tpl_sender = $sender || $repoauthor || $repocommitter || '';
-	my $tpl_subject = $initial_subject || '';
-	my $tpl_reply_to = $initial_reply_to || '';
-
-	print C <<EOT;
-From $tpl_sender # This line is ignored.
-GIT: Lines beginning in "GIT:" will be removed.
-GIT: Consider including an overall diffstat or table of contents
-GIT: for the patch you are writing.
-GIT:
-GIT: Clear the body content if you don't wish to send a summary.
-From: $tpl_sender
-Subject: $tpl_subject
-In-Reply-To: $tpl_reply_to
+if (defined $compose) {
 
-EOT
-	for my $f (@files) {
-		print C get_patch_subject($f);
+	# Some parameters for creating temporary files:
+
+	my $template = ".gitsendemail.msg.XXXXXX";
+	my @dir = (DIR => ($repo ? $repo->repo_path() : "."));
+	my @suffix = (SUFFIX => ".final");
+
+	# Figure out the file that the user should be editing:
+
+	my $compose_path_filehandle;
+
+	if ($compose eq '') { # if no path was given
+		($compose_path_filehandle, $compose_path) = tempfile($template, @dir)
+			or die "--compose: Could not create temporary file for the user to edit: $!\n";
+	} else {
+		$compose_path = $compose;
+		unless (-f $compose_path) {
+			open $compose_path_filehandle, ">", $compose_path
+				or die "--compose: Could not open '$compose_path' for writing default content: $!\n";
+		}
 	}
-	close(C);
+
+	# Fill in default content if necessary:
+
+	if ($compose_path_filehandle) {
+
+		# For convenience:
+
+		local *STDOUT = $compose_path_filehandle;
+
+		# Help the user out with some instruction and initial headers:
+
+		my $from = $sender || $repoauthor || $repocommitter || '';
+		my $subject = $initial_subject || '';
+		my $reply_to = $initial_reply_to || '';
+
+		print "From $from # This line is ignored.\n";
+		print "GIT:\n";
+		print "GIT: Lines beginning in 'GIT:' will be removed.\n";
+		print "GIT:\n";
+		print "GIT: Consider including an overall diffstat\n";
+		print "GIT: (git diff --stat) or table of contents\n";
+		print "GIT: (as provide below).\n";
+		print "GIT:\n";
+		print "GIT: Clear the body content if you decide not\n";
+		print "GIT: to send this message.\n";
+		print "GIT:\n";
+		print "GIT: Here are the <headers>:\n";
+		print "From: $from\n";
+		print "Subject: $subject\n";
+		print "In-Reply-To: $reply_to\n";
+		print "\n";
+		print "GIT: This is the first line of the <body>:\n";
+		print "\n";
+
+		for my $f (@files) {
+			print get_patch_subject($f);
+		}
+	}
+
+	# Do the editing:
 
 	if ($annotate) {
-		do_edit($compose_filename, @files);
+		do_edit($compose_path, @files);
 	} else {
-		do_edit($compose_filename);
+		do_edit($compose_path);
 	}
 
-	open(C2,">",$compose_filename . ".final")
-		or die "Failed to open $compose_filename.final : " . $!;
+	# Now transform the user-edited introduction into something
+	# suitable for sending via email; the user's editor may have
+	# unlinked the original file and replaced it with an entirely
+	# new one. If this be the case, then it wouldn't do just to seek
+	# to the beginning and start reading, because then only the
+	# original content would be retrieved. Consequently, the file
+	# must be reopened to be safe (note, the original filehandle is
+	# closed automatically):
+
+	unless (-f $compose_path) {
+		die "--compose: File '$compose_path' doesn't exist; aborting.\n";
+	}
+
+	open $compose_path_filehandle, "<", $compose_path
+		or die "--compose: Failed to open '$compose_path' for reading: $!";
 
-	open(C,"<",$compose_filename)
-		or die "Failed to open $compose_filename : " . $!;
+	# Create the final version:
 
-	my $need_8bit_cte = file_has_nonascii($compose_filename);
+	(my $compose_final_file, $compose_final_path) = tempfile($template, @dir, @suffix)
+		or die "--compose: Could not create temporary file for final version: $!\n";
+
+	my ($subject, $reply_to, $from);
+	my $need_8bit_cte = file_has_nonascii($compose_path);
 	my $in_body = 0;
 	my $summary_empty = 1;
-	while(<C>) {
+	while(<$compose_path_filehandle>) {
 		next if m/^GIT:/;
 		if ($in_body) {
 			$summary_empty = 0 unless (/^\n$/);
 		} elsif (/^\n$/) {
 			$in_body = 1;
 			if ($need_8bit_cte) {
-				print C2
+				print $compose_final_file
 					"MIME-Version: 1.0\n",
 					"Content-Type: text/plain; ",
 					"charset=utf-8\n",
@@ -653,12 +699,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;
@@ -669,15 +710,14 @@ EOT
 			print "To/Cc/Bcc fields are not interpreted yet, they have been ignored\n";
 			next;
 		}
-		print C2 $_;
-	}
-	close(C);
-	close(C2);
 
-	if ($summary_empty) {
-		print "Summary email is empty, skipping it\n";
-		$compose = -1;
+		print $compose_final_file $_;
 	}
+
+	$summary_empty and die "Introductory email is empty; aborting.\n";
+
+	unshift(@files, $compose_final_path);
+
 } elsif ($annotate) {
 	do_edit(@files);
 }
@@ -770,10 +810,6 @@ if (defined $smtp_server && $smtp_server ne '') {
 	$smtp_server_is_a_command = 0;
 }
 
-if ($compose && $compose > 0) {
-	@files = ($compose_filename . ".final", @files);
-}
-
 # Variables we set as part of the loop over files
 our ($message_id, %mail, $subject, $reply_to, $references, $message,
 	$needs_confirm, $message_num, $ask_default);
@@ -918,9 +954,12 @@ sub send_message
 	my $sanitized_sender = sanitize_address($sender);
 	make_message_id() unless defined($message_id);
 
+	my $has_non_ascii = ($subject =~ /[^[:ascii:]]/);
+	my $sanitized_subject = $has_non_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
@@ -1280,7 +1319,14 @@ for (my $index = 0; $index < @files; $index++) {
 cleanup_compose_files();
 
 sub cleanup_compose_files() {
-	unlink($compose_filename, $compose_filename . ".final") if $compose;
+
+	if (defined $compose_final_path) {
+		unlink $compose_final_path;
+	}
+
+	if (defined $compose_path and not $compose) {
+		unlink $compose_path;
+	}
 }
 
 $smtp->quit if $smtp;
-- 
1.6.2.2.479.g2aec

[PATCH RFC3 13/13] send-email: --compose always includes a 'GIT: ' prefixed list of patch subjects

[Michael Witten]

I've found that having a list of the patch subjects appended to
the end of an existing --compose email is quite useful.

I've tried to make appending smart enough not to alter existing
lines beyond the addition of some kind of newline.

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 5ef8b12..6edef52 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -61,7 +61,8 @@ The --cc option must be repeated for each user you want on the cc list.
 	`$GIT_EDITOR`, 'core.editor', `$VISUAL`, `$EDITOR`, or `vi`.
 +
 A path for the composition may be given. If the path exists, then
-the file at the path is opened for editing as-is. If the path
+the file at the path is opened for editing with the list of patch
+subjects (prefixed with 'GIT: ') appended to the end. If the path
 doesn't exist, a file with default content is created at the path
 and opened for editing. If no path is specified, a new temporary
 file with default content is created and opened for editing.
diff --git a/git-send-email.perl b/git-send-email.perl
index 2ab76c6..696b132 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -644,9 +644,25 @@ if (defined $compose) {
 		print "GIT: This is the first line of the <body>:\n";
 		print "\n";
 
-		for my $f (@files) {
-			print get_patch_subject($f);
+		print get_patch_subject($_) for (@files);
+
+	} else {
+
+		# Supply a listing of the patch subjects, in case
+		# it might be useful to the user:
+
+		open $compose_path_filehandle, "+<", $compose_path
+			or die "--compose: Could not open '$compose_path' for appending listing: $!\n";
+
+		# Figure out whether we should begin a new line
+		# (we don't want to append to an existing line):
+
+		if (seek $compose_path_filehandle, -1, 2) {
+			read $compose_path_filehandle, my $c, 1;
+			$c !~ /\n|\r/ and print $compose_path_filehandle "\n";
 		}
+
+		print $compose_path_filehandle get_patch_subject($_) for (@files);
 	}
 
 	# Do the editing:
-- 
1.6.2.2.479.g2aec

Re: [PATCH RFC3 INTRO] I hope this will do it!

[Michael Witten]

On Mon, Apr 13, 2009 at 13:23, Michael Witten <mfwitten@gmail.com> wrote:
>  Docs: send-email: Remove superfluous information in CONFIGURATION
>    * Remove comment block?
>
>  Docs: send-email: Put options back into alphabetical order
>    * Remove comment block?

s/?//

>    [PATCH RFC3 12/13] send-email: --compose takes optional argument to existing file

This would be better: "send-email: --compose takes optional path"

Re: [PATCH RFC3 03/13] Docs: send-email: Remove superfluous information in CONFIGURATION

[Junio C Hamano]

I
Michael Witten <mfwitten@gmail.com> writes:

> n particular, sendemail.confirm was removed, because it's already
> described along with its corresponding option.
>
> Signed-off-by: Michael Witten <mfwitten@gmail.com>
> ---
>  Documentation/git-send-email.txt |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 7b87d6e..b58b433 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -271,12 +271,6 @@ sendemail.multiedit::
>  	summary when '--compose' is used). If false, files will be edited one
>  	after the other, spawning a new editor each time.
>  
> -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.

I am not sure if this is a good idea.  It may look "superfluous" for
people who read the manual cover-to-cover.  However, a new person whose
mentor allowed her to peek into his configuration to learn tricks from may
find sendemail.confirm and would try to find an entry in the manual.  The
change in this patch will make it inconvenient to go from variable to
description.

Re: [PATCH RFC3 05/13] send-email: Cleanup the usage text and docs a bit

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> All usage text lines should be < 80 characters.
>
> A port number in --smtp-server is no longer handled,
> so the suggestion has been removed.

That makes it sound like there is a regression.

	require Net::SMTP;
	$smtp ||= Net::SMTP->new((defined $smtp_server_port)
				 ? "$smtp_server:$smtp_server_port"
				 : $smtp_server);

and because Net::SMTP is a subclass of IO::Socket::INET, I'd assume that
this use will accept smtp_server=there:submission when $smtp_server_port
is undef.

> ---in-reply-to=<identifier>::
> +--in-reply-to=<message-id>::

Changes along this line in your patch looked sensible, except for the
"identity" one which is a bit iffy.

Other than that I think the patch is sane.

Re: [PATCH RFC3 09/13] send-email: Remove horrible mix of tabs and spaces

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> Currently, no lines match the following:
>
>     [\t]+[ ]+
>     [ ]+[\t]+

I understand the latter but what's wrong with the former?  The width of a
HT is by definition 8 columns throughout the git codebase.

Re: [PATCH RFC3 03/13] Docs: send-email: Remove superfluous information in CONFIGURATION

[Michael Witten]

On Mon, Apr 13, 2009 at 15:45, Junio C Hamano <gitster@pobox.com> wrote:
> I am not sure if this is a good idea.  It may look "superfluous" for
> people who read the manual cover-to-cover.

People don't do this? How else are you supposed to know what's going on?

> However, a new person whose
> mentor allowed her to peek into his configuration to learn tricks from may
> find sendemail.confirm and would try to find an entry in the manual.  The
> change in this patch will make it inconvenient to go from variable to
> description.

Well, couldn't she search? I doubt she'd try to look in the CONFIGURATION
section anyway. I certainly wouldn't know about it without more experience.

Anyway, it's actually how this man page has been organized for a while; you'll
notice that the CONFIGURATION section is pretty empty.

Perhaps the CONFIGURATION section should at least reference the associated
command line argument, where a description can be found.

Re: [PATCH RFC3 05/13] send-email: Cleanup the usage text and docs a bit

[Michael Witten]

On Mon, Apr 13, 2009 at 15:51, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> All usage text lines should be < 80 characters.
>>
>> A port number in --smtp-server is no longer handled,
>> so the suggestion has been removed.
>
> That makes it sound like there is a regression.
>
>        require Net::SMTP;
>        $smtp ||= Net::SMTP->new((defined $smtp_server_port)
>                                 ? "$smtp_server:$smtp_server_port"
>                                 : $smtp_server);
>
> and because Net::SMTP is a subclass of IO::Socket::INET, I'd assume that
> this use will accept smtp_server=there:submission when $smtp_server_port
> is undef.

I may have been concerned that the SSL connection code doesn't support
a host:port specification, so I thought it would be easier not to
advertise it:

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


>> ---in-reply-to=<identifier>::
>> +--in-reply-to=<message-id>::
>
> Changes along this line in your patch looked sensible, except for the
> "identity" one which is a bit iffy.

The change to <id> ? Well, I did so, because the usage text uses <id>
for compactness; I think everything else matches.

Re: [PATCH RFC3 09/13] send-email: Remove horrible mix of tabs and spaces

[Michael Witten]

On Mon, Apr 13, 2009 at 15:55, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> Currently, no lines match the following:
>>
>>     [\t]+[ ]+
>>     [ ]+[\t]+
>
> I understand the latter but what's wrong with the former?  The width of a
> HT is by definition 8 columns throughout the git codebase.

Ah, well, the width of a HT has been a free variable in my
calculations; I was operating under the assumption that whitespace
used for indentation can float freely according to the user's
settings. A few of the lines were aligning function arguments via tabs
and a few extra spaces, which is not reliable in my model.

Frankly, I don't like tabs and spaces sharing the same contiguous
block. I don't like it all. ;-B

Re: [PATCH RFC3 08/13] send-email: Remove superfluous `my $editor = ...'

[Stephen Boyd]

Michael Witten wrote:
> -	my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";

I don't want to start an editor war, but why is 'vi' here? It seems that one of the previous four should be set at all times, correct?

Re: [PATCH RFC3 08/13] send-email: Remove superfluous `my $editor = ...'

[Michael Witten]

On Mon, Apr 13, 2009 at 18:39, Stephen Boyd <bebarino@gmail.com> wrote:
> I don't want to start an editor war, but why is 'vi' here?

The program vi is on every single Unix installation. Take that as an axiom.

Re: [PATCH RFC3 08/13] send-email: Remove superfluous `my $editor = ...'

[Michael Witten]

On Mon, Apr 13, 2009 at 19:41, Michael Witten <mfwitten@gmail.com> wrote:
> On Mon, Apr 13, 2009 at 18:39, Stephen Boyd <bebarino@gmail.com> wrote:
>> I don't want to start an editor war, but why is 'vi' here?
>
> The program vi is on every single Unix installation. Take that as an axiom.

However, I would be OK with confronting the user about setting one of
the variables.

Re: [PATCH RFC3 09/13] send-email: Remove horrible mix of tabs and spaces

[Andreas Ericsson]

Michael Witten wrote:
> On Mon, Apr 13, 2009 at 15:55, Junio C Hamano <gitster@pobox.com> wrote:
>> Michael Witten <mfwitten@gmail.com> writes:
>>
>>> Currently, no lines match the following:
>>>
>>>     [\t]+[ ]+
>>>     [ ]+[\t]+
>> I understand the latter but what's wrong with the former?  The width of a
>> HT is by definition 8 columns throughout the git codebase.
> 
> Ah, well, the width of a HT has been a free variable in my
> calculations; I was operating under the assumption that whitespace
> used for indentation can float freely according to the user's
> settings. A few of the lines were aligning function arguments via tabs
> and a few extra spaces, which is not reliable in my model.
> 
> Frankly, I don't like tabs and spaces sharing the same contiguous
> block. I don't like it all. ;-B


Using tabs to align stuff to indentation level and spaces to align
line continuation is the only possible way to let users choose
whichever indentation depth they want while preserving the continuation
alignment. What's not to like about that? Especially if you think a
horizontal tab can be any size at all, you should be all agog.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

Re: [PATCH RFC3 05/13] send-email: Cleanup the usage text and docs a bit

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

>>> A port number in --smtp-server is no longer handled,
>>> so the suggestion has been removed.
> ...
> I may have been concerned that the SSL connection code doesn't support
> a host:port specification, so I thought it would be easier not to
> advertise it:

These two statements contradict with each other.  Please make up your mind
;-).

Re: [PATCH RFC3 05/13] send-email: Cleanup the usage text and docs a bit

[Michael Witten]

On Tue, Apr 14, 2009 at 00:39, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>>>> A port number in --smtp-server is no longer handled,
>>>> so the suggestion has been removed.
>> ...
>> I may have been concerned that the SSL connection code doesn't support
>> a host:port specification, so I thought it would be easier not to
>> advertise it:
>
> These two statements contradict with each other.  Please make up your mind
> ;-).

I'm confused.

The usage text for --smtp-server currently has <host:port>, which
doesn't work when 'ssl' is used for the connection to the server.
However, the other forms of connection DO work when a <host:port>
address is given.

I set out to add <host:port> support for SSL, but then decided that
people should just use the --smtp-server-port input. It already works
for every connection method and why else have it? However, rather than
remove ALL support for <host:port>, I just changed <host:port> to
<host> in the usage text for --smtp-server.

So, effectively, I've deprecated <host:port> by changing the
documentation, but the code hasn't changed. Would you rather support
<host:port> for everything?

Re: [PATCH RFC3 08/13] send-email: Remove superfluous `my $editor = ...'

[Björn Steinbrink]

On 2009.04.13 16:39:52 -0700, Stephen Boyd wrote:
> Michael Witten wrote:
>> -	my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
>
> I don't want to start an editor war, but why is 'vi' here? It seems
> that one of the previous four should be set at all times, correct?

Probably because that's how the rest of git works, too. See core.editor
in git-config(1) or launch_editor() in editor.c if you don't trust the
docs.

Oh, and I seem to rely on that behaviour :-)

Björn

Re: [PATCH RFC3 09/13] send-email: Remove horrible mix of tabs and spaces

[Junio C Hamano]

Andreas Ericsson <ae@op5.se> writes:

> Using tabs to align stuff to indentation level and spaces to align
> line continuation is the only possible way to let users choose
> whichever indentation depth they want while preserving the continuation
> alignment.

Sure, it will look aligned with whatever width of HT ">":

	>>if (this is a looong
        >>....expression that is alighed) {
        >>>statement1;
        >>>statement2;
        >>}

but it is *only true* if your SP "." and everything else is of the same
width.

People seem to repeat that without realizing what they are saying, but I
find the assumption the argument is based on quite bogus.  Why do people
think it is only sane to assume flexible HT width but still monospaced
font whose SP, l and w are all of the same width?

You either forget about the alignment (there is no such thing---suck it
up), or use time honored HT=8 and monospace convention.

And this project uses the latter.

Re: [PATCH RFC3 05/13] send-email: Cleanup the usage text and docs a bit

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> The usage text for --smtp-server currently has <host:port>, which
> doesn't work when 'ssl' is used for the connection to the server.

Ah, I see.  But it does work when ssl is not used, right?  A possible
solution would be one of:

 - support host:port in SSL codepath (shouldn't it be trivial?);

 - split the description in the documentation to clarify it does not work
   for SSL; or

 - remove host:port support to make both consistent.

To me, the last one makes the least sense.  Is that the approach you are
taking?

Re: [PATCH RFC3 09/13] send-email: Remove horrible mix of tabs and spaces

[Michael Witten]

On Tue, Apr 14, 2009 at 00:31, Andreas Ericsson <ae@op5.se> wrote:
> Michael Witten wrote:
>>
>> On Mon, Apr 13, 2009 at 15:55, Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> Michael Witten <mfwitten@gmail.com> writes:
>>>
>>>> Currently, no lines match the following:
>>>>
>>>>    [\t]+[ ]+
>>>>    [ ]+[\t]+
>>>
>>> I understand the latter but what's wrong with the former?  The width of a
>>> HT is by definition 8 columns throughout the git codebase.
>>
>> Ah, well, the width of a HT has been a free variable in my
>> calculations; I was operating under the assumption that whitespace
>> used for indentation can float freely according to the user's
>> settings. A few of the lines were aligning function arguments via tabs
>> and a few extra spaces, which is not reliable in my model.
>>
>> Frankly, I don't like tabs and spaces sharing the same contiguous
>> block. I don't like it all. ;-B
>
> Using tabs to align stuff to indentation level and spaces to align
> line continuation is the only possible way to let users choose
> whichever indentation depth they want while preserving the continuation
> alignment. What's not to like about that? Especially if you think a
> horizontal tab can be any size at all, you should be all agog.

However, nobody ever gets that right, because they don't understand
what you mean by indentation level. For most code that people write,
indentation whitespace overlaps with non-whitespace (from the
previous line). Therefore, the spaces used for continuation alignment
with the previous line are left 'sitting on shaky ground'. For
instance, consider this fine piece of code, given by lines 609-612 in
builtin-log.c. If you're viewing this code with a monospace font and
HT set to 8 columns, then the tab->spaces expansion should give the
following result (monospace font required to view this properly):

static void make_cover_letter(struct rev_info *rev, int use_stdout,
                              int numbered, int numbered_files,
                              struct commit *origin,
                              int nr, struct commit **list, struct commit *head)

Beautiful, isn't it?

In this case, we're at indentation level ---> 0 <---, and this means
that to form the continuation alignment, 0 tabs should be used
followed by however many spaces are needed to achieve the alignment.
However, whoever wrote that code actually used indentation level 3:

<HT><HT><HT><SP><SP><SP><SP><SP><SP>

The problem is that variable-width characters (HTs) are being aligned
with fixed-width characters. For shame!

To be explicit, if the HT width is set to 4, the result would be
(after HT->spaces expansion):

static void make_cover_letter(struct rev_info *rev, int use_stdout,
                  int numbered, int numbered_files,
                  struct commit *origin,
                  int nr, struct commit **list, struct commit *head)

Clearly, this is never a problem if you declare HTs to be fixed-width
(which, I believe, Linus did do: 8 columns per HT), or if you can
remember that HTs should never be used to align against non-HTs.

What I like to do is provide a more regular structure that removes the
juxtaposition of indentation and alignment whitespace. Basically, HTs
and spaces should never be contiguous, and the first use of a non-HT
prohibits HTs from being used anywhere further down the line. Also,
the structure of the code should facilitate identation levels:

static void make_cover_letter
(
	/* These are all at indentation level 1 */

	struct rev_info* rev            ,
	int              use_stdout     ,
	int              numbered       ,
	int              numbered_files ,
	struct commit*   origin         ,
	int              nr             ,
	struct commit**  list           ,
	struct commit*   head
)
{
	/* Code at indentation level 1 */
}

Yeah, yeah, I know it's crazy, but it makes code wildly readable---to me.

Sincerely,
Michael Witten

Re: [PATCH RFC3 05/13] send-email: Cleanup the usage text and docs a bit

[Michael Witten]

On Tue, Apr 14, 2009 at 01:46, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Witten <mfwitten@gmail.com> writes:
>
>> The usage text for --smtp-server currently has <host:port>, which
>> doesn't work when 'ssl' is used for the connection to the server.
>
> Ah, I see.  But it does work when ssl is not used, right?

Right.

> A possible solution would be one of:
>
>  - support host:port in SSL codepath (shouldn't it be trivial?);

Extremely trivial.

>  - split the description in the documentation to clarify it does not work
>   for SSL; or
>
>  - remove host:port support to make both consistent.
>
> To me, the last one makes the least sense.  Is that the approach you are
> taking?

That is SORT OF the approach I took. I got lazy and didn't care to
make <host:port> work for SSL, because there is already
--smtp-server-port and by just replacing <host:port> with <host> in
the USAGE text, the whole <host:port> notation was effectively
deprecated but still usable as before. Genius, right? Grade *A*
laziness.

Re: [PATCH RFC3 09/13] send-email: Remove horrible mix of tabs and spaces

[Andreas Ericsson]

Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
> 
>> Using tabs to align stuff to indentation level and spaces to align
>> line continuation is the only possible way to let users choose
>> whichever indentation depth they want while preserving the continuation
>> alignment.
> 
> Sure, it will look aligned with whatever width of HT ">":
> 
> 	>>if (this is a looong
>         >>....expression that is alighed) {
>         >>>statement1;
>         >>>statement2;
>         >>}
> 
> but it is *only true* if your SP "." and everything else is of the same
> width.
> 
> People seem to repeat that without realizing what they are saying, but I
> find the assumption the argument is based on quite bogus.  Why do people
> think it is only sane to assume flexible HT width but still monospaced
> font whose SP, l and w are all of the same width?
> 

Because 99% of all editors designed for programming use a fixed-width font
by default for precisely that reason, and all others can be set to display
fixed-width fonts without losing much (if any) readability, but indentation
comes in 2*$number_of_programmers variations.


> You either forget about the alignment (there is no such thing---suck it
> up), or use time honored HT=8 and monospace convention.
> 

Or you use "indent with tabs, align with spaces" and let those who use a
word processor for programming go hang. It's quite common actually. Or
you use some own homecooked style of indentation that's quite obvious
*to you* Either way, a project's founding father always sets the style,
and those who contribute get to either follow it or provide some pretty
strong argument to change it (a lot better than:

"A few of the lines were aligning function arguments via tabs
and a few extra spaces, which is not reliable in my model.

Frankly, I don't like tabs and spaces sharing the same contiguous
block. I don't like it all."

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

Re: [PATCH RFC3 09/13] send-email: Remove horrible mix of tabs and spaces

[Andreas Ericsson]

Michael Witten wrote:
> On Tue, Apr 14, 2009 at 00:31, Andreas Ericsson <ae@op5.se> wrote:
>> Michael Witten wrote:
>>> On Mon, Apr 13, 2009 at 15:55, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Michael Witten <mfwitten@gmail.com> writes:
>>>>
>>>>> Currently, no lines match the following:
>>>>>
>>>>>    [\t]+[ ]+
>>>>>    [ ]+[\t]+
>>>> I understand the latter but what's wrong with the former?  The width of a
>>>> HT is by definition 8 columns throughout the git codebase.
>>> Ah, well, the width of a HT has been a free variable in my
>>> calculations; I was operating under the assumption that whitespace
>>> used for indentation can float freely according to the user's
>>> settings. A few of the lines were aligning function arguments via tabs
>>> and a few extra spaces, which is not reliable in my model.
>>>
>>> Frankly, I don't like tabs and spaces sharing the same contiguous
>>> block. I don't like it all. ;-B
>> Using tabs to align stuff to indentation level and spaces to align
>> line continuation is the only possible way to let users choose
>> whichever indentation depth they want while preserving the continuation
>> alignment. What's not to like about that? Especially if you think a
>> horizontal tab can be any size at all, you should be all agog.
> 
> However, nobody ever gets that right, because they don't understand
> what you mean by indentation level. For most code that people write,
> indentation whitespace overlaps with non-whitespace (from the
> previous line). Therefore, the spaces used for continuation alignment
> with the previous line are left 'sitting on shaky ground'. For
> instance, consider this fine piece of code, given by lines 609-612 in
> builtin-log.c. If you're viewing this code with a monospace font and
> HT set to 8 columns, then the tab->spaces expansion should give the
> following result (monospace font required to view this properly):
> 
> static void make_cover_letter(struct rev_info *rev, int use_stdout,
>                               int numbered, int numbered_files,
>                               struct commit *origin,
>                               int nr, struct commit **list, struct commit *head)
> 
> Beautiful, isn't it?
> 
> In this case, we're at indentation level ---> 0 <---, and this means
> that to form the continuation alignment, 0 tabs should be used
> followed by however many spaces are needed to achieve the alignment.
> However, whoever wrote that code actually used indentation level 3:
> 
> <HT><HT><HT><SP><SP><SP><SP><SP><SP>
> 

Well, that would have been true if git had used "indent with tabs align
with spaces" (allowing variable width HT's), but HT is defined as 8
spaces wide, so that's a non-issue for us.

> The problem is that variable-width characters (HTs) are being aligned
> with fixed-width characters. For shame!
> 
> To be explicit, if the HT width is set to 4, the result would be
> (after HT->spaces expansion):
> 
> static void make_cover_letter(struct rev_info *rev, int use_stdout,
>                   int numbered, int numbered_files,
>                   struct commit *origin,
>                   int nr, struct commit **list, struct commit *head)
> 
> Clearly, this is never a problem if you declare HTs to be fixed-width
> (which, I believe, Linus did do: 8 columns per HT), or if you can
> remember that HTs should never be used to align against non-HTs.
> 

There you have it then; It's not a problem for git.git, and yet you
propose to fix it.

> What I like to do is provide a more regular structure that removes the
> juxtaposition of indentation and alignment whitespace. Basically, HTs
> and spaces should never be contiguous, and the first use of a non-HT
> prohibits HTs from being used anywhere further down the line. Also,
> the structure of the code should facilitate identation levels:
> 
> static void make_cover_letter
> (
> 	/* These are all at indentation level 1 */
> 
> 	struct rev_info* rev            ,
> 	int              use_stdout     ,
> 	int              numbered       ,
> 	int              numbered_files ,
> 	struct commit*   origin         ,
> 	int              nr             ,
> 	struct commit**  list           ,
> 	struct commit*   head
> )
> {
> 	/* Code at indentation level 1 */
> }
> 
> Yeah, yeah, I know it's crazy, but it makes code wildly readable---to me.
> 

It would still fail with a variable-width font though, which you seem
keen on reminding people about.

Since you seem really, really keen on getting this done, here's the only
way Junio would possibly even consider your new indentation style for
git.git:
* Make patches for all the .c and .h files.
* Make patches to all topics sent to the list that haven't been merged
  or dropped yet so that merges can go on cleanly.
* Create a public git repository somewhere which Junio can fetch from,
  marking your topic-branches clearly.
* Come up with a better argument than "Hey, this is better for me".
  Since churning code just to make life a little bit easier for one guy
  responsible for 0.07% of the committed codebase would be damn silly,
  I'm reasonably certain Junio will just shake his head at your proposal.
  Without the patches though, he'll probably not even look at it. 


There's one pretty good reason *not* to change indentation though:
Ownership tracking in case of relicensing. Currently, the "owners"
of a file in git.git can be determined pretty accurately by issuing

  git blame -C -C -M "$f" | \
  sed -e 's/[^(]*(\([^0-9]*\).*/\1/' -e 's/[\t ]*$//' \
        | sort | uniq -c | sort -nr

which is immensely useful when asking permission to relicense code
for libgit2. With your proposed code churn, that goes out the window
for no benefit what so ever.

Happy hacking. I'll drop this subject now, hoping you do the same.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

Re: [PATCH RFC3 08/13] send-email: Remove superfluous `my $editor = ...'

[Junio C Hamano]

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> On 2009.04.13 16:39:52 -0700, Stephen Boyd wrote:
>> Michael Witten wrote:
>>> -	my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
>>
>> I don't want to start an editor war, but why is 'vi' here? It seems
>> that one of the previous four should be set at all times, correct?
>
> Probably because that's how the rest of git works, too. See core.editor
> in git-config(1) or launch_editor() in editor.c if you don't trust the
> docs.
>
> Oh, and I seem to rely on that behaviour :-)

The git specific parts are obviously our invention, but $VISUAL then
$EDITOR and finally as the last resort falling back on vi has been a
longstanding UNIXy tradition that is shared by many tools.

Re: [PATCH RFC3 INTRO] I hope this will do it!

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> Here is the list of notable improvements:

I've picked up pieces that I think cannot possibly be controversial to
reduce the size of the remaining patch series still in flight.

Michael Witten (6):
      Docs: send-email: Put options back into alphabetical order
      Docs: send-email: Refer to CONFIGURATION section for sendemail.multiedit
      Docs: send-email: --smtp-server-port can take symbolic ports
      send-email: Handle "GIT:" rather than "GIT: " during --compose
      send-email: 'References:' should only reference what is sent
      send-email: Remove superfluous `my $editor = ...'

I've reworded the description in "symbolic port names" one a bit.

It is unfortunate that the interesting ones begin at 10th in the series,
which are beind the 9th one that is a "churn in the middle".

Thanks.

Re: [PATCH RFC3 INTRO] I hope this will do it!

[Michael Witten]

On Tue, Apr 14, 2009 at 04:02, Junio C Hamano <gitster@pobox.com> wrote:
>
> It is unfortunate that the interesting ones begin at 10th in the series,
> which are beind the 9th one that is a "churn in the middle".

(http://marc.info/?l=git&m=123959564630157&w=2):
> The general rule of thumb is to do such a clean-up before you start to work on something of substance.

(http://marc.info/?l=git&m=123914648915106&w=2):
> a good rule of thumb when preparing a series is to have this kind of obvious clean-up first, leaving enhancement patches later in the series.

I guess, then, this whitespace patch is something of substance---an
enhancement ;-D

... or unnecessary "code churn"... I suppose.

Re: [PATCH RFC3 INTRO] I hope this will do it!

[Junio C Hamano]

Michael Witten <mfwitten@gmail.com> writes:

> On Tue, Apr 14, 2009 at 04:02, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> It is unfortunate that the interesting ones begin at 10th in the series,
>> which are beind the 9th one that is a "churn in the middle".
>
> (http://marc.info/?l=git&m=123959564630157&w=2):
>> The general rule of thumb is to do such a clean-up before you start to work on something of substance.
>
> (http://marc.info/?l=git&m=123914648915106&w=2):
>> a good rule of thumb when preparing a series is to have this kind of obvious clean-up first, leaving enhancement patches later in the series.
>
> I guess, then, this whitespace patch is something of substance---an
> enhancement ;-D
>
> ... or unnecessary "code churn"... I suppose.

An obviously good and uncontroversial clean-up should come first, so that
it can be applied and meat of the change can be discussed on the cleaned
base version.

A clean-up that might be judged as a mere churn should come last, so that
enhancements and fixes can go first without waiting for the controversy to
settle.

Re: [PATCH RFC3 INTRO] I hope this will do it!

[Michael Witten]

On Tue, Apr 14, 2009 at 13:47, Junio C Hamano <gitster@pobox.com> wrote:
>>> It is unfortunate that the interesting ones begin at 10th in the series,
>>> which are beind the 9th one that is a "churn in the middle".
>>
>> (http://marc.info/?l=git&m=123959564630157&w=2):
>>> The general rule of thumb is to do such a clean-up before you start to work on something of substance.
>>
>> (http://marc.info/?l=git&m=123914648915106&w=2):
>>> a good rule of thumb when preparing a series is to have this kind of obvious clean-up first, leaving enhancement patches later in the series.
>>
>> I guess, then, this whitespace patch is something of substance---an
>> enhancement ;-D
>>
>> ... or unnecessary "code churn"... I suppose.
>
> An obviously good and uncontroversial clean-up should come first, so that
> it can be applied and meat of the change can be discussed on the cleaned
> base version.
>
> A clean-up that might be judged as a mere churn should come last, so that
> enhancements and fixes can go first without waiting for the controversy to
> settle.

I'm was just being facetious. In fact, I have already moved that patch
to the end on my side.