threads / patch / 1376

patch, 3 partsAdd git-send-email-script - tool to send emails from git-format-patch-script

Subject: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

## tl;dr

16 messages between Jul 31, 2005 and Aug 3, 2005. Diffs are folded; open one to read it.

replies: 15people: 6as markdown or json

Ryan Anderson· Jul 31, 2005, 08:17 UTC · lore

This is based off of GregKH's script, send-lots-of-email.pl, and strives to do all the nice things a good subsystem maintainer does when forwarding a patch or 50 upstream:

	All the prior handlers of the patch, as determined by the
	Signed-off-by: lines, and/or the author of the commit, are cc:ed on the
	email.
	All emails are sent as a reply to the previous email, making it easy to
	skip a collection of emails that are uninteresting.
Signed-off-by: Ryan Anderson <ryan@michonline.com>
---
 Makefile              |    2 
 git-send-email-script |  265 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 266 insertions(+), 1 deletions(-)
 create mode 100755 git-send-email-script
55d4b5b7a11448d60eb00b5a7081954663842b06
Show changes to 2 files +266 −1

Makefile, git-send-email-script

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -62,7 +62,7 @@ SCRIPTS=git git-apply-patch-script git-m
 	git-format-patch-script git-sh-setup-script git-push-script \
 	git-branch-script git-parse-remote git-verify-tag-script \
 	git-ls-remote-script git-clone-dumb-http git-rename-script \
-	git-request-pull-script
+	git-request-pull-script git-send-email-script
 
 PROG=   git-update-cache git-diff-files git-init-db git-write-tree \
 	git-read-tree git-commit-tree git-cat-file git-fsck-cache \
diff --git a/git-send-email-script b/git-send-email-script
new file mode 100755
--- /dev/null
+++ b/git-send-email-script
@@ -0,0 +1,265 @@
+#!/usr/bin/perl -w
+# horrible hack of a script to send off a large number of email messages, one after
+# each other, all chained together.  This is useful for large numbers of patches.
+#
+# Use at your own risk!!!!
+#
+# greg kroah-hartman Jan 8, 2002
+# <greg@kroah.com>
+#
+# GPL v2 (See COPYING)
+# 
+# Ported to support git "mbox" format files by Ryan Anderson <ryan@michonline.com>
+#
+# Sends emails to the email listed on the command line.
+# 
+# updated to give a valid subject and CC the owner of the patch - Jan 2005
+# first line of the message is who to CC, 
+# and second line is the subject of the message.
+# 
+
+use strict;
+use warnings;
+use Term::ReadLine;
+use Mail::Sendmail;
+use Getopt::Long;
+use Data::Dumper;
+use Email::Valid;
+
+# Variables we fill in automatically, or via prompting:
+my (@to,@cc,$initial_reply_to,$initial_subject,@files,$from);
+
+# Example of them
+# modify these options each time you run the script
+#$to = 'torvalds@osdl.org,git@vger.kernel.org';
+#$initial_reply_to = ''; #<20050203173208.GA23964@foobar.com>';
+#$initial_subject = "[PATCH] Deb package build fixes";
+#@files = (qw(
+#0001-Make-debian-rules-executable-and-correct-the-spelling-of-rsync-in.txt
+#0002-Debian-packages-should-include-the-binaries.txt
+#0003-The-deb-package-building-needs-these-two-new-files-to-work-correctly.txt
+#));
+
+# change this to your email address.
+#$from = "Ryan Anderson <ryan\@michonline.com>";
+
+my $term = new Term::ReadLine 'git-send-email';
+
+# Begin by accumulating all the variables (defined above), that we will end up
+# needing, first, from the command line:
+
+my $rc = GetOptions("from=s" => \$from,
+                    "in-reply-to=s" => \$initial_reply_to,
+		    "subject=s" => \$initial_subject,
+		    "to=s" => \@to,
+	 );
+
+# Now, let's fill any that aren't set in with defaults:
+
+open(GITVAR,"-|","git-var","-l")
+	or die "Failed to open pipe from git-var: $!";
+
+my ($author,$committer);
+while(<GITVAR>) {
+	chomp;
+	my ($var,$data) = split /=/,$_,2;
+	my @fields = split /\s+/, $data;
+
+	my $ident = join(" ", @fields[0...(@fields-3)]);
+
+	if ($var eq 'GIT_AUTHOR_IDENT') {
+		$author = $ident;
+	} elsif ($var eq 'GIT_COMMITTER_IDENT') {
+		$committer = $ident;
+	}
+}
+close(GITVAR);
+
+
+if (!defined $from) {
+	$from = $author || $committer;
+	1 while (!defined ($_ = $term->readline("Who should the emails appear to be from? ", 
+				$from)));
+	$from = $_;
+	print "Emails will be sent from: ", $from, "\n";
+}
+
+if (!@to) {
+	1 while (!defined ($_ = $term->readline("Who should the emails be sent to? ", 
+				"")));
+	my $to = $_;
+	push @to, split /,/, $to;
+}
+
+if (!defined $initial_subject) {
+	1 while (!defined ($_ = 
+		$term->readline("What subject should the emails start with? ", 
+			$initial_subject)));
+	$initial_subject = $_;
+}
+
+if (!defined $initial_reply_to) {
+	1 while (!defined ($_ = 
+		$term->readline("Message-ID to be used as In-Reply-To? ", 
+			$initial_reply_to)));
+	$initial_reply_to = $_;
+}
+
+# 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) {
+	if (-d $f) {
+		opendir(DH,$f)
+			or die "Failed to opendir $f: $!";
+
+		push @files, map { +$f . "/" . $_ } grep !/^\.{1,2}$/,
+			sort readdir(DH);
+	} elsif (-f $f) {
+		push @files, $f;
+
+	} else {
+		print STDERR "Skipping $f - not found.\n";
+	}
+}
+
+if (@files) {
+	print $_,"\n" for @files;
+} else {
+	print <<EOT;
+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.
+   --subject      Specify the initial "Subject:" line.
+   --in-reply-to  Specify the first "In-Reply-To:" header line.
+
+Error: Please specify a file or a directory on the command line.
+EOT
+	exit(1);
+}
+
+# Variables we set as part of the loop over files
+our ($message_id, $cc, %mail, $subject, $reply_to, $message);
+
+
+# Usually don't need to change anything below here.
+
+# we make a "fake" message id by taking the current number
+# of seconds since the beginning of Unix time and tacking on
+# a random number to the end, in case we are called quicker than
+# 1 second since the last time we were called.
+sub make_message_id
+{
+	my $date = `date "+\%s"`;
+	chomp($date);
+	my $pseudo_rand = int (rand(4200));
+	$message_id = "<$date$pseudo_rand\@foobar.com>";
+	print "new message id = $message_id\n";
+}
+
+
+
+$cc = "";
+
+sub send_message
+{
+	my %to;
+	$to{lc(Email::Valid->address($_))}++ for (@to);
+
+	my $to = join(",", keys %to);
+
+	%mail = (	To	=>	$to,
+			From	=>	$from,
+			CC	=>	$cc,
+			Subject	=>	$subject,
+			Message	=>	$message,
+			'Reply-to'	=>	$from,
+			'In-Reply-To'	=>	$reply_to,
+			'Message-ID'	=>	$message_id,
+			'X-Mailer'	=>	"git-send-email-script",
+		);
+
+	$mail{smtp} = 'localhost';
+
+	#print Data::Dumper->Dump([\%mail],[qw(*mail)]);
+
+	sendmail(%mail) or die $Mail::Sendmail::error;
+
+	print "OK. Log says:\n", $Mail::Sendmail::log;
+	print "\n\n"
+}
+
+
+$reply_to = $initial_reply_to;
+make_message_id();
+$subject = $initial_subject;
+
+foreach my $t (@files) {
+	my $F = $t;
+	open(F,"<",$t) or die "can't open file $t";
+
+	@cc = ();
+	my $found_mbox = 0;
+	my $header_done = 0;
+	$message = "";
+	while(<F>) {
+		if (!$header_done) {
+			$found_mbox = 1, next if (/^From /);
+			chomp;
+
+			if ($found_mbox) {
+				if (/^Subject:\s+(.*)$/) {
+					$subject = $1;
+
+				} elsif (/^(Cc|From):\s+(.*)$/) {
+					printf("(mbox) Adding cc: %s from line '%s'\n",
+						$2, $_);
+					push @cc, $2;
+				}
+
+			} else {
+				# In the traditional
+				# "send lots of email" format,
+				# line 1 = cc
+				# line 2 = subject
+				# So let's support that, too.
+				if (@cc == 0) {
+					printf("(non-mbox) Adding cc: %s from line '%s'\n",
+						$_, $_);
+
+					push @cc, $_;
+
+				} elsif (!defined $subject) {
+					$subject = $_;
+				}
+			}
+			
+			# A whitespace line will terminate the headers
+			if (m/^\s*$/) {
+				$header_done = 1;
+			}
+		} else {
+			$message .=  $_;
+			if (/^Signed-off-by: (.*)$/i) {
+				my $c = $1;
+				chomp $c;
+				push @cc, $c;
+				printf("(sob) Adding cc: %s from line '%s'\n",
+					$c, $_);
+			}
+		}
+	}
+	close F;
+
+	my %clean_ccs;
+	$clean_ccs{lc(Email::Valid->address($_))}++ for @cc;
+
+	$cc = join(",", keys %clean_ccs);
+
+	send_message();
+
+	# set up for the next message
+	$reply_to = $message_id;
+	make_message_id();
+#	$subject = "Re: ".$initial_subject;
+}
Ryan Anderson· Jul 31, 2005, 08:24 UTC · re: Ryan Anderson · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

On Sun, Jul 31, 2005 at 04:17:25AM -0400, Ryan Anderson wrote:
Show 12 quoted lines
> This is based off of GregKH's script, send-lots-of-email.pl, and strives to do
> all the nice things a good subsystem maintainer does when forwarding a patch or
> 50 upstream:
> 
> 	All the prior handlers of the patch, as determined by the
> 	Signed-off-by: lines, and/or the author of the commit, are cc:ed on the
> 	email.
> 
> 	All emails are sent as a reply to the previous email, making it easy to
> 	skip a collection of emails that are uninteresting.
> 
> Signed-off-by: Ryan Anderson <ryan@michonline.com>

And yes, I did generate this thread with this script - so I have proof that it works nicely.

Actually, with this:
	git format-patch -n --mbox -o ../pending/ origin
	git send-email ../pending/
	<answer some questions>

In 2-3 minutes I should have my git tree at rsync://h4x0r5.com/git-ryan.git/ updated with these changes, and my SYNOPSIS draft. (I haven't added the short tutorial that Sam suggested yet, that's a task for another 4am hackfest.)

-- 
Ryan Anderson
  sometimes Pug Majere
Matthias Urlichs· Jul 31, 2005, 09:36 UTC · re: Ryan Anderson · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

Hi, Ryan Anderson wrote:
> And yes, I did generate this thread with this script - so I have proof
> that it works nicely.

It might make sense to create a "Patch 0/N" with a short explanation, and have the actual patches be replies to that -- or to patch 1/N if that's not necessary.

As it is, patch N hangs off patch N-1 in my email threading view, which gets slightly cumbersome if N>10.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
Nothing makes a person more productive than the last minute.
Junio C Hamano· Jul 31, 2005, 09:45 UTC · re: Ryan Anderson · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

Ryan Anderson <ryan@michonline.com> writes:
> 	All emails are sent as a reply to the previous email, making it easy to
> 	skip a collection of emails that are uninteresting.

I understand why _some_ people consider this preferable, but wonder if this should have a knob to be tweaked.

For example, I myself often find it very hard to read when a cascading thread goes very deep like this:

     [PATCH 0/9] cover
       [PATCH 1/9] first one
         [PATCH 2/9] second one
          [PATCH 3/9] third one in the series
            ...

and prefer to see this instead (this assumes your MUA is half-way decent and lets you sort by subject):

     [PATCH 0/9] cover
       [PATCH 1/9] first one
       [PATCH 2/9] second one
       [PATCH 3/9] third one in the series
       ...
> +# horrible hack of a script to send off a large number of email messages, one after
> +# each other, all chained together.  This is useful for large numbers of patches.
> +#
> +# Use at your own risk!!!!

Well, if it is "Use at your own risk" maybe it should stay outside the official distribution for a while until it gets safer ;-).

> +	my @fields = split /\s+/, $data;
> +	my $ident = join(" ", @fields[0...(@fields-3)]);
Wouldn't "s/>.*/>/" be easier than splitting and joining?
> +if (!defined $from) {
> +	$from = $author || $committer;
> +	1 while (!defined ($_ = $term->readline("Who should the emails appear to be from? ", 
> +				$from)));

Judging from your past patches, you seem to really like statement modifiers[*]. While they _are_ valid Perl constructs, it is extremely hard to read when used outside very simple idiomatic use. Please consider rewriting the above and the like using compound statements[*] (I am using these terms according to the definition in perlsyn.pod). Remember, there are people Perl is not their native language, but are intelligent enough to be of great help fixing problems in programs you write in Perl. To most of them, compound statements are more familiar, so try to be gentle to them.

> +		opendir(DH,$f)
> +			or die "Failed to opendir $f: $!";
> +		push @files, map { +$f . "/" . $_ } grep !/^\.{1,2}$/,
> +			sort readdir(DH);
Maybe skip potential subdirs while you are at it, something like this?
    push @files, sort grep { -f $_ } map { "$f/$_" } readdir(DH)
> +	my $pseudo_rand = int (rand(4200));
> +	$message_id = "<$date$pseudo_rand\@foobar.com>";
> +	print "new message id = $message_id\n";

I doubt this hardcoded foobar.com is a good idea. Did you mean to print it, by the way?

> +	$to{lc(Email::Valid->address($_))}++ for (@to);
> +	my $to = join(",", keys %to);
Is this the culprit that produced this mechanical-looking line?
    To: junkio@cox.net,git@vger.kernel.org

Interestingly enough, you do not seem to do it for the From: line.

    From: Ryan Anderson <ryan@michonline.com>

Also you seem to be losing the ordering in @to and @cc by the use of uniquefying "keys %to" and "keys %cc". I can not offhand tell if it matters, but you probably would care, at least for the primary recipients listed in @to array.

> +	$mail{smtp} = 'localhost';

I suspect this probably need to be configurable. I may be a minority, but my outgoing messages are directly handed to my local ISP smtp server from my MUA, and the smtp server running on the locahost does not talk to the outside world.

> +	# set up for the next message
> +	$reply_to = $message_id;

Making chaining policy configurable would be just one liner change here, I suppose.

Since there are always 47 different ways to do the same thing in Perl, Perl style varies a lot more than Shell style which in turn varies a lot more than C. You should be prepared to be nitpicked a lot when you post Perl ;-). And I was in nitpicking mood tonight.

Thanks for the patch. Overall, very good intent. Slightly troublesome details.

-jc
Ryan Anderson· Jul 31, 2005, 23:52 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

On Sun, Jul 31, 2005 at 02:45:29AM -0700, Junio C Hamano wrote:
Show 7 quoted lines
> Ryan Anderson <ryan@michonline.com> writes:
> 
> > 	All emails are sent as a reply to the previous email, making it easy to
> > 	skip a collection of emails that are uninteresting.
> 
> I understand why _some_ people consider this preferable, but
> wonder if this should have a knob to be tweaked.
Hmm, fair enough.

I'll send a few more patches in a minute that deal with the things in this email but for now:

	--chain-reply-to (or --no-chain-reply-to)
	Will toggle between these two behaviors.  (This will not be
	prompted for by the ReadLine interface, btw.)
Show 8 quoted lines
> > +# horrible hack of a script to send off a large number of email messages, one after
> > +# each other, all chained together.  This is useful for large numbers of patches.
> > +#
> > +# Use at your own risk!!!!
> 
> Well, if it is "Use at your own risk" maybe it should stay
> outside the official distribution for a while until it gets
> safer ;-).

Heh. I missed some comments that I meant to clean up that were in Greg's original script. One of the patches will clean up the comments.

> > +	my @fields = split /\s+/, $data;
> > +	my $ident = join(" ", @fields[0...(@fields-3)]);
> 
> Wouldn't "s/>.*/>/" be easier than splitting and joining?

Most of GIT_COMMITTER_IDENT (and GIT_AUTHOR_IDENT) is use controllable, except for the "date" section of it. I know we delimit that with spaces, so the above is guaranteed to work unless we change the format that git-var returns.

If I hope that nobody has done something like:
	GIT_AUTHOR="Ryan <> Anderson"
	GIT_AUTHOR_EMAIL="ryan@michonline.com"

I get more confusing results. (I suddenly have to think about what that regular expression does in this case - and I'm pretty sure that the one you gave would do bad things.)

Probably the best fix for this would be to take libgit.a, make a shared library out of it, and then interface the Perl scripts directly with it via a .xs module. I was thinking that I'd rather have direct access to the git_ident* functions than calling out to git-var, anyway. Consider that a plan for a revamp after the core seems to have settled down a bit more.

Show 15 quoted lines
> > +if (!defined $from) {
> > +	$from = $author || $committer;
> > +	1 while (!defined ($_ = $term->readline("Who should the emails appear to be from? ", 
> > +				$from)));
> 
> Judging from your past patches, you seem to really like
> statement modifiers[*].  While they _are_ valid Perl constructs,
> it is extremely hard to read when used outside very simple
> idiomatic use.  Please consider rewriting the above and the like
> using compound statements[*] (I am using these terms according
> to the definition in perlsyn.pod).  Remember, there are people
> Perl is not their native language, but are intelligent enough to
> be of great help fixing problems in programs you write in Perl.
> To most of them, compound statements are more familiar, so try
> to be gentle to them.

I copied this from another program of mine, and I'm *sure* I copied the style directly from a ReadLine example. But, I can't find a current example that says this is good, so, I'll fix this, too. It is rather ugly. (The other uses of this style are... less bad, IMO, than this abuse here.)

Show 9 quoted lines
> 
> > +		opendir(DH,$f)
> > +			or die "Failed to opendir $f: $!";
> > +		push @files, map { +$f . "/" . $_ } grep !/^\.{1,2}$/,
> > +			sort readdir(DH);
> 
> Maybe skip potential subdirs while you are at it, something like this?
> 
>     push @files, sort grep { -f $_ } map { "$f/$_" } readdir(DH)

Good point. One one hand I'd say, "Let it break for people who do strange things like that", but I'll make it safer anyway.

(Someone is going to reply and ask for it to recurse into subdirectories now. Maybe Andrew Morton would find that useful with his rather massive collection of patches in -mm kernels. But that's a feature for next week.)

Show 6 quoted lines
> > +	my $pseudo_rand = int (rand(4200));
> > +	$message_id = "<$date$pseudo_rand\@foobar.com>";
> > +	print "new message id = $message_id\n";
> 
> I doubt this hardcoded foobar.com is a good idea.  Did you mean
> to print it, by the way?

I'll convert this to something that is based off the $from address instead. It's probably better that way, anyway.

Show 6 quoted lines
> > +	$to{lc(Email::Valid->address($_))}++ for (@to);
> > +	my $to = join(",", keys %to);
> 
> Is this the culprit that produced this mechanical-looking line?
> 
>     To: junkio@cox.net,git@vger.kernel.org
No, that line was exactly what I put into the readline entry.
Show 9 quoted lines
> Interestingly enough, you do not seem to do it for the From:
> line.
> 
>     From: Ryan Anderson <ryan@michonline.com>
> 
> Also you seem to be losing the ordering in @to and @cc by the
> use of uniquefying "keys %to" and "keys %cc".  I can not offhand
> tell if it matters, but you probably would care, at least for
> the primary recipients listed in @to array.

Well, it was kind of annoying to see the same email address appear 2-3 times in the email, because of the way I pull in all the relevant emails from various places. So I really needed a way to cull the duplicates. I don't believe ordering is really significant in To: or Cc: lines, for really anyone. I could do soemthing like this, instead, I suppose:

	my @clean_to = ();
	my %dupe_check_to = ();
	foreach my $to_entry (@to) {
		if (!$dupe_check_to{Email::Valid->address($to_entry)}++) {
			push @clean_to, $to_entry;
		}
	}
	my $to = join(", ", @clean_to);

I just like the first one a little better (though, I can't really pin down why).

Show 6 quoted lines
> > +	$mail{smtp} = 'localhost';
> 
> I suspect this probably need to be configurable.  I may be a
> minority, but my outgoing messages are directly handed to my
> local ISP smtp server from my MUA, and the smtp server running
> on the locahost does not talk to the outside world.

Oddly, I think this is even the wrong setting after having read over the Mail::Sendmail docs to figure out how to change the MIME settings.

Show 5 quoted lines
> Since there are always 47 different ways to do the same thing in
> Perl, Perl style varies a lot more than Shell style which in
> turn varies a lot more than C.  You should be prepared to be
> nitpicked a lot when you post Perl ;-).  And I was in nitpicking
> mood tonight.

It's Perl. There does seem to be a lot of nitpicking when people write Perl.

(If you write it in idiomatic Perl, the non-Perl users hate you. If you write it like C, the Perl users complain that you write it like C. *sigh*)

> Thanks for the patch.  Overall, very good intent.  Slightly
> troublesome details.
Thanks.

I'll have a series of patches out in a few minutes that will update what I originally sent to fix your issues here.

-- 
Ryan Anderson
  sometimes Pug Majere
Junio C Hamano· Aug 1, 2005, 00:40 UTC · re: Ryan Anderson · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

Ryan Anderson <ryan@michonline.com> writes:
Show 5 quoted lines
> If I hope that nobody has done something like:
> 	GIT_AUTHOR="Ryan <> Anderson"
> 	GIT_AUTHOR_EMAIL="ryan@michonline.com"
>
> I get more confusing results.

The function git_author_info() would remove <> from the above GIT_AUTHOR_* environment values by calling ident.c:copy(), so I think you would get more-or-less what you _should_ expect without getting confused.

    $ GIT_AUTHOR_NAME="Ryan <> Anderson" \
      GIT_AUTHOR_EMAIL="ryan@michonline.com" git-var -l
    GIT_COMMITTER_IDENT=Junio C Hamano <junkio@cox.net> 1122855849 -0700
    GIT_AUTHOR_IDENT=Ryan  Anderson <ryan@michonline.com> 1122855849 -0700
Show 5 quoted lines
>> Is this the culprit that produced this mechanical-looking line?
>> 
>>     To: junkio@cox.net,git@vger.kernel.org
>
> No, that line was exactly what I put into the readline entry.

I was mostly talking about Email::Valid seeming to be stripping out the display-name[*] part and keeping only addr-spec[*] part, like this:

    $ cat j.perl
    #!/usr/bin/perl
    use Email::Valid;
    for ('Junio C Hamano <junkio@cox.net>',
         'Ryan Anderson <ryan@michonline.com>') {
        print $_, " => ", lc(Email::Valid->address($_)), "\n";
    }
    $ perl j.perl
    Junio C Hamano <junkio@cox.net> => junkio@cox.net
    Ryan Anderson <ryan@michonline.com> => ryan@michonline.com

Also, I wonder if running lc() to downcase the local-part[*] is safe/allowed/correct; domain[*] part is case insensitive and should be OK to downcase, though.

> ..., because of the way I pull in all the relevant emails
> from various places.  So I really needed a way to cull the duplicates.
> ...  I could do soemthing like this, instead, I suppose:

I understand your needs, and you can make it a "sub filter_dups", which I think would make things a lot more pleasant to read.

[Footnote]
Terms marked [*] are from RFC2822, section 3.4.
Matthias Urlichs· Aug 1, 2005, 07:59 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

Hi, Junio C Hamano wrote:
> Also, I wonder if running lc() to downcase the local-part[*] is
> safe/allowed/correct
mostly/no/no.
It's unlikely to be a real-life problem, but we still shouldn't do it.
-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
Amusements to virtue are like breezes of air to the flame -- gentle ones will
fan it, but strong ones will put it out.
					-- David Thomas
Ryan Anderson· Aug 3, 2005, 01:30 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

On Sun, Jul 31, 2005 at 02:45:29AM -0700, Junio C Hamano wrote:
Show 25 quoted lines
> Ryan Anderson <ryan@michonline.com> writes:
> 
> > 	All emails are sent as a reply to the previous email, making it easy to
> > 	skip a collection of emails that are uninteresting.
> 
> I understand why _some_ people consider this preferable, but
> wonder if this should have a knob to be tweaked.
> 
> For example, I myself often find it very hard to read when
> a cascading thread goes very deep like this:
> 
>      [PATCH 0/9] cover
>        [PATCH 1/9] first one
>          [PATCH 2/9] second one
>           [PATCH 3/9] third one in the series
>             ...
> 
> and prefer to see this instead (this assumes your MUA is
> half-way decent and lets you sort by subject):
> 
>      [PATCH 0/9] cover
>        [PATCH 1/9] first one
>        [PATCH 2/9] second one
>        [PATCH 3/9] third one in the series
>        ...

In testing this, I think I figured out *why* the former is preferred - it guarantees the sorting when in the standard mode of "order by date".

Since these emails are sent *very* fast, delivery order tends to be the dominating factor in how they sort in your inbox, as they will all have the same time. So that's a trifle annoying.

Yes, sorting by subject will fix it, but "threaded + subject" is actually something I'm not sure much of anything supports. (It's not instantly obvious how to do it in mutt, for example - though, admittedly, Mutt may very well have a way to do it.)

I noticed this while testing my 2 new fixes - those will sent in a few seconds.

-- 
Ryan Anderson
  sometimes Pug Majere
Matthias Urlichs· Aug 3, 2005, 07:32 UTC · re: Ryan Anderson · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

Hi, Ryan Anderson wrote:
> Since these emails are sent *very* fast, delivery order tends to be the
> dominating factor in how they sort in your inbox, as they will all have
> the same time.  So that's a trifle annoying.

That's trivially fixable: just generate your own Date: header and add a second for each email.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
Be careful whilst playing under the anvil tree.
Johannes Schindelin· Jul 31, 2005, 10:25 UTC · re: Ryan Anderson · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

Hi,

wouldn't it be a good idea to make $from and $to required parameters? At least you could infer a sensible default of $from from GIT_* environment variables, no? I am not quite comfortable with a hard coded sender in a script possibly deployed into a multi-user environment.

Ciao, Dscho

Ryan Anderson· Jul 31, 2005, 23:54 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

On Sun, Jul 31, 2005 at 12:25:13PM +0200, Johannes Schindelin wrote:
Show 6 quoted lines
> Hi,
> 
> wouldn't it be a good idea to make $from and $to required parameters? At
> least you could infer a sensible default of $from from GIT_* environment
> variables, no? I am not quite comfortable with a hard coded sender in a
> script possibly deployed into a multi-user environment.
The sender isn't hardcoded.

If it isn't sent on the command line, the return value from git-var -l is used to set a default, which is then confirmed by the ReadLine interface.

Some of the comments were leftover from Greg's original, before I remembered to remove them - it should be better now if you take another look. (See my, soon to be sent, set of additional changes.)

-- 
Ryan Anderson
  sometimes Pug Majere
Sergey Vlasov· Jul 31, 2005, 10:50 UTC · re: Ryan Anderson · lore

Re: [PATCH 1/3] Add git-send-email-script - tool to send emails from git-format-patch-script

On Sun, 31 Jul 2005 04:17:25 -0400 Ryan Anderson wrote:
Show 7 quoted lines
> This is based off of GregKH's script, send-lots-of-email.pl, and
> strives to do all the nice things a good subsystem maintainer does
> when forwarding a patch or 50 upstream:
> 
> 	All the prior handlers of the patch, as determined by the
> 	Signed-off-by: lines, and/or the author of the commit, are cc:ed
> 	on the email.
> 	All emails are sent as a reply to the previous email, making it
> 	easy to skip a collection of emails that are uninteresting.
Actually, this is the part of GregKH's script which I hate ;)

50 patches sent this way produce an enormous email thread; if someone then tries to comment on a 40th patch, this part of the thread ends up far behind the right edge of the message list window in almost any mail client (and if someone comments on the first patch, the comments are far from that patch).

Sending a [PATCH 0/N] message with the overall description of the patchset and then all patches as replies to that message looks much better in that respect. However, missing messages in this form are less obvious.

Noel Maddy· Aug 2, 2005, 12:48 UTC · lore

Re: [OT] Perl-ish perl vs. C-ish perl.

On Mon, Aug 01, 2005 at 04:21:08PM -0700, Junio C Hamano wrote:
> Noel Maddy <noel@zhtwn.com> writes:
(silly perl stuff)
> Please refrain from making this thread "I know more Perl than
> you do"; thank you.
Sorry. Just trying to help, but suitably chastened.
Thanks
-- 
Time is an illusion.  Lunchtime doubly so.
							-- Ford Prefect
+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
Noel Maddy <noel@zhtwn.com>
Junio C Hamano· Aug 2, 2005, 16:37 UTC · re: Noel Maddy · lore

Re: [OT] Perl-ish perl vs. C-ish perl.

Noel Maddy <noel@zhtwn.com> writes:
>> Please refrain from making this thread "I know more Perl than
>> you do"; thank you.
>
> Sorry. Just trying to help, but suitably chastened.

I realize that what you sent was not _too_ Perlish and being helpful. If you feel I overreacted, I am sorry; I _do_ think I did overreact, attempting to be preemptive.

Noel Maddy· Aug 2, 2005, 17:20 UTC · re: Junio C Hamano · lore

Re: [OT] Perl-ish perl vs. C-ish perl.

On Tue, Aug 02, 2005 at 09:37:36AM -0700, Junio C Hamano wrote:
Show 10 quoted lines
> Noel Maddy <noel@zhtwn.com> writes:
> 
> >> Please refrain from making this thread "I know more Perl than
> >> you do"; thank you.
> >
> > Sorry. Just trying to help, but suitably chastened.
> 
> I realize that what you sent was not _too_ Perlish and being
> helpful.  If you feel I overreacted, I am sorry; I _do_ think I
> did overreact, attempting to be preemptive.
No, I don't think you overreacted. Well, maybe a _little_ bit. ;)

I think you're focused on making git good (as in reliable and maintainable), and that strong focus is admirable.

Your clarification on the expected developer profile, and how to target the code to those developers helps, too.

For those of us who've spent years living in Perl, the idioms come much easier than those who are coming from other languages like C. But if the expected maintainers are not perl weenies (like me, I guess), then you're right, it's better to stay away from the more perl-y stuffy.

Again, thanks.
> 
-- 
If we can't define the user experience of Windows so that all Windows
machines operate the same way, then the Windows brand is meaningless.
							  -- Bill Gates
+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
Noel Maddy <noel@zhtwn.com>

← back to recent threads