On May 21, 2015 9:05 PM, "Eric Sunshine" <sunsh...@sunshineco.com> wrote:
>
> On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe <alle...@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 <a...@example.com>
> >         bob: Robert Bobbyton <b...@example.com>
> >         chloe: ch...@example.com
> >         abgroup: alice, bob
> >         bcgrp: bob, chloe, Other <o...@example.com>
> >
> > Signed-off-by: Allen Hubbe <alle...@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 <a...@example.com>"
> > +               echo "bob: Robert Bobbyton <b...@example.com>"
> > +               echo "chloe: ch...@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 <a...@example.com>
>         bob: Robert Bobbyton <b...@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 <nob...@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
--
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

Reply via email to