[PATCH] git-send-email.perl: Add sendmail aliases support

[PATCH] git-send-email.perl: Add sendmail aliases support

[Allen Hubbe]

Support sendmail (and postfix) style email aliases in git-send-email.
This is the format of /etc/mail/aliases, documented by `man 5 aliases`.
The man page may be provided by sendmail or postfix on your system, or
by another email service that uses the same configuration file.

See also: http://www.postfix.org/aliases.5.html.

This format is more simple than the other alias file formats, so it may
be preferred by some users.  The format is as follows.

<alias>: <address|alias>[, <address|alias>...]

Example (no indent in aliases file):
	alice: Alice W Land <awol@example.com>
	bob: Robert Bobbyton <bob@example.com>
	chloe: chloe@example.com
	abgroup: alice, bob
	bcgrp: bob, chloe, Other <o@example.com>

This patch DOES NOT support line continuations as are described by the
specification.  Line continuations are indicated by either (or both) a
trailing '\' on the line to be continued, or leading whitespace on the
continuing line.  This patch does not do anything with the trailing '\',
and ignores lines starting with whitespace.

This patch also ignores lines that starts with '#', comment character,
and empty lines.

Signed-off-by: Allen Hubbe <allenbh@gmail.com>
---
 git-send-email.perl | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index e1e9b14..5f2ec0d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -515,7 +515,12 @@ my %parse_alias = (
 			       $aliases{$alias} = [ split_addrs($addr) ];
 			  }
 		      } },
-
+	sendmail => sub { my $fh = shift; while (<$fh>) {
+		next if /^$|^#|^\s/;
+		if (/^(\S+)\s*:\s*(.*?)\\?$/) {
+			my ($alias, $addr) = ($1, $2);
+			$aliases{$alias} = [ split_addrs($addr) ];
+		}}},
 	gnus => sub { my $fh = shift; while (<$fh>) {
 		if (/\(define-mail-alias\s+"(\S+?)"\s+"(\S+?)"\)/) {
 			$aliases{$1} = [ $2 ];
-- 
2.3.4

Re: [PATCH] git-send-email.perl: Add sendmail aliases support

[Junio C Hamano]

Allen Hubbe <allenbh@gmail.com> writes:

> diff --git a/git-send-email.perl b/git-send-email.perl
> index e1e9b14..5f2ec0d 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -515,7 +515,12 @@ my %parse_alias = (
>  			       $aliases{$alias} = [ split_addrs($addr) ];
>  			  }
>  		      } },
> -
> +	sendmail => sub { my $fh = shift; while (<$fh>) {
> +		next if /^$|^#|^\s/;
> +		if (/^(\S+)\s*:\s*(.*?)\\?$/) {
> +			my ($alias, $addr) = ($1, $2);
> +			$aliases{$alias} = [ split_addrs($addr) ];
> +		}}},

Let me unfold the line only to make commenting it easier.

	sendmail => sub {
        	my $fh = shift;
                while (<$fh>) {
			next if /^$|^#|^\s/;
			if (/^(\S+)\s*:\s*(.*?)\\?$/) {
				my ($alias, $addr) = ($1, $2);
				$aliases{$alias} = [ split_addrs($addr) ];
			}
		}
	},

It is probably OK to omit support for folded lines, but wouldn't it
be easy enough to be a bit more helpful to give a warning when you
find such lines in the input?  Otherwise you will leave the users
wondering why some of their aliases work while others don't.

Perhaps like this (this is not even an output from "diff" but typed
in my MUA, so there may be typos---take it just as illustrating
ideas)?

That way, users can fold the input themselves and try again if they
wanted to.  The warning _may_ have to be squelched after a few hits
to keep the result usable, though.

	sendmail => sub {
        	my $fh = shift;
                while (<$fh>) {
-			next if /^$|^#|^\s/;
-			if (/^(\S+)\s*:\s*(.*?)\\?$/) {
+			next if /^$|^#/;
+			if (/^\s/ || /\\$/) {
+				print STDERR "$.: $_";
+				print STDERR "continuation lines in alias not supported\n";
+				next;
+			}
+			if (/^(\S+)\s*:\s*(.*)$/) {
				my ($alias, $addr) = ($1, $2);
				$aliases{$alias} = [ split_addrs($addr) ];
			}
		}
	},

Thanks.

Re: [PATCH] git-send-email.perl: Add sendmail aliases support

[Allen Hubbe]

On Thu, May 21, 2015 at 4:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Allen Hubbe <allenbh@gmail.com> writes:
>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index e1e9b14..5f2ec0d 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -515,7 +515,12 @@ my %parse_alias = (
>>                              $aliases{$alias} = [ split_addrs($addr) ];
>>                         }
>>                     } },
>> -
>> +     sendmail => sub { my $fh = shift; while (<$fh>) {
>> +             next if /^$|^#|^\s/;
>> +             if (/^(\S+)\s*:\s*(.*?)\\?$/) {
>> +                     my ($alias, $addr) = ($1, $2);
>> +                     $aliases{$alias} = [ split_addrs($addr) ];
>> +             }}},
>
> Let me unfold the line only to make commenting it easier.
>
>         sendmail => sub {
>                 my $fh = shift;
>                 while (<$fh>) {
>                         next if /^$|^#|^\s/;
>                         if (/^(\S+)\s*:\s*(.*?)\\?$/) {
>                                 my ($alias, $addr) = ($1, $2);
>                                 $aliases{$alias} = [ split_addrs($addr) ];
>                         }
>                 }
>         },
>
> It is probably OK to omit support for folded lines, but wouldn't it
> be easy enough to be a bit more helpful to give a warning when you
> find such lines in the input?  Otherwise you will leave the users
> wondering why some of their aliases work while others don't.

The diff doesn't show enough context to include this comment:

my %parse_alias = (
        # multiline formats can be supported in the future
...

I can't be sure the author's intent, but my interpretation is such.
The parsers do not support multiline, even though the format might
allow it by definition.  Another interpretation could be, no multiline
formats allowed, or, the first person to add a multiline format should
remove this comment.

I think the first interpretation is correct, because according to this
script, the mutt format also has continuation lines.  I didn't find a
more authoritative document in my quick search.

http://www.wizzu.com/mutt/checkalias.pl

I suppose at this point it is also worth mentioning that /etc/aliases
doesn't claim to support aliases of aliases, but does support
non-email-addresses like mail directories and pipes.  I don't think
most git users would try to send email directly to a pipe.

My motivation for this patch was not really to support the sendmail
aliases file directly.  The commit message may therefore be
misleading.  So, I could also rewrite the commit message to say
something like, "loosely based on" the sendmail aliases format, if the
exceptions to the format in the current message are not enough.
Really, I just prefer the simpler <alias>: <email|alias> syntax
instead of the ones for mutt, elm, etc, and that is why I wrote this
patch.

>
> Perhaps like this (this is not even an output from "diff" but typed
> in my MUA, so there may be typos---take it just as illustrating
> ideas)?
>
> That way, users can fold the input themselves and try again if they
> wanted to.  The warning _may_ have to be squelched after a few hits
> to keep the result usable, though.
>
>         sendmail => sub {
>                 my $fh = shift;
>                 while (<$fh>) {
> -                       next if /^$|^#|^\s/;
> -                       if (/^(\S+)\s*:\s*(.*?)\\?$/) {
> +                       next if /^$|^#/;
> +                       if (/^\s/ || /\\$/) {
> +                               print STDERR "$.: $_";
> +                               print STDERR "continuation lines in alias not supported\n";
> +                               next;
> +                       }
> +                       if (/^(\S+)\s*:\s*(.*)$/) {
>                                 my ($alias, $addr) = ($1, $2);
>                                 $aliases{$alias} = [ split_addrs($addr) ];
>                         }
>                 }
>         },

That's interesting.  I'd like to hear a second opinion before I add
that.  It's a good idea, but none of the other parsing routines print
out messages.

>
> Thanks.

Re: [PATCH] git-send-email.perl: Add sendmail aliases support

[Eric Sunshine]

On Thu, May 21, 2015 at 4:48 PM, Allen Hubbe <allenbh@gmail.com> wrote:
> My motivation for this patch was not really to support the sendmail
> aliases file directly.  The commit message may therefore be
> misleading.  So, I could also rewrite the commit message to say
> something like, "loosely based on" the sendmail aliases format, if the
> exceptions to the format in the current message are not enough.

That probably would be a good idea. After reading the 'aliases' man
page you cited[1], I was wondering why your parser diverged from it in
so many ways. In addition to lack of support for folded lines, your
format:

* only recognizes comment lines when '#' is the first character,
whereas [1] allows whitespace before '#'

* only recognizes zero-length lines as empty, whereas [1] more loosely
interprets a whitespace-only line as empty

* doesn't support quoting the 'name' part of "name: value" as [1] does

> Really, I just prefer the simpler <alias>: <email|alias> syntax
> instead of the ones for mutt, elm, etc, and that is why I wrote this
> patch.

Your patch is missing a documentation update
(Documentation/git-send-email.txt) and new tests
(t/t9001-send-email.sh).

[1] http://www.postfix.org/aliases.5.html

Re: [PATCH] git-send-email.perl: Add sendmail aliases support

[Junio C Hamano]

Allen Hubbe <allenbh@gmail.com> writes:

> The diff doesn't show enough context to include this comment:
>
> my %parse_alias = (
>         # multiline formats can be supported in the future
> ...
>
> I can't be sure the author's intent, but my interpretation is such.
> The parsers do not support multiline, even though the format might
> allow it by definition.  Another interpretation could be, no multiline
> formats allowed, or, the first person to add a multiline format should
> remove this comment.

I think that comment were only meant to apply to the first one, mutt.

In any case, you should read it this way:

    Some formats may support multi-line; this parser back when the
    comment was written did not support an alias file that uses such
    a feature, but it is OK to make it support them in the future. 

After all, these subs are slurping from $fh and doing the parsing
themselves, so they are free to do multi-line if they wanted to.
It's not like there is a calling function that feeds input line by
line after splitting a logically continued line into two (and if
that were the case, supporting multi-line format may become harder
or even impossible).

And as I said, it is OK not to support ones that have folded lines.

All I was saying was that we should not SILENTLY fail or do a wrong
thing when we find something we do not support.

Re: [PATCH] git-send-email.perl: Add sendmail aliases support

[Allen Hubbe]

On Thu, May 21, 2015 at 5:05 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Thu, May 21, 2015 at 4:48 PM, Allen Hubbe <allenbh@gmail.com> wrote:
>> My motivation for this patch was not really to support the sendmail
>> aliases file directly.  The commit message may therefore be
>> misleading.  So, I could also rewrite the commit message to say
>> something like, "loosely based on" the sendmail aliases format, if the
>> exceptions to the format in the current message are not enough.
>
> That probably would be a good idea. After reading the 'aliases' man
> page you cited[1], I was wondering why your parser diverged from it in
> so many ways. In addition to lack of support for folded lines, your
> format:
>
> * only recognizes comment lines when '#' is the first character,
> whereas [1] allows whitespace before '#'
>
> * only recognizes zero-length lines as empty, whereas [1] more loosely
> interprets a whitespace-only line as empty
>
> * doesn't support quoting the 'name' part of "name: value" as [1] does
>

Those are good points.  Maybe I shouldn't even mention sendmail at
all, not in the name of the format, and not in the commit message.
What name would be a good name for this format?

All the other formats would are based on the formats of email clients.
Adding sendmail was a departure from that.  Adding a format that is
not associated with any email program would be an even further
departure.  I don't know how other people feel about this.  Hopefully,
if this new format seems like it would be useful, the fact that it is
not an email client format can be overlooked.

>> Really, I just prefer the simpler <alias>: <email|alias> syntax
>> instead of the ones for mutt, elm, etc, and that is why I wrote this
>> patch.
>
> Your patch is missing a documentation update
> (Documentation/git-send-email.txt) and new tests
> (t/t9001-send-email.sh).

Thanks for the direction.

>
> [1] http://www.postfix.org/aliases.5.html

Re: [PATCH] git-send-email.perl: Add sendmail aliases support

[Junio C Hamano]

Allen Hubbe <allenbh@gmail.com> writes:

> Those are good points.  Maybe I shouldn't even mention sendmail at
> all, not in the name of the format, and not in the commit message.
> What name would be a good name for this format?

"simple"?

And if you are going to define such a format, then I do not think
you would even need to pretend that someday you might support
line folding (hence there is no need for "eh, excuse me, you seem to
have wanted to express a long folded line here, but I do not support
it (yet)" warning messages).

Re: [PATCH] git-send-email.perl: Add sendmail aliases support

[Allen Hubbe]

On Thu, May 21, 2015 at 5:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Allen Hubbe <allenbh@gmail.com> writes:
>
>> Those are good points.  Maybe I shouldn't even mention sendmail at
>> all, not in the name of the format, and not in the commit message.
>> What name would be a good name for this format?
>
> "simple"?

Alright, as long as people agree that _this_ is the simple format, and
not something else.  I'll call it simple in v2, and see if anyone
complains.

>
> And if you are going to define such a format, then I do not think
> you would even need to pretend that someday you might support
> line folding (hence there is no need for "eh, excuse me, you seem to
> have wanted to express a long folded line here, but I do not support
> it (yet)" warning messages).
>