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

2015-05-22 Thread Allen Hubbe
On Thu, May 21, 2015 at 11:59 PM, Eric Sunshine  wrote:
> On Thu, May 21, 2015 at 11:19 PM, Allen Hubbe  wrote:
>> On May 21, 2015 9:05 PM, "Eric Sunshine"  wrote:
>>> On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe  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 "
>>> > +   echo "bob: Robert Bobbyton "
>>> > +   echo "chloe: ch...@example.com"
>>> > +   echo "abgroup: alice, bob"
>>> > +   echo "bcgrp: bob, chloe, Other "
>>> > +   } >~/.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 < 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 11:19 PM, Allen Hubbe  wrote:
> On May 21, 2015 9:05 PM, "Eric Sunshine"  wrote:
>> On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe  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 "
>> > +   echo "bob: Robert Bobbyton "
>> > +   echo "chloe: ch...@example.com"
>> > +   echo "abgroup: alice, bob"
>> > +   echo "bcgrp: bob, chloe, Other "
>> > +   } >~/.tmp-email-aliases &&
>>
>> A here-doc would be easier to maintain and read:
>>
>> cat >~/.tmp-email-aliases <<-\EOF &&
>> alice: Alice W Land 
>> bob: Robert Bobbyton 
>> ...
>> 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 < 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".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-21 Thread Allen Hubbe
On May 21, 2015 9:05 PM, "Eric Sunshine"  wrote:
>
> On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe  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.
> >
> > : [, ...]
> >
> > Aliases are specified one per line.  There is no line splitting.
> >
> > Example:
> > alice: Alice W Land 
> > bob: Robert Bobbyton 
> > chloe: ch...@example.com
> > abgroup: alice, bob
> > bcgrp: bob, chloe, Other 
> >
> > Signed-off-by: Allen Hubbe 
> > ---
> > 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 "
> > +   echo "bob: Robert Bobbyton "
> > +   echo "chloe: ch...@example.com"
> > +   echo "abgroup: alice, bob"
> > +   echo "bcgrp: bob, chloe, Other "
> > +   } >~/.tmp-email-aliases &&
>
> A here-doc would be easier to maintain and read:
>
> cat >~/.tmp-email-aliases <<-\EOF &&
> alice: Alice W Land 
> bob: Robert Bobbyton 
> ...
> 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, 

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

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe  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.
>
> : [, ...]
>
> Aliases are specified one per line.  There is no line splitting.
>
> Example:
> alice: Alice W Land 
> bob: Robert Bobbyton 
> chloe: ch...@example.com
> abgroup: alice, bob
> bcgrp: bob, chloe, Other 
>
> Signed-off-by: Allen Hubbe 
> ---
> 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 "
> +   echo "bob: Robert Bobbyton "
> +   echo "chloe: ch...@example.com"
> +   echo "abgroup: alice, bob"
> +   echo "bcgrp: bob, chloe, Other "
> +   } >~/.tmp-email-aliases &&

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

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

> +   git config --replace-all sendemail.aliasesfile \
> +   "$(pwd)/.tmp-email-aliases" &&
> +   git config sendemail.aliasfiletype simple &&
> +   git send-email \
> +   --from="Example " \
> +   --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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-21 Thread 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.

: [, ...]

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

Example:
alice: Alice W Land 
bob: Robert Bobbyton 
chloe: ch...@example.com
abgroup: alice, bob
bcgrp: bob, chloe, Other 

Signed-off-by: Allen Hubbe 
---

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 "
+   echo "bob: Robert Bobbyton "
+   echo "chloe: ch...@example.com"
+   echo "abgroup: alice, bob"
+   echo "bcgrp: bob, chloe, Other "
+   } >~/.tmp-email-aliases &&
+   git config --replace-all sendemail.aliasesfile \
+   "$(pwd)/.tmp-email-aliases" &&
+   git config sendemail.aliasfiletype simple &&
+   git send-email \
+   --from="Example " \
+   --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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html