[PATCH 3/3] Add new dependencies caused by git-send-email-script to debian/control

[PATCH 3/3] Add new dependencies caused by git-send-email-script to debian/control

[Ryan Anderson]

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

 debian/control |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

6dbf602b0931608831888e779612fcc89b90d16f
diff --git a/debian/control b/debian/control
--- a/debian/control
+++ b/debian/control
@@ -8,7 +8,7 @@ Standards-Version: 3.6.1
 Package: git-core
 Architecture: any
 Depends: ${shlibs:Depends}, ${misc:Depends}, patch, diff, rcs
-Recommends: rsync, curl, ssh
+Recommends: rsync, curl, ssh, libmail-sendmail-perl, libemail-valid-perl
 Conflicts: git
 Description: The git content addressable filesystem
  GIT comes in two layers. The bottom layer is merely an extremely fast

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

[Ryan Anderson]

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

[PATCH 2/3] Add documentation for git-send-email-script

[Ryan Anderson]

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

 Documentation/git-send-email-script.txt |   61 +++++++++++++++++++++++++++++++
 1 files changed, 61 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-send-email-script.txt

799a6320d3b07347869093beec303afbc005cf26
diff --git a/Documentation/git-send-email-script.txt b/Documentation/git-send-email-script.txt
new file mode 100644
--- /dev/null
+++ b/Documentation/git-send-email-script.txt
@@ -0,0 +1,61 @@
+git-send-email-script(1)
+=======================
+v0.1, July 2005
+
+NAME
+----
+git-send-email-script - Send a collection of patches as emails
+
+
+SYNOPSIS
+--------
+'git-send-email-script' [options] <file|directory> [... file|directory]
+
+
+
+DESCRIPTION
+-----------
+Takes the patches given on the command line and emails them out.
+
+The header of the email is configurable by command line options.  If not
+specified on the command line, the user will be prompted with a ReadLine
+enabled interface to provide the necessary information.
+
+The options available are:
+
+  --to
+	Specify the primary recipient of the emails generated.
+	Generally, this will be the upstream maintainer of the
+	project involved.
+
+   --from
+	Specify the sender of the emails.  This will default to
+	the value GIT_COMMITTER_IDENT, as returned by "git-var -l".
+	The user will still be prompted to confirm this entry.
+
+   --subject
+   	Specify the initial subject of the email thread.
+
+   --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=" "
+
+Author
+------
+Written by Ryan Anderson <ryan@michonline.com>
+
+git-send-email-script is originally based upon
+send_lots_of_email.pl by Greg Kroah-Hartman.
+
+Documentation
+--------------
+Documentation by Ryan Anderson
+
+GIT
+---
+Part of the link:git.html[git] suite
+

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

[Ryan Anderson]

On Sun, Jul 31, 2005 at 04:17:25AM -0400, Ryan Anderson wrote:
> 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

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

[Matthias Urlichs]

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.

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

[Junio C Hamano]

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

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

[Johannes Schindelin]

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

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

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

On Sun, 31 Jul 2005 04:17:25 -0400 Ryan Anderson wrote:

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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

[Junio C Hamano]

Oh, another thing.  Could you refrain from doing
quoted-printable when possible?  Thanks.

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

[Ryan Anderson]

On Sun, Jul 31, 2005 at 02:45:29AM -0700, Junio C Hamano wrote:
> 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.)

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

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


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

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

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

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

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

> 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

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

[Ryan Anderson]

On Sun, Jul 31, 2005 at 12:25:13PM +0200, Johannes Schindelin wrote:
> 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

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

[Junio C Hamano]

Ryan Anderson <ryan@michonline.com> writes:

> 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

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

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

[Matthias Urlichs]

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

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

[Noel Maddy]

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>

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

[Junio C Hamano]

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.

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

[Noel Maddy]

On Tue, Aug 02, 2005 at 09:37:36AM -0700, Junio C Hamano wrote:
> 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>

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

[Ryan Anderson]

On Sun, Jul 31, 2005 at 02:45:29AM -0700, Junio C Hamano wrote:
> 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

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

[Matthias Urlichs]

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.