[PATCH v2] send-email: Add simple email aliases format

[PATCH v2] send-email: Add simple email aliases format

[Allen Hubbe]

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

Aliases are specified one per line.  There is no line splitting.

Example:
	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>

Signed-off-by: Allen Hubbe <allenbh@gmail.com>
---

Notes:
    The v1 of this patch had the following subject line:
    git-send-email.perl: Add sendmail aliases support
    
    This v2 renames this email alias format to simple, because the syntax
    that is actually supported by the parser differs from the format used by
    sendmail.  Now, there is no mention of sendmail in the name of the
    format, the documentation, or the commit message.
    
    This v2 also adds a test case to t/t9001-send-email.sh, and updates the
    list of alias file types in Documentation/git-send-email.txt.

 Documentation/git-send-email.txt |  2 +-
 git-send-email.perl              |  6 +++++-
 t/t9001-send-email.sh            | 24 ++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 804554609def..99583c4f8969 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -383,7 +383,7 @@ sendemail.aliasesFile::
 
 sendemail.aliasFileType::
 	Format of the file(s) specified in sendemail.aliasesFile. Must be
-	one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
+	one of 'mutt', 'mailrc', 'pine', 'elm', 'gnus', or 'simple'.
 
 sendemail.multiEdit::
 	If true (default), a single editor instance will be spawned to edit
diff --git a/git-send-email.perl b/git-send-email.perl
index e1e9b1460ced..25d72e8db8bf 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -515,7 +515,11 @@ my %parse_alias = (
 			       $aliases{$alias} = [ split_addrs($addr) ];
 			  }
 		      } },
-
+	simple => sub { my $fh = shift; while (<$fh>) {
+		if (/^\s*(\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 ];
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 7be14a4e37f7..bbb73cdf8bec 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1548,6 +1548,30 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
 		2>errors >out &&
 	grep "^!someone@example\.org!$" commandline1
 '
+test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
+	clean_fake_sendmail && rm -fr outdir &&
+	git format-patch -1 -o outdir &&
+	{
+		echo "alice: Alice W Land <awol@example.com>"
+		echo "bob: Robert Bobbyton <bob@example.com>"
+		echo "chloe: chloe@example.com"
+		echo "abgroup: alice, bob"
+		echo "bcgrp: bob, chloe, Other <o@example.com>"
+	} >~/.tmp-email-aliases &&
+	git config --replace-all sendemail.aliasesfile \
+		"$(pwd)/.tmp-email-aliases" &&
+	git config sendemail.aliasfiletype simple &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=alice --to=bcgrp \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		outdir/0001-*.patch \
+		2>errors >out &&
+	grep "^!awol@example\.com!$" commandline1 &&
+	grep "^!bob@example\.com!$" commandline1 &&
+	grep "^!chloe@example\.com!$" commandline1 &&
+	grep "^!o@example\.com!$" commandline1
+'
 
 do_xmailer_test () {
 	expected=$1 params=$2 &&
-- 
2.3.4

Re: [PATCH v2] send-email: Add simple email aliases format

[Eric Sunshine]

On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe <allenbh@gmail.com> wrote:
> 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>...]
>
> Aliases are specified one per line.  There is no line splitting.
>
> Example:
>         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>
>
> Signed-off-by: Allen Hubbe <allenbh@gmail.com>
> ---
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 804554609def..99583c4f8969 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -383,7 +383,7 @@ sendemail.aliasesFile::
>
>  sendemail.aliasFileType::
>         Format of the file(s) specified in sendemail.aliasesFile. Must be
> -       one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
> +       one of 'mutt', 'mailrc', 'pine', 'elm', 'gnus', or 'simple'.

It's perhaps somewhat unfortunate that the formats of the other alias
file types aren't described here, however, the reader can at least
look them up. But the new "simple" format is never described anywhere
in the documentation, so it's effectively unusable. Most users will be
unable or unwilling to consult the source code or the commit message
to figure out how to use this format. The description you wrote for
the commit message might be sufficient as proper documentation (with
proper Asciidoc formatting, of course).

>  sendemail.multiEdit::
>         If true (default), a single editor instance will be spawned to edit
> diff --git a/git-send-email.perl b/git-send-email.perl
> index e1e9b1460ced..25d72e8db8bf 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -515,7 +515,11 @@ my %parse_alias = (
>                                $aliases{$alias} = [ split_addrs($addr) ];
>                           }
>                       } },
> -
> +       simple => sub { my $fh = shift; while (<$fh>) {
> +               if (/^\s*(\S+)\s*:\s*(.+)$/) {

I imagine that users would appreciate being able to add comments to
their aliases file, and the implementation complexity to support
comment lines and blank lines (as described in the Postfix aliases
documentation you cited earlier[1]) would be so minor that I'm rather
surprised you chose not to do so.

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

> +                       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 ];
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 7be14a4e37f7..bbb73cdf8bec 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1548,6 +1548,30 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
>                 2>errors >out &&
>         grep "^!someone@example\.org!$" commandline1
>  '
> +test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
> +       clean_fake_sendmail && rm -fr outdir &&
> +       git format-patch -1 -o outdir &&
> +       {
> +               echo "alice: Alice W Land <awol@example.com>"
> +               echo "bob: Robert Bobbyton <bob@example.com>"
> +               echo "chloe: chloe@example.com"
> +               echo "abgroup: alice, bob"
> +               echo "bcgrp: bob, chloe, Other <o@example.com>"
> +       } >~/.tmp-email-aliases &&

A here-doc would be easier to maintain and read:

    cat >~/.tmp-email-aliases <<-\EOF &&
        alice: Alice W Land <awol@example.com>
        bob: Robert Bobbyton <bob@example.com>
        ...
    EOF

> +       git config --replace-all sendemail.aliasesfile \
> +               "$(pwd)/.tmp-email-aliases" &&
> +       git config sendemail.aliasfiletype simple &&
> +       git send-email \
> +               --from="Example <nobody@example.com>" \
> +               --to=alice --to=bcgrp \
> +               --smtp-server="$(pwd)/fake.sendmail" \
> +               outdir/0001-*.patch \
> +               2>errors >out &&
> +       grep "^!awol@example\.com!$" commandline1 &&
> +       grep "^!bob@example\.com!$" commandline1 &&
> +       grep "^!chloe@example\.com!$" commandline1 &&
> +       grep "^!o@example\.com!$" commandline1
> +'
>
>  do_xmailer_test () {
>         expected=$1 params=$2 &&
> --
> 2.3.4

Re: [PATCH v2] send-email: Add simple email aliases format

[Allen Hubbe]

On May 21, 2015 9:05 PM, "Eric Sunshine" <sunshine@sunshineco.com> wrote:
>
> On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe <allenbh@gmail.com> wrote:
> > 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>...]
> >
> > Aliases are specified one per line.  There is no line splitting.
> >
> > Example:
> >         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>
> >
> > Signed-off-by: Allen Hubbe <allenbh@gmail.com>
> > ---
> > diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> > index 804554609def..99583c4f8969 100644
> > --- a/Documentation/git-send-email.txt
> > +++ b/Documentation/git-send-email.txt
> > @@ -383,7 +383,7 @@ sendemail.aliasesFile::
> >
> >  sendemail.aliasFileType::
> >         Format of the file(s) specified in sendemail.aliasesFile. Must be
> > -       one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
> > +       one of 'mutt', 'mailrc', 'pine', 'elm', 'gnus', or 'simple'.
>
> It's perhaps somewhat unfortunate that the formats of the other alias
> file types aren't described here, however, the reader can at least
> look them up. But the new "simple" format is never described anywhere
> in the documentation, so it's effectively unusable. Most users will be
> unable or unwilling to consult the source code or the commit message
> to figure out how to use this format. The description you wrote for
> the commit message might be sufficient as proper documentation (with
> proper Asciidoc formatting, of course).

Ok, I will add the description in the commit, formatted, to the documentation.

>
> >  sendemail.multiEdit::
> >         If true (default), a single editor instance will be spawned to edit
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > index e1e9b1460ced..25d72e8db8bf 100755
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -515,7 +515,11 @@ my %parse_alias = (
> >                                $aliases{$alias} = [ split_addrs($addr) ];
> >                           }
> >                       } },
> > -
> > +       simple => sub { my $fh = shift; while (<$fh>) {
> > +               if (/^\s*(\S+)\s*:\s*(.+)$/) {
>
> I imagine that users would appreciate being able to add comments to
> their aliases file, and the implementation complexity to support
> comment lines and blank lines (as described in the Postfix aliases
> documentation you cited earlier[1]) would be so minor that I'm rather
> surprised you chose not to do so.

I will add support for comments. Anything after the first '#' in any
line will be treated as a comment.

>
> [1]: http://www.postfix.org/aliases.5.html
>
> > +                       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 ];
> > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> > index 7be14a4e37f7..bbb73cdf8bec 100755
> > --- a/t/t9001-send-email.sh
> > +++ b/t/t9001-send-email.sh
> > @@ -1548,6 +1548,30 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
> >                 2>errors >out &&
> >         grep "^!someone@example\.org!$" commandline1
> >  '
> > +test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
> > +       clean_fake_sendmail && rm -fr outdir &&
> > +       git format-patch -1 -o outdir &&
> > +       {
> > +               echo "alice: Alice W Land <awol@example.com>"
> > +               echo "bob: Robert Bobbyton <bob@example.com>"
> > +               echo "chloe: chloe@example.com"
> > +               echo "abgroup: alice, bob"
> > +               echo "bcgrp: bob, chloe, Other <o@example.com>"
> > +       } >~/.tmp-email-aliases &&
>
> A here-doc would be easier to maintain and read:
>
>     cat >~/.tmp-email-aliases <<-\EOF &&
>         alice: Alice W Land <awol@example.com>
>         bob: Robert Bobbyton <bob@example.com>
>         ...
>     EOF

A here-doc does not flow nicely in an indented block.  Each line in
the here-doc will also contain any indentation which may appear to the
reader to be part of the test case.  Alternatively, the here-doc could
be indented differently than the surrounding test case (all the way to
the left column), but that also has a negative impact for readability.
Finally, the EOF marker can not be indented.

With echo "string", exactly "string" is output to the line.  The
operation is obvious to the reader.  The test case can use sane
indentation, and the resulting output will be exactly what what it
would appear to be in the test case.

Especially for something like a test case where there should be
absolutely no confusion as to exactly what is the input to the test,
clarity matters.  Any operation where the result is not immediately
obvious to the reader, does not belong here.  Therefore, I will keep
the lines in the test case as echo "string".

>
> > +       git config --replace-all sendemail.aliasesfile \
> > +               "$(pwd)/.tmp-email-aliases" &&
> > +       git config sendemail.aliasfiletype simple &&
> > +       git send-email \
> > +               --from="Example <nobody@example.com>" \
> > +               --to=alice --to=bcgrp \
> > +               --smtp-server="$(pwd)/fake.sendmail" \
> > +               outdir/0001-*.patch \
> > +               2>errors >out &&
> > +       grep "^!awol@example\.com!$" commandline1 &&
> > +       grep "^!bob@example\.com!$" commandline1 &&
> > +       grep "^!chloe@example\.com!$" commandline1 &&
> > +       grep "^!o@example\.com!$" commandline1
> > +'
> >
> >  do_xmailer_test () {
> >         expected=$1 params=$2 &&
> > --
> > 2.3.4

Re: [PATCH v2] send-email: Add simple email aliases format

[Eric Sunshine]

On Thu, May 21, 2015 at 11:19 PM, Allen Hubbe <allenbh@gmail.com> wrote:
> On May 21, 2015 9:05 PM, "Eric Sunshine" <sunshine@sunshineco.com> wrote:
>> On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe <allenbh@gmail.com> wrote:
>> > +test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
>> > +       clean_fake_sendmail && rm -fr outdir &&
>> > +       git format-patch -1 -o outdir &&
>> > +       {
>> > +               echo "alice: Alice W Land <awol@example.com>"
>> > +               echo "bob: Robert Bobbyton <bob@example.com>"
>> > +               echo "chloe: chloe@example.com"
>> > +               echo "abgroup: alice, bob"
>> > +               echo "bcgrp: bob, chloe, Other <o@example.com>"
>> > +       } >~/.tmp-email-aliases &&
>>
>> A here-doc would be easier to maintain and read:
>>
>>     cat >~/.tmp-email-aliases <<-\EOF &&
>>         alice: Alice W Land <awol@example.com>
>>         bob: Robert Bobbyton <bob@example.com>
>>         ...
>>     EOF
>
> A here-doc does not flow nicely in an indented block.  Each line in
> the here-doc will also contain any indentation which may appear to the
> reader to be part of the test case.  Alternatively, the here-doc could
> be indented differently than the surrounding test case (all the way to
> the left column), but that also has a negative impact for readability.
> Finally, the EOF marker can not be indented.

That's true if you use <<EOF here-doc, but not for <<-EOF, as I did in
the example. With <<-EOF, all leading tabs are stripped from the input
lines, including from the EOF line, which is why it can be indented to
the same level as the other code in the test. The added '\' in <<-\EOF
from my example indicates that you don't want/expect any interpolation
inside the here-doc. The <<-\EOF form is used extensively throughout
the Git test suite.

> With echo "string", exactly "string" is output to the line.  The
> operation is obvious to the reader.  The test case can use sane
> indentation, and the resulting output will be exactly what what it
> would appear to be in the test case.

Same with <<-\EOF; plus <<-\EOF content is more readable since it's
not polluted with 'echo' noise.

> Especially for something like a test case where there should be
> absolutely no confusion as to exactly what is the input to the test,
> clarity matters.  Any operation where the result is not immediately
> obvious to the reader, does not belong here.  Therefore, I will keep
> the lines in the test case as echo "string".

Re: [PATCH v2] send-email: Add simple email aliases format

[Allen Hubbe]

On Thu, May 21, 2015 at 11:59 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Thu, May 21, 2015 at 11:19 PM, Allen Hubbe <allenbh@gmail.com> wrote:
>> On May 21, 2015 9:05 PM, "Eric Sunshine" <sunshine@sunshineco.com> wrote:
>>> On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe <allenbh@gmail.com> wrote:
>>> > +test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
>>> > +       clean_fake_sendmail && rm -fr outdir &&
>>> > +       git format-patch -1 -o outdir &&
>>> > +       {
>>> > +               echo "alice: Alice W Land <awol@example.com>"
>>> > +               echo "bob: Robert Bobbyton <bob@example.com>"
>>> > +               echo "chloe: chloe@example.com"
>>> > +               echo "abgroup: alice, bob"
>>> > +               echo "bcgrp: bob, chloe, Other <o@example.com>"
>>> > +       } >~/.tmp-email-aliases &&
>>>
>>> A here-doc would be easier to maintain and read:
>> A here-doc does not flow nicely in an indented block.  Each line in
> That's true if you use <<EOF here-doc, but not for <<-EOF, as I did in
> the example. With <<-EOF, all leading tabs are stripped from the input
> lines, including from the EOF line, which is why it can be indented to
> the same level as the other code in the test. The added '\' in <<-\EOF
> from my example indicates that you don't want/expect any interpolation
> inside the here-doc. The <<-\EOF form is used extensively throughout
> the Git test suite.

Alright.