Re: [PATCH v5 1/1] send-email: Add sendmail email aliases format
On Tue, May 26, 2015 at 4:53 PM, Eric Sunshine wrote:
> On Tue, May 26, 2015 at 3:41 PM, Allen Hubbe wrote:
>> On Tue, May 26, 2015 at 3:10 PM, Eric Sunshine
>> wrote:
>>> On Saturday, May 23, 2015, Allen Hubbe wrote:
+ # recognize lines that look like an alias
+ elsif (/^(\S+)\s*:\s*(.+?)$/) {
>>>
>>> Observation: Given "foo:bar:baz", this regex will take "foo:bar" as
>>> the key, and "baz" as the value, which is probably not what was
>>> intended, however, it likely doesn't matter much in this case since
>>> colon isn't legal in an email address[1].
>>
>> That's a keen observation. I think it would work simply to use a
>> non-greedy +? in the first capture group.
>
> Yes, that would work. Alternately: /^([^\s:]+)\s*:\s*(.+?)$/
I will use the non-greedy +? because the resulting expression is easier to read.
I will remove the non-greedy +? from the second capture group. It
serves no purpose there any more. It had been there to allow matching
a trailing backslash after the group, but now lines with trailing
backslash are ignored entirely before reaching here.
--
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 v5 1/1] send-email: Add sendmail email aliases format
On Tue, May 26, 2015 at 3:41 PM, Allen Hubbe wrote:
> On Tue, May 26, 2015 at 3:10 PM, Eric Sunshine
> wrote:
>> On Saturday, May 23, 2015, Allen Hubbe wrote:
>>> diff --git a/git-send-email.perl b/git-send-email.perl
>>> index e1e9b1460ced..ffea50094a48 100755
>>> --- a/git-send-email.perl
>>> +++ b/git-send-email.perl
>>> @@ -516,6 +518,33 @@ my %parse_alias = (
>>> }
>>> } },
>>>
>>> + sendmail => sub { my $fh = shift; while (<$fh>) {
>>> + # ignore comment lines
>>> + if (/^\s*(?:#.*)?$/) { }
>>
>> This confused me at first because the comment talks only about
>> "comment lines", for which a simpler /^\s*#/ would suffice. The regex,
>> however, actually matches blank lines and comment lines (both of which
>> get skipped). Either the comment should be fixed or the regex could be
>> split into two much simpler ones. The splitting into simpler regex's
>> has the benefit of being easier to comprehend at a glance. For
>> instance:
>>
>> next if /^\s*$/;
>> next if /^\s*#/;
>
> I noticed this too after sending the patch, and I have already changed
> the comment to mention blank lines or comment lines.
>
> Splitting the regex would be more simple, but the regex is already
> quite simple as it is.
To be clear, the reason that I brought up the idea of splitting the
regex was that /^\s*$/ and /^\s*#/ are very common idioms which people
can and do recognize and understand at-a-glance without having to
spend time deciphering them. On the other hand, /^\s*(?:#.*)?$/
doesn't lend itself to that sort of instant comprehension; it requires
a certain amount of mental effort to understand.
Anyhow, it's just an idea put forth in case you or someone favors it;
not an outright request for a change.
>>> + # recognize lines that look like an alias
>>> + elsif (/^(\S+)\s*:\s*(.+?)$/) {
>>
>> Observation: Given "foo:bar:baz", this regex will take "foo:bar" as
>> the key, and "baz" as the value, which is probably not what was
>> intended, however, it likely doesn't matter much in this case since
>> colon isn't legal in an email address[1].
>
> That's a keen observation. I think it would work simply to use a
> non-greedy +? in the first capture group.
Yes, that would work. Alternately: /^([^\s:]+)\s*:\s*(.+?)$/
--
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 v5 1/1] send-email: Add sendmail email aliases format
On Tue, May 26, 2015 at 3:10 PM, Eric Sunshine wrote:
> On Saturday, May 23, 2015, Allen Hubbe wrote:
>> Note that this only adds support for a limited subset of the sendmail
>> format. The format is is as follows.
>>
>> : [, ...]
>>
>> Aliases are specified one per line, and must start on the first column of the
>> line. Blank lines are ignored. If the first non whitespace character
>> on a line is a '#' symbol, then the whole line is considered a comment,
>> and is ignored.
>> [...]
>> Signed-off-by: Allen Hubbe
>> ---
>>
>> Notes:
>> This v5 renames the parser 'sendmail' again, from 'simple'.
>> Therefore, the subject line is changed again, too.
>>
>> Previous subject line: send-email: Add simple email aliases format
>>
>> The format is restricted to a subset of sendmail. When the subset
>> diverges from sendmail, the parser warns about the line that diverges,
>> and ignores the line. The supported format is described in the
>> documentation, as well as the behavior when an unsupported format
>> construct is detected.
>>
>> A badly constructed sentence was corrected in the documentation.
>>
>> The test case was changed to use a here document, and the unsupported
>> comment after an alias was removed from the test case alias file input.
>
> Thanks. This round looks much nicer. A few minor comments below...
>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index e1e9b1460ced..ffea50094a48 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -487,6 +487,8 @@ sub split_addrs {
>> }
>>
>> my %aliases;
>> +
>> +
>
> Unnecessary whitespace change sneaked in.
>
>> my %parse_alias = (
>> # multiline formats can be supported in the future
>> mutt => sub { my $fh = shift; while (<$fh>) {
>> @@ -516,6 +518,33 @@ my %parse_alias = (
>> }
>> } },
>>
>> + sendmail => sub { my $fh = shift; while (<$fh>) {
>> + # ignore comment lines
>> + if (/^\s*(?:#.*)?$/) { }
>
> This confused me at first because the comment talks only about
> "comment lines", for which a simpler /^\s*#/ would suffice. The regex,
> however, actually matches blank lines and comment lines (both of which
> get skipped). Either the comment should be fixed or the regex could be
> split into two much simpler ones. The splitting into simpler regex's
> has the benefit of being easier to comprehend at a glance. For
> instance:
>
> next if /^\s*$/;
> next if /^\s*#/;
I noticed this too after sending the patch, and I have already changed
the comment to mention blank lines or comment lines.
Splitting the regex would be more simple, but the regex is already
quite simple as it is.
>
> Speaking of 'next', its use here is inconsistent. Due to use of the
> if/elsif/else chain, 'next' is not needed at all, yet it is used for
> some cases but not others. To be consistent, either use it everywhere
> or nowhere.
These used to be `if (foo) { somthing; next; }` while this version was
work in progress, which I changed to elsif with the intention of
removing the next. Thanks for catching the inconsistency. I will
remove the next.
>
>> + # warn on lines that contain quotes
>> + elsif (/"/) {
>> + print STDERR "sendmail alias with quotes is not
>> supported: $_\n";
>> + next;
>> + }
>> +
>> + # warn on lines that continue
>> + elsif (/^\s|\\$/) {
>> + print STDERR "sendmail continuation line is not
>> supported: $_\n";
>> + next;
>> + }
>> +
>> + # recognize lines that look like an alias
>> + elsif (/^(\S+)\s*:\s*(.+?)$/) {
>
> Observation: Given "foo:bar:baz", this regex will take "foo:bar" as
> the key, and "baz" as the value, which is probably not what was
> intended, however, it likely doesn't matter much in this case since
> colon isn't legal in an email address[1].
That's a keen observation. I think it would work simply to use a
non-greedy +? in the first capture group.
>
> [1]: However, I could have sworn that colon was legal in some type of
> email address years ago, but I can no longer remember which type it
> was. UUCP used '!' in email addresses, so that wasn't it.
>
>> + my ($alias, $addr) = ($1, $2);
>> + $aliases{$alias} = [ split_addrs($addr) ];
>> + }
>> +
>> + # warn on lines that are not recognized
>> + else {
>> + print STDERR "sendmail line is not recognized: $_\n";
>> + }}},
>> +
>> gnus => sub { my $fh = shift; while (<$fh>) {
>> if (/\(define-mail-alias\s+"(\S+?)"\s+"(\S+?)"\)/) {
>> $aliases{$1} = [ $2 ];
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body
Re: [PATCH v5 1/1] send-email: Add sendmail email aliases format
On Saturday, May 23, 2015, Allen Hubbe wrote:
> Note that this only adds support for a limited subset of the sendmail
> format. The format is is as follows.
>
> : [, ...]
>
> Aliases are specified one per line, and must start on the first column of the
> line. Blank lines are ignored. If the first non whitespace character
> on a line is a '#' symbol, then the whole line is considered a comment,
> and is ignored.
> [...]
> Signed-off-by: Allen Hubbe
> ---
>
> Notes:
> This v5 renames the parser 'sendmail' again, from 'simple'.
> Therefore, the subject line is changed again, too.
>
> Previous subject line: send-email: Add simple email aliases format
>
> The format is restricted to a subset of sendmail. When the subset
> diverges from sendmail, the parser warns about the line that diverges,
> and ignores the line. The supported format is described in the
> documentation, as well as the behavior when an unsupported format
> construct is detected.
>
> A badly constructed sentence was corrected in the documentation.
>
> The test case was changed to use a here document, and the unsupported
> comment after an alias was removed from the test case alias file input.
Thanks. This round looks much nicer. A few minor comments below...
> diff --git a/git-send-email.perl b/git-send-email.perl
> index e1e9b1460ced..ffea50094a48 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -487,6 +487,8 @@ sub split_addrs {
> }
>
> my %aliases;
> +
> +
Unnecessary whitespace change sneaked in.
> my %parse_alias = (
> # multiline formats can be supported in the future
> mutt => sub { my $fh = shift; while (<$fh>) {
> @@ -516,6 +518,33 @@ my %parse_alias = (
> }
> } },
>
> + sendmail => sub { my $fh = shift; while (<$fh>) {
> + # ignore comment lines
> + if (/^\s*(?:#.*)?$/) { }
This confused me at first because the comment talks only about
"comment lines", for which a simpler /^\s*#/ would suffice. The regex,
however, actually matches blank lines and comment lines (both of which
get skipped). Either the comment should be fixed or the regex could be
split into two much simpler ones. The splitting into simpler regex's
has the benefit of being easier to comprehend at a glance. For
instance:
next if /^\s*$/;
next if /^\s*#/;
Speaking of 'next', its use here is inconsistent. Due to use of the
if/elsif/else chain, 'next' is not needed at all, yet it is used for
some cases but not others. To be consistent, either use it everywhere
or nowhere.
> + # warn on lines that contain quotes
> + elsif (/"/) {
> + print STDERR "sendmail alias with quotes is not
> supported: $_\n";
> + next;
> + }
> +
> + # warn on lines that continue
> + elsif (/^\s|\\$/) {
> + print STDERR "sendmail continuation line is not
> supported: $_\n";
> + next;
> + }
> +
> + # recognize lines that look like an alias
> + elsif (/^(\S+)\s*:\s*(.+?)$/) {
Observation: Given "foo:bar:baz", this regex will take "foo:bar" as
the key, and "baz" as the value, which is probably not what was
intended, however, it likely doesn't matter much in this case since
colon isn't legal in an email address[1].
[1]: However, I could have sworn that colon was legal in some type of
email address years ago, but I can no longer remember which type it
was. UUCP used '!' in email addresses, so that wasn't it.
> + my ($alias, $addr) = ($1, $2);
> + $aliases{$alias} = [ split_addrs($addr) ];
> + }
> +
> + # warn on lines that are not recognized
> + else {
> + print STDERR "sendmail line is not recognized: $_\n";
> + }}},
> +
> gnus => sub { my $fh = shift; while (<$fh>) {
> if (/\(define-mail-alias\s+"(\S+?)"\s+"(\S+?)"\)/) {
> $aliases{$1} = [ $2 ];
--
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 v5 1/1] send-email: Add sendmail email aliases format
On Sat, May 23, 2015 at 1:45 PM, Junio C Hamano wrote: > Allen Hubbe writes: >> Note that this only adds support for a limited subset of the sendmail >> format. The format is is as follows. >> >> : [, ...] >> >> Aliases are specified one per line, and must start on the first column of the >> line. Blank lines are ignored. If the first non whitespace character >> on a line is a '#' symbol, then the whole line is considered a comment, >> and is ignored. >> [...] >> Signed-off-by: Allen Hubbe >> --- > > Thanks. > > A small thing I noticed in the test (and this patch is not adding a > new "breakage"---there are a few existing instances) is the use of > "~/"; it should be spelled "$HOME/" instead for portability (not in > POSIX, even though bash, dash and ksh all seem to understand it). > > I think this round looks good from a cursory read. > > Eric, what do you think? Sorry for the delay. This round looks much better. I do have a few very minor comments, which I'll post in reply to the patch itself, but nothing worth holding the series up. Thanks. -- 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 v5 1/1] send-email: Add sendmail email aliases format
Junio C Hamano writes: > It probably should be two patches. Your sendmail thing with docs > and tests as one patch (with $HOME in test), and fix to mailrc tests > I did (minus the part that fixes your sendmail test, which should > now become unnecessary) on top. > > If the documentation I queued on 'pu' formats well already (which I > cannot check myself until tomorrow), then I'd guess the above would > be like squashing 8b8fb5a into dc6183c and then 6b733ee on top, I > think. > > 6b733ee t9001: write $HOME/, not ~/, to help shells without tilde expansion > 8b8fb5a git-send-email doc: refer to upstream document for alias format > dc6183c send-email: add sendmail email aliases format Well, I lied [*1*]. I think the documentation part of what is in 'pu' formats fine, so let me just clean them up and push the result out for your final review. Give me a few hours (leaving time for dinner and etc., too). [Footnote] *1* My Git time is spent on in a terminal-only environment, a virtual machine running somewhere in Google datacenters, and when I am home working from a Chromebook via ssh, I lack a convenient way to grab a single file out of there to view in the browser locally. The virtual machine does let me upload to Google Drive and I can grab a file from there to my Chromebook, and that is what I did to see what the AsciiDoc formatted result looked like just now ;-) -- 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 v5 1/1] send-email: Add sendmail email aliases format
Allen Hubbe writes: >> Could you fetch from me and then run: >> >> $ git log --reverse -3 -p 6b733ee4ba330e1187017895b8426dd9171c33b8 >> >> to see if you agree with the result? That is what I queued on 'pu' >> for now with my fixups. > > It looks good to me. How would you like me to proceed? I assume you > would like your patch on top of mine will stay, to use HOME instead of > tilde. Or, would you like me to use HOME in my v6, too? > > Should I send you v6 like v5, but with the documentation fixed, or > would you now prefer a separate patch on top of that to fix the > documentation? It probably should be two patches. Your sendmail thing with docs and tests as one patch (with $HOME in test), and fix to mailrc tests I did (minus the part that fixes your sendmail test, which should now become unnecessary) on top. If the documentation I queued on 'pu' formats well already (which I cannot check myself until tomorrow), then I'd guess the above would be like squashing 8b8fb5a into dc6183c and then 6b733ee on top, I think. 6b733ee t9001: write $HOME/, not ~/, to help shells without tilde expansion 8b8fb5a git-send-email doc: refer to upstream document for alias format dc6183c send-email: add sendmail email aliases format -- 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 v5 1/1] send-email: Add sendmail email aliases format
On Mon, May 25, 2015 at 9:58 PM, Junio C Hamano wrote: > Allen Hubbe writes: > >> Thanks for letting me know. Are you still expecting v6 from me then? >> The other thing you asked for was a change in the documentation: just >> mention the email programs' documentation, and describe the >> exceptions. > > Could you fetch from me and then run: > > $ git log --reverse -3 -p 6b733ee4ba330e1187017895b8426dd9171c33b8 > > to see if you agree with the result? That is what I queued on 'pu' > for now with my fixups. It looks good to me. How would you like me to proceed? I assume you would like your patch on top of mine will stay, to use HOME instead of tilde. Or, would you like me to use HOME in my v6, too? Should I send you v6 like v5, but with the documentation fixed, or would you now prefer a separate patch on top of that to fix the documentation? I can do either, and you would be welcome to rebase/fixup the second patch into the earlier one with my sign off. > > We have not heard from Eric on this round yet, so he (and others) > may have further input, but as far as I am concerned, that one > looked more or less ready to be merged down to 'next', except for > the documentation part, which I haven't had a chance to look at the > results and may need further AsciiDoc mark-up fixes. > > Thanks. -- 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 v5 1/1] send-email: Add sendmail email aliases format
Allen Hubbe writes: > Thanks for letting me know. Are you still expecting v6 from me then? > The other thing you asked for was a change in the documentation: just > mention the email programs' documentation, and describe the > exceptions. Could you fetch from me and then run: $ git log --reverse -3 -p 6b733ee4ba330e1187017895b8426dd9171c33b8 to see if you agree with the result? That is what I queued on 'pu' for now with my fixups. We have not heard from Eric on this round yet, so he (and others) may have further input, but as far as I am concerned, that one looked more or less ready to be merged down to 'next', except for the documentation part, which I haven't had a chance to look at the results and may need further AsciiDoc mark-up fixes. Thanks. -- 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 v5 1/1] send-email: Add sendmail email aliases format
On Mon, May 25, 2015 at 5:35 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >> Junio C Hamano writes: >> >> FYI, I have tentatively queued this on top of your patch. Please >> see "git log master..cf954075" to double check. > > Sorry but I had a typo there... > >> git format-patch -1 -o outdir && >> - cat >>~/.tmp-email-aliases <<-\EOF && >> + cat >>./.tmp-email-aliases" <<-\EOF && > > This should just be > > cat >>.tmp-email-aliases <<-\EOF && > Thanks for letting me know. Are you still expecting v6 from me then? The other thing you asked for was a change in the documentation: just mention the email programs' documentation, and describe the exceptions. -- 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 v5 1/1] send-email: Add sendmail email aliases format
Junio C Hamano writes: > Junio C Hamano writes: > > FYI, I have tentatively queued this on top of your patch. Please > see "git log master..cf954075" to double check. Sorry but I had a typo there... > git format-patch -1 -o outdir && > - cat >>~/.tmp-email-aliases <<-\EOF && > + cat >>./.tmp-email-aliases" <<-\EOF && This should just be cat >>.tmp-email-aliases <<-\EOF && -- 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 v5 1/1] send-email: Add sendmail email aliases format
Junio C Hamano writes: > Allen Hubbe writes: > >> Looking closer at this and the other test cases, they are inconsistent >> about using ".mailrc", "~/.mailrc", and "$(pwd)/.mailrc". This would >> add another one, "$HOME/.mailrc". > > In t9001, I see two tests on mailrc: > ... > So I do not see any reason to change most of these; except that the > target of 'echo' should be changed from ~/.mailrc to $HOME/.mailrc. FYI, I have tentatively queued this on top of your patch. Please see "git log master..cf954075" to double check. Thanks. -- >8 -- Subject: [PATCH] t9001: write $HOME/, not ~/, to help shells without tilde expansion Even though it is in POSIX, we do not have to use it, only to hurt shells that may lack the support. The .mailrc test tries to define an alias in .mailrc in the home directory by shell redirection, and then tries to see ~/.mailrc in config is tilde-expanded by Git without help from shell. So the creation should become $HOME/ to be portable for shells that may lack tilde expansion but the reference should be done as "~/.mailrc". The sendmail one refers to the file from the configuration with full path, so it does not need to know that $HOME during the test run is set to the current "trash" directory. Signed-off-by: Junio C Hamano --- t/t9001-send-email.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index b04d263..c5c6867 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1537,7 +1537,7 @@ test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' ' test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' ' clean_fake_sendmail && - echo "alias sbd some...@example.org" >~/.mailrc && + echo "alias sbd some...@example.org" >"$HOME/.mailrc" && git config --replace-all sendemail.aliasesfile "~/.mailrc" && git config sendemail.aliasfiletype mailrc && git send-email \ @@ -1552,7 +1552,7 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' ' test_expect_success $PREREQ 'sendemail.aliasfiletype=sendmail' ' clean_fake_sendmail && rm -fr outdir && git format-patch -1 -o outdir && - cat >>~/.tmp-email-aliases <<-\EOF && + cat >>./.tmp-email-aliases" <<-\EOF && alice: Alice W Land bob: Robert Bobbyton # this is a comment -- 2.4.1-455-ga49e496 -- 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 v5 1/1] send-email: Add sendmail email aliases format
Allen Hubbe writes: > Looking closer at this and the other test cases, they are inconsistent > about using ".mailrc", "~/.mailrc", and "$(pwd)/.mailrc". This would > add another one, "$HOME/.mailrc". In t9001, I see two tests on mailrc: * Create .mailrc in the current directory and point at it from the configuration file sendemail.aliasfile with $(pwd)/.mailrc This one is correct in that test wants to make sure an absolute path is usable as the value; the creation in the current directory (hence >.mailrc) is a mere implementation detail that the file it wants to use can be created by pathname relative to the current directory when "echo" is run. * Create ~/.mailrc, relying on tilde expansion of the shell when "echo" is run, and then point at it from the configuration file as "~/.mailrc". The former, i.e. "echo ... >~/.mailrc" should instead redirect into >$HOME/.mailrc in order to support shells that do not understand tilde expansion. However, the latter is correct, as this test wants to make sure that whoever reads the configuration interprets ~/.mailrc as "file .mailrc in the home directory", without help from the shell. Specifically, the difference between these two tests is not inconcistency; they are covering two different use patterns. So I do not see any reason to change most of these; except that the target of 'echo' should be changed from ~/.mailrc to $HOME/.mailrc. Thanks. -- 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 v5 1/1] send-email: Add sendmail email aliases format
On Sat, May 23, 2015 at 6:24 PM, Allen Hubbe wrote: > On Sat, May 23, 2015 at 2:07 PM, Junio C Hamano wrote: >> Junio C Hamano writes: >> >>> A small thing I noticed in the test (and this patch is not adding a >>> new "breakage"---there are a few existing instances) is the use of >>> "~/"; it should be spelled "$HOME/" instead for portability (not in >>> POSIX, even though bash, dash and ksh all seem to understand it). >> >> Well, I was wrong. Tilde expansion is in POSIX. >> >> Nevertheless, I'd prefer if we avoided it. > > Alright, $HOME it will be. Looking closer at this and the other test cases, they are inconsistent about using ".mailrc", "~/.mailrc", and "$(pwd)/.mailrc". This would add another one, "$HOME/.mailrc". How do you feel about using just ".mailrc", and "$(pwd)/.mailrc" when an absolute path is needed in gitconfig? -- 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 v5 1/1] send-email: Add sendmail email aliases format
On Sat, May 23, 2015 at 2:00 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >>> diff --git a/Documentation/git-send-email.txt >>> b/Documentation/git-send-email.txt >>> index 804554609def..97387fd27a8d 100644 >>> --- a/Documentation/git-send-email.txt >>> +++ b/Documentation/git-send-email.txt >>> @@ -383,7 +383,42 @@ 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 'sendmail', 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'. > > We prefer to append to an existing list of equals, not prepend. > I initially thought to put it last, too. I'll move it back to the end. I moved it to the beginning, because it seemed odd to me for only the last thing in the list to have a further description. If the intent is that eventually (perhaps in an ideal world), the other formats will have expanded documentation, too, then you are right that adding new things to the end makes the most sense. >>> ++ >>> +If the format is 'sendmail', then the alias file format is described below. >>> +Descriptions of the other file formats can be found by searching the >>> +documentation of the email program of the same name. > > The phrasing feels somewhat awkward. How about dropping the first > line, pretending as if 'sendmail' is also fully 'sendmail' format, > and then describe the limitations (like you already did below)? I > have a feeling that other formats have similar limitations (e.g. I > do not think piping to commands in any other formats would be > supported by send-email), and other people can follow suit and > describe the limitations. > > That is, drop the paragraph that describes the basics (which can be > learned by searching the documentation of the email program of the > same name), and dive right into the differences. > > IOW, > > What an alias file in each format looks like can be found in > the documentation of the email program of the same name. The > differences and limitations from the standard formats are > described below: > + > -- > sendmail;; >>> +* Quoted aliases and quoted addresses are not supported: lines that >>> +contain a `"` symbol are ignored. >>> +* Line continuations are not supported: any lines that start with >>> +whitespace, or end with a `\` symbol are ignored. >>> +* Warnings are printed on the standard error output for any explicitly >>> +unsupported constructs, and any other lines that are not recognized >>> +by the parser. > -- Alright. Thanks for showing me '--'. I had some trouble with asciidoc, where my intention was to insert a bulleted list between two paragraphs in a containing definition-list item. The paragraph that I intended to be after the bulleted list was instead nested in the last bulleted item in the list. The documentation for asciidoc soesn't seem to be very helpful in describing this construct. There is one example, that I could find, and I didn't find a description of the syntax for it. Perhaps I missed it among all the other uses of a series of '-'. I don't see any way for this to distinguish between different levels of nesting, like a block of --/-- in another block of --/--; that might be syntactically indistinguishable from a block of --/-- followed by another block of --/--. > > That way, limitations and deviations of other formats can be added > later in a consistent way. > > Just a thought. -- 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 v5 1/1] send-email: Add sendmail email aliases format
On Sat, May 23, 2015 at 2:07 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >> A small thing I noticed in the test (and this patch is not adding a >> new "breakage"---there are a few existing instances) is the use of >> "~/"; it should be spelled "$HOME/" instead for portability (not in >> POSIX, even though bash, dash and ksh all seem to understand it). > > Well, I was wrong. Tilde expansion is in POSIX. > > Nevertheless, I'd prefer if we avoided it. Alright, $HOME it will be. -- 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 v5 1/1] send-email: Add sendmail email aliases format
Junio C Hamano writes: > A small thing I noticed in the test (and this patch is not adding a > new "breakage"---there are a few existing instances) is the use of > "~/"; it should be spelled "$HOME/" instead for portability (not in > POSIX, even though bash, dash and ksh all seem to understand it). Well, I was wrong. Tilde expansion is in POSIX. Nevertheless, I'd prefer if we avoided it. -- 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 v5 1/1] send-email: Add sendmail email aliases format
Junio C Hamano writes: >> diff --git a/Documentation/git-send-email.txt >> b/Documentation/git-send-email.txt >> index 804554609def..97387fd27a8d 100644 >> --- a/Documentation/git-send-email.txt >> +++ b/Documentation/git-send-email.txt >> @@ -383,7 +383,42 @@ 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 'sendmail', 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'. We prefer to append to an existing list of equals, not prepend. >> ++ >> +If the format is 'sendmail', then the alias file format is described below. >> +Descriptions of the other file formats can be found by searching the >> +documentation of the email program of the same name. The phrasing feels somewhat awkward. How about dropping the first line, pretending as if 'sendmail' is also fully 'sendmail' format, and then describe the limitations (like you already did below)? I have a feeling that other formats have similar limitations (e.g. I do not think piping to commands in any other formats would be supported by send-email), and other people can follow suit and describe the limitations. That is, drop the paragraph that describes the basics (which can be learned by searching the documentation of the email program of the same name), and dive right into the differences. IOW, What an alias file in each format looks like can be found in the documentation of the email program of the same name. The differences and limitations from the standard formats are described below: + -- sendmail;; >> +* Quoted aliases and quoted addresses are not supported: lines that >> +contain a `"` symbol are ignored. >> +* Line continuations are not supported: any lines that start with >> +whitespace, or end with a `\` symbol are ignored. >> +* Warnings are printed on the standard error output for any explicitly >> +unsupported constructs, and any other lines that are not recognized >> +by the parser. -- That way, limitations and deviations of other formats can be added later in a consistent way. Just a thought. -- 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 v5 1/1] send-email: Add sendmail email aliases format
Allen Hubbe writes:
> Note that this only adds support for a limited subset of the sendmail
> format. The format is is as follows.
>
> : [, ...]
>
> Aliases are specified one per line, and must start on the first column of the
> line. Blank lines are ignored. If the first non whitespace character
> on a line is a '#' symbol, then the whole line is considered a comment,
> and is ignored.
>
> Example:
>
> alice: Alice W Land
> bob: Robert Bobbyton
> # this is a comment
> # this is also a comment
> chloe: ch...@example.com
> abgroup: alice, bob
> bcgrp: bob, chloe, Other
>
> Unlike the standard sendmail format, this does not support quoted
> aliases or quoted addresses. Line continuations are not supported.
> Warnings are printed for explicitly unsupported constructs, and any
> other lines that are not recognized.
>
> Signed-off-by: Allen Hubbe
> ---
>
> Notes:
> This v5 renames the parser 'sendmail' again, from 'simple'.
> Therefore, the subject line is changed again, too.
>
> Previous subject line: send-email: Add simple email aliases format
>
> The format is restricted to a subset of sendmail. When the subset
> diverges from sendmail, the parser warns about the line that diverges,
> and ignores the line. The supported format is described in the
> documentation, as well as the behavior when an unsupported format
> construct is detected.
>
> A badly constructed sentence was corrected in the documentation.
>
> The test case was changed to use a here document, and the unsupported
> comment after an alias was removed from the test case alias file input.
Thanks.
A small thing I noticed in the test (and this patch is not adding a
new "breakage"---there are a few existing instances) is the use of
"~/"; it should be spelled "$HOME/" instead for portability (not in
POSIX, even though bash, dash and ksh all seem to understand it).
I think this round looks good from a cursory read.
Eric, what do you think?
> Documentation/git-send-email.txt | 37 -
> git-send-email.perl | 29 +
> t/t9001-send-email.sh| 27 +++
> 3 files changed, 92 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-send-email.txt
> b/Documentation/git-send-email.txt
> index 804554609def..97387fd27a8d 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -383,7 +383,42 @@ 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 'sendmail', 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
> ++
> +If the format is 'sendmail', then the alias file format is described below.
> +Descriptions of the other file formats can be found by searching the
> +documentation of the email program of the same name.
> ++
> +The 'sendmail' format is is as follows. Note that 'git-send-email' currently
> +only supports a limited subset of the sendmail format.
> ++
> + : [, ...]
> ++
> +Aliases are specified one per line, and must start on the first column of the
> +line. Blank lines are ignored. If the first non whitespace character on a
> +line is a `#` symbol, then the whole line is considered a comment, and is
> +ignored.
> ++
> +Example of the 'sendmail' format:
> ++
> + alice: Alice W Land
> + bob: Robert Bobbyton
> + # this is a comment
> +# this is also a comment
> + chloe: ch...@example.com
> + abgroup: alice, bob
> + bcgrp: bob, chloe, Other
> ++
> +Unlike the standard sendmail format, 'git-send-email' currently diverges in
> the
> +following ways.
> ++
> +*Quoted aliases and quoted addresses are not supported: lines that
> + contain a `"` symbol are ignored.
> +*Line continuations are not supported: any lines that start with
> + whitespace, or end with a `\` symbol are ignored.
> +*Warnings are printed on the standard error output for any explicitly
> + unsupported constructs, and any other lines that are not recognized
> + by the parser.
>
> 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..ffea50094a48 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -487,6 +487,8 @@ sub split_addrs {
> }
>
> my %aliases;
> +
> +
> my %parse_alias = (
> # multiline formats can be supported in the future
> mutt => sub { my $fh = shift; while (<$fh>) {
> @@ -516,6 +518,33 @@ my %parse_alias = (
> }
> } },
>
> + sendmail => sub { my $fh = shift; while (<$fh>) {
> + # ignore comment lines
> + if (/^\s*(?:#.*)?$/) { }
> +
> + # warn o
[PATCH v5 1/1] send-email: Add sendmail email aliases format
Note that this only adds support for a limited subset of the sendmail
format. The format is is as follows.
: [, ...]
Aliases are specified one per line, and must start on the first column of the
line. Blank lines are ignored. If the first non whitespace character
on a line is a '#' symbol, then the whole line is considered a comment,
and is ignored.
Example:
alice: Alice W Land
bob: Robert Bobbyton
# this is a comment
# this is also a comment
chloe: ch...@example.com
abgroup: alice, bob
bcgrp: bob, chloe, Other
Unlike the standard sendmail format, this does not support quoted
aliases or quoted addresses. Line continuations are not supported.
Warnings are printed for explicitly unsupported constructs, and any
other lines that are not recognized.
Signed-off-by: Allen Hubbe
---
Notes:
This v5 renames the parser 'sendmail' again, from 'simple'.
Therefore, the subject line is changed again, too.
Previous subject line: send-email: Add simple email aliases format
The format is restricted to a subset of sendmail. When the subset
diverges from sendmail, the parser warns about the line that diverges,
and ignores the line. The supported format is described in the
documentation, as well as the behavior when an unsupported format
construct is detected.
A badly constructed sentence was corrected in the documentation.
The test case was changed to use a here document, and the unsupported
comment after an alias was removed from the test case alias file input.
Documentation/git-send-email.txt | 37 -
git-send-email.perl | 29 +
t/t9001-send-email.sh| 27 +++
3 files changed, 92 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 804554609def..97387fd27a8d 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -383,7 +383,42 @@ 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 'sendmail', 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
++
+If the format is 'sendmail', then the alias file format is described below.
+Descriptions of the other file formats can be found by searching the
+documentation of the email program of the same name.
++
+The 'sendmail' format is is as follows. Note that 'git-send-email' currently
+only supports a limited subset of the sendmail format.
++
+ : [, ...]
++
+Aliases are specified one per line, and must start on the first column of the
+line. Blank lines are ignored. If the first non whitespace character on a
+line is a `#` symbol, then the whole line is considered a comment, and is
+ignored.
++
+Example of the 'sendmail' format:
++
+ alice: Alice W Land
+ bob: Robert Bobbyton
+ # this is a comment
+ # this is also a comment
+ chloe: ch...@example.com
+ abgroup: alice, bob
+ bcgrp: bob, chloe, Other
++
+Unlike the standard sendmail format, 'git-send-email' currently diverges in the
+following ways.
++
+* Quoted aliases and quoted addresses are not supported: lines that
+ contain a `"` symbol are ignored.
+* Line continuations are not supported: any lines that start with
+ whitespace, or end with a `\` symbol are ignored.
+* Warnings are printed on the standard error output for any explicitly
+ unsupported constructs, and any other lines that are not recognized
+ by the parser.
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..ffea50094a48 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -487,6 +487,8 @@ sub split_addrs {
}
my %aliases;
+
+
my %parse_alias = (
# multiline formats can be supported in the future
mutt => sub { my $fh = shift; while (<$fh>) {
@@ -516,6 +518,33 @@ my %parse_alias = (
}
} },
+ sendmail => sub { my $fh = shift; while (<$fh>) {
+ # ignore comment lines
+ if (/^\s*(?:#.*)?$/) { }
+
+ # warn on lines that contain quotes
+ elsif (/"/) {
+ print STDERR "sendmail alias with quotes is not
supported: $_\n";
+ next;
+ }
+
+ # warn on lines that continue
+ elsif (/^\s|\\$/) {
+ print STDERR "sendmail continuation line is not
supported: $_\n";
+ next;
+ }
+
+ # recognize lines that look like an alias
+ elsif (/^(\S+)\s*:\s*(.+?)$/) {
+
