[PATCH 0/2] Update git-send-email-script with --compose

[PATCH 0/2] Update git-send-email-script with --compose

[Ryan Anderson]

As I promised Junio a few weeks ago (via private email), here's an update to git-send-email-script
that fixes a few annoying things it does.

1. No longer require --subject for most uses.
2. No longer require --in-reply-to for most uses.
3. If not given as command line parameters, don't prompt for either 1 or 2.
4. Add --compose that lets the user compose an email using $EDITOR, and make
use of --subject and --in-reply-to

The first patch is the update to the code, the second patch updates the
documentation to match.  And the fact that this is on the list at all is proof
that it works (for me, at least.)

-- 
Ryan Anderson
  sometimes Pug Majere

[PATCH 1/2] Make git-send-email-script ignore some unnecessary options when operating in batch mode.

[Ryan Anderson]

Add a "--compose" option that uses $EDITOR to edit an "introductory" email to the patch series.

Signed-off-by: Ryan Anderson <ryan@michonline.com>

---

 git-send-email-script |   86 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 81 insertions(+), 5 deletions(-)

b3c7bf0ee80c1d207c9208972f011540b4c0528f
diff --git a/git-send-email-script b/git-send-email-script
--- a/git-send-email-script
+++ b/git-send-email-script
@@ -25,9 +25,13 @@ use Data::Dumper;
 use Email::Valid;
 
 sub unique_email_list(@);
+sub cleanup_compose_files();
+
+# Constants (essentially)
+my $compose_filename = ".msg.$$";
 
 # Variables we fill in automatically, or via prompting:
-my (@to,@cc,$initial_reply_to,$initial_subject,@files,$from);
+my (@to,@cc,$initial_reply_to,$initial_subject,@files,$from,$compose);
 
 # Behavior modification variables
 my ($chain_reply_to, $smtp_server) = (1, "localhost");
@@ -46,6 +50,7 @@ my $rc = GetOptions("from=s" => \$from,
 		    "to=s" => \@to,
 		    "chain-reply-to!" => \$chain_reply_to,
 		    "smtp-server=s" => \$smtp_server,
+		    "compose" => \$compose,
 	 );
 
 # Now, let's fill any that aren't set in with defaults:
@@ -69,7 +74,7 @@ while(<GITVAR>) {
 }
 close(GITVAR);
 
-
+my $prompting = 0;
 if (!defined $from) {
 	$from = $author || $committer;
 	do {
@@ -79,6 +84,7 @@ if (!defined $from) {
 
 	$from = $_;
 	print "Emails will be sent from: ", $from, "\n";
+	$prompting++;
 }
 
 if (!@to) {
@@ -88,19 +94,21 @@ if (!@to) {
 	} while (!defined $_);
 	my $to = $_;
 	push @to, split /,/, $to;
+	$prompting++;
 }
 
-if (!defined $initial_subject) {
+if (!defined $initial_subject && $compose) {
 	do {
 		$_ = $term->readline("What subject should the emails start with? ",
 			$initial_subject);
 	} while (!defined $_);
 	$initial_subject = $_;
+	$prompting++;
 }
 
-if (!defined $initial_reply_to) {
+if (!defined $initial_reply_to && $prompting) {
 	do {
-		$_= $term->readline("Message-ID to be used as In-Reply-To? ",
+		$_= $term->readline("Message-ID to be used as In-Reply-To for the first email? ",
 			$initial_reply_to);
 	} while (!defined $_);
 
@@ -112,6 +120,52 @@ if (!defined $smtp_server) {
 	$smtp_server = "localhost";
 }
 
+if ($compose) {
+	# Note that this does not need to be secure, but we will make a small
+	# effort to have it be unique
+	open(C,">",$compose_filename)
+		or die "Failed to open for writing $compose_filename: $!";
+	print C "From \n";
+	printf C "Subject: %s\n\n", $initial_subject;
+	printf C <<EOT;
+GIT: Please enter your email below.
+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.
+
+EOT
+	close(C);
+
+	my $editor = $ENV{EDITOR};
+	$editor = 'vi' unless defined $editor;
+	system($editor, $compose_filename);
+
+	open(C2,">",$compose_filename . ".final")
+		or die "Failed to open $compose_filename.final : " . $!;
+
+	open(C,"<",$compose_filename)
+		or die "Failed to open $compose_filename : " . $!;
+
+	while(<C>) {
+		next if m/^GIT: /;
+		print C2 $_;
+	}
+	close(C);
+	close(C2);
+
+	do {
+		$_ = $term->readline("Send this email? (y|n) ");
+	} while (!defined $_);
+
+	if (uc substr($_,0,1) ne 'Y') {
+		cleanup_compose_files();
+		exit(0);
+	}
+
+	@files = ($compose_filename . ".final");
+}
+
+
 # Now that all the defaults are set, process the rest of the command line
 # arguments and collect up the files that need to be processed.
 for my $f (@ARGV) {
@@ -137,12 +191,24 @@ if (@files) {
 git-send-email-script [options] <file | directory> [... file | directory ]
 Options:
    --from         Specify the "From:" line of the email to be sent.
+
    --to           Specify the primary "To:" line of the email.
+
+   --compose      Use \$EDITOR to edit an introductory message for the
+                  patch series.
+
    --subject      Specify the initial "Subject:" line.
+                  Only necessary if --compose is also set.  If --compose
+		  is not set, this will be prompted for.
+
    --in-reply-to  Specify the first "In-Reply-To:" header line.
+                  Only used if --compose is also set.  If --compose is not
+		  set, this will be prompted for.
+
    --chain-reply-to If set, the replies will all be to the previous
                   email sent, rather than to the first email sent.
                   Defaults to on.
+
    --smtp-server  If set, specifies the outgoing SMTP server to use.
                   Defaults to localhost.
 
@@ -278,6 +344,16 @@ foreach my $t (@files) {
 	make_message_id();
 }
 
+if ($compose) {
+	cleanup_compose_files();
+}
+
+sub cleanup_compose_files() {
+	unlink($compose_filename, $compose_filename . ".final");
+
+}
+
+
 
 sub unique_email_list(@) {
 	my %seen;

[PATCH 2/2] Update documentation of --compose to git-send-email-script.txt

[Ryan Anderson]

Signed-off-by: Ryan Anderson <ryan@michonline.com>

---

 Documentation/git-send-email-script.txt |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

60765e20aa12da748f43204e25cb582f88fb16c8
diff --git a/Documentation/git-send-email-script.txt b/Documentation/git-send-email-script.txt
--- a/Documentation/git-send-email-script.txt
+++ b/Documentation/git-send-email-script.txt
@@ -33,16 +33,21 @@ The options available are:
 	the value GIT_COMMITTER_IDENT, as returned by "git-var -l".
 	The user will still be prompted to confirm this entry.
 
+   --compose
+	Use \$EDITOR to edit an introductory message for the
+	patch series.
+
    --subject
    	Specify the initial subject of the email thread.
+	Only necessary if --compose is also set.  If --compose
+	is not set, this will be prompted for.
 
    --in-reply-to
 	Specify the contents of the first In-Reply-To header.
 	Subsequent emails will refer to the previous email 
-	instead of this.
-	When overriding on the command line, it may be necessary
-	to set this to a space.  For example
-		--in-reply-to=" "
+	instead of this if --chain-reply-to is set (the default)
+	Only necessary if --compose is also set.  If --compose
+	is not set, this will be prompted for.
 
    --chain-reply-to, --no-chain-reply-to
 	If this is set, each email will be sent as a reply to the previous

Re: [PATCH 0/2] Update git-send-email-script with --compose

[Martin Langhoff]

Ryan,

is it possible to fix the git-send-email script to "just work" reading
in the emails that `git-format-patch-script -o patchdir origin`
generates? I have a very ugly local patch to git-send-email-script
that

 - reads "from" from git-var, can be overridden by passing an explicit --from
 - reads "subject" from the first line of STDIN or file. If the line
doesn't start with [PATCH it provides the [PATCH] prefix. I found it
really confusing that it wants to get 'from' in the first line...
that's not what git-format-patch produces!
 - it never prompts for anything

I then invoke it with 

  git-send-email-script --to git@vger.kernel.org patches/0001-bad-uglypatch

and it "just works". I haven't sent them anywhere because I just
wanted it to work locally for me, and it's just a bunch of hacks. And
you are clearly using something other than git-format-patch to
generate those patchfiles -- and my patches would break that.

OTOH, it'd be great if it did support the git-format-patch output. Let
me know if you want bits and pieces of my hack - though it's trivial.

cheers,


martin

Re: [PATCH 0/2] Update git-send-email-script with --compose

[Ryan Anderson]

On Mon, Sep 05, 2005 at 11:16:57PM +1200, Martin Langhoff wrote:
> Ryan,
> 
> is it possible to fix the git-send-email script to "just work" reading
> in the emails that `git-format-patch-script -o patchdir origin`
> generates? I have a very ugly local patch to git-send-email-script
> that
> 
>  - reads "from" from git-var, can be overridden by passing an explicit --from

git-send-email-script already reads the "From:" from git-var - but I
suppose I should only *prompt* for the from if something isn't
satisfactory with the output of git-var.  TODO #1

>  - reads "subject" from the first line of STDIN or file. If the line
> doesn't start with [PATCH it provides the [PATCH] prefix. I found it
> really confusing that it wants to get 'from' in the first line...
> that's not what git-format-patch produces!

Sorry about that - I always export using git-format-patch using --mbox,
and those work nicely.  I'm a bit reluctant to do the [PATCH] fixup, but
I think I will:

	1. Detect [PATCH] or [PATCH [0-9]+/[0-9]+] (Sorry for the horrid
	fake-regexp)
	2. Provide a --no-fixup-subject to turn that off.

(TODO #2)

>  - it never prompts for anything

I think I'm tending in that direction - I'll keep making the prompting a
fall-back by adding better default detection.

In this case, remember that this was an attempt to help users patch bomb
lists, getting all the subtle details correct.  The prompting is there
to help get the subtle details correct!

> I then invoke it with 
> 
>   git-send-email-script --to git@vger.kernel.org patches/0001-bad-uglypatch
> 
> and it "just works". I haven't sent them anywhere because I just
> wanted it to work locally for me, and it's just a bunch of hacks. And
> you are clearly using something other than git-format-patch to
> generate those patchfiles -- and my patches would break that.

Well, I'm not.  Try "git format-patch --mbox -o patchdir origin" and see
if that works better for you.

> OTOH, it'd be great if it did support the git-format-patch output. Let
> me know if you want bits and pieces of my hack - though it's trivial.

Sure, send it at me, and I'll see what I can incorporate.

I do apologize for not realizing that the default git format-patch
output doesn't match what git send-email script expects the "legacy"
mode - I'll sort that out one way or another as well. (TODO #3)

-- 

Ryan Anderson
  sometimes Pug Majere

Re: [PATCH 0/2] Update git-send-email-script with --compose

[Junio C Hamano]

Ryan Anderson <ryan@michonline.com> writes:

> On Mon, Sep 05, 2005 at 11:16:57PM +1200, Martin Langhoff wrote:
>> 
>>  - reads "subject" from the first line of STDIN or file. If the line
>> doesn't start with [PATCH it provides the [PATCH] prefix. I found it
>> really confusing that it wants to get 'from' in the first line...
>> that's not what git-format-patch produces!
>
> Sorry about that - I always export using git-format-patch using --mbox,
> and those work nicely.  I'm a bit reluctant to do the [PATCH] fixup, but
> I think I will:
>
> 	1. Detect [PATCH] or [PATCH [0-9]+/[0-9]+] (Sorry for the horrid
> 	fake-regexp)
> 	2. Provide a --no-fixup-subject to turn that off.
>
> (TODO #2)

To be consistent with the other tools in tools/ directory, the
above is probably 's/^/[PATCH] / unless (/^\[PATCH/])'

> In this case, remember that this was an attempt to help users patch bomb
> lists, getting all the subtle details correct.  The prompting is there
> to help get the subtle details correct!

You could error out without asking if that is what is happening.

> Well, I'm not.  Try "git format-patch --mbox -o patchdir origin" and see
> if that works better for you.

Martin, --mbox has the added benefit that it consistently
preserves the From: and Date: information even for your own
patches, because it implies --date and --author.  By default
without --author and --date these are not preserved from the
original commits for your own patches, primarily because
format-patch without --mbox was written for reorganizing and
reordering existing patches (i.e. export, concatenate some, edit
some hunks, and eventually feed it to applymbox to make commits;
you do not typically want to keep the original author date for
this kind of use).

> I do apologize for not realizing that the default git format-patch
> output doesn't match what git send-email script expects the "legacy"
> mode - I'll sort that out one way or another as well. (TODO #3)

I do apologize for not really saying what --mbox does and what
the format-patch output without --mbox is meant for.

Martin, is there a reason you do not want --mbox format
(e.g. format-patch --mbox spits out Subject: line undesirably
formatted while it does what you want without --mbox)?

Re: [PATCH 0/2] Update git-send-email-script with --compose

[Martin Langhoff]

On 9/6/05, Junio C Hamano <junkio@cox.net> wrote:
> Ryan Anderson <ryan@michonline.com> writes:
> > Sorry about that - I always export using git-format-patch using --mbox,
> > and those work nicely.  I'm a bit reluctant to do the [PATCH] fixup, but
> > I think I will:

Thanks Ryan for the clarification! I hadn't realized it would work
correctly with --mbox -- unfortunately it doesn't work very well in
the one-file-per-patch (legacy?) mode. Also, telling it _not_ to
prompt when it can guess it, is far better (a confirm y/n can still be
a good thing if you want to ensure the user gets a chance to review
the values guessed).

> Martin, --mbox has the added benefit that it consistently
> preserves the From: and Date: information even for your own
> patches, because it implies --date and --author.  By default
> without --author and --date these are not preserved from the
> original commits for your own patches, primarily because
> format-patch without --mbox was written for reorganizing and
> reordering existing patches (i.e. export, concatenate some, edit
> some hunks, and eventually feed it to applymbox to make commits;
> you do not typically want to keep the original author date for
> this kind of use).

Fair enough -- blame it on my primitive approach of only having 2
working repositories, and having some patches in them that I'm not
pushing upstream. Exporting to mbox would mean that I have to edit the
mbox file to remove the patches I don't intend to publish.

... and on my naive reading of git-send-email documentation -- it
doesn't mention mbox format at all, so I assumed it would expect one
patch per file.

> Martin, is there a reason you do not want --mbox format
> (e.g. format-patch --mbox spits out Subject: line undesirably
> formatted while it does what you want without --mbox)?

Hmmm -- that I am too lazy to keep several heads or several repos, and
organize them to have a "tojunio" branch? So far I've been working on
one or two files (archimport) and customizing a couple of others with
strictly local changes (git-send-email for instance), so it didn't
make sense to formally segregate the heads. A simple review and manual
"cherrypicking" of the patches I wanted to send was enough.

cheers,


martin

Re: [PATCH 0/2] Update git-send-email-script with --compose

[Junio C Hamano]

Martin Langhoff <martin.langhoff@gmail.com> writes:

> Fair enough -- blame it on my primitive approach of only having 2
> working repositories, and having some patches in them that I'm not
> pushing upstream. Exporting to mbox would mean that I have to edit the
> mbox file to remove the patches I don't intend to publish.
>
> ... and on my naive reading of git-send-email documentation -- it
> doesn't mention mbox format at all, so I assumed it would expect one
> patch per file.

Not really; --mbox output is one-file-per-patch and it is up to
you which ones to pick and concatenate them in what order, if you
want them in a single file.

Re: [PATCH 0/2] Update git-send-email-script with --compose

[Martin Langhoff]

On 9/6/05, Junio C Hamano <junkio@cox.net> wrote:
> Not really; --mbox output is one-file-per-patch and it is up to
> you which ones to pick and concatenate them in what order, if you
> want them in a single file.

Hrmmmm. Then I better hide away in a little cave, and shut my big mouth up. ;-)

It shows that I was never familiar with the practices of linux
hackers. I've always read the references to mboxes holding patchbombs
meaning literally one mbox file with a zillion contatenated patches
received via email.

apologies,


martin

Re: [PATCH 0/2] Update git-send-email-script with --compose

[Junio C Hamano]

Martin Langhoff <martin.langhoff@gmail.com> writes:

> It shows that I was never familiar with the practices of linux
> hackers. I've always read the references to mboxes holding patchbombs
> meaning literally one mbox file with a zillion contatenated patches
> received via email.

To be fair to you, it is _not_ the practices of Linux hackers.
For one thing, I am not among them.  I think it is perfectly
reasonable to expect it to produce a single mailbox, like you
expected.

The primary reason why "format-patch --mbox" only does one file
per patch is because I am too lazy to add a --single-mbox option
which does the "right thing".  This _could_ be "fixed", but on
the other hand the current one file per patch behaviour is
arguably more flexible than always creating a single mbox, and
that is what I use as an excuse to justify my laziness ;-).