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

Ryan Anderson [off-list ref] 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 [off-list ref]

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