git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUG] send-email: alias expansion broken
@ 2011-10-14 12:29 Michael J Gruber
  2011-10-14 14:25 ` [PATCH] fix alias expansion with new Git::config_path() Cord Seele
  0 siblings, 1 reply; 11+ messages in thread
From: Michael J Gruber @ 2011-10-14 12:29 UTC (permalink / raw)
  To: Git Mailing List, Cord Seele

cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)

broke the expansion of aliases for me:

./git-send-email --cc=junio  --dry-run
0001-t7800-avoid-arithmetic-expansion-notation.patch
0001-t7800-avoid-arithmetic-expansion-notation.patch
Who should the emails appear to be from? [Michael J Gruber
<git@drmicha.warpmail.net>]
Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>
Dry-OK. Log says:
Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio
git@drmicha.warpmail.net
From: Michael J Gruber <git@drmicha.warpmail.net>
To: git@vger.kernel.org
Cc: junio
...

Happens with both "--cc junio" and "--cc=junio".

Reverting cec5dae brings my aliases back. Relevant config:

git config --get-regexp sendemail.alias\*
sendemail.aliasesfile /home/mjg/git/gitauthors
sendemail.aliasfiletype mutt

Can I please have alias expansion back?

No, I don't know what cec5dae tries to achieve, and I lack the perl fu
to fix it.

Michael

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] fix alias expansion with new Git::config_path()
  2011-10-14 12:29 [BUG] send-email: alias expansion broken Michael J Gruber
@ 2011-10-14 14:25 ` Cord Seele
  2011-10-14 14:30   ` Michael J Gruber
  0 siblings, 1 reply; 11+ messages in thread
From: Cord Seele @ 2011-10-14 14:25 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List, Junio C Hamano, Jakub Narebski

On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:

> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
> 
> broke the expansion of aliases for me:
> 
> ./git-send-email --cc=junio  --dry-run
> 0001-t7800-avoid-arithmetic-expansion-notation.patch
> 0001-t7800-avoid-arithmetic-expansion-notation.patch
> Who should the emails appear to be from? [Michael J Gruber
> <git@drmicha.warpmail.net>]
> Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>
> Dry-OK. Log says:
> Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio
> git@drmicha.warpmail.net
> From: Michael J Gruber <git@drmicha.warpmail.net>
> To: git@vger.kernel.org
> Cc: junio
> ...
> 
> Happens with both "--cc junio" and "--cc=junio".
> 
> Reverting cec5dae brings my aliases back. Relevant config:
> 
> git config --get-regexp sendemail.alias\*
> sendemail.aliasesfile /home/mjg/git/gitauthors
> sendemail.aliasfiletype mutt
> 
> Can I please have alias expansion back?

The following patch fixes it for me, please give it a try.

Since this fix is simply copy&pasting some code from the config_settings path
someone with better perl understanding might wnat to refactor it
(Junio/Jacob)?

-- Cord


Signed-off-by: Cord Seele <cowose@gmail.com>
---
 git-send-email.perl |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 91607c5..6885dfa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -337,8 +337,16 @@ sub read_config {
 	}
 
 	foreach my $setting (keys %config_path_settings) {
-		my $target = $config_path_settings{$setting}->[0];
-		$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+		my $target = $config_path_settings{$setting};
+		if (ref($target) eq "ARRAY") {
+			unless (@$target) {
+				my @values = Git::config_path(@repo, "$prefix.$setting");
+				@$target = @values if (@values && defined $values[0]);
+			}
+		}
+		else {
+			$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+		}
 	}
 
 	foreach my $setting (keys %config_settings) {
-- 
1.7.6.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] fix alias expansion with new Git::config_path()
  2011-10-14 14:25 ` [PATCH] fix alias expansion with new Git::config_path() Cord Seele
@ 2011-10-14 14:30   ` Michael J Gruber
  2011-10-14 14:42     ` Cord Seele
  2011-10-14 16:38     ` Jakub Narebski
  0 siblings, 2 replies; 11+ messages in thread
From: Michael J Gruber @ 2011-10-14 14:30 UTC (permalink / raw)
  To: Git Mailing List, Junio C Hamano, Jakub Narebski

Cord Seele venit, vidit, dixit 14.10.2011 16:25:
> On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
> 
>> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
>>
>> broke the expansion of aliases for me:
>>
>> ./git-send-email --cc=junio  --dry-run
>> 0001-t7800-avoid-arithmetic-expansion-notation.patch
>> 0001-t7800-avoid-arithmetic-expansion-notation.patch
>> Who should the emails appear to be from? [Michael J Gruber
>> <git@drmicha.warpmail.net>]
>> Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>
>> Dry-OK. Log says:
>> Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio
>> git@drmicha.warpmail.net
>> From: Michael J Gruber <git@drmicha.warpmail.net>
>> To: git@vger.kernel.org
>> Cc: junio
>> ...
>>
>> Happens with both "--cc junio" and "--cc=junio".
>>
>> Reverting cec5dae brings my aliases back. Relevant config:
>>
>> git config --get-regexp sendemail.alias\*
>> sendemail.aliasesfile /home/mjg/git/gitauthors
>> sendemail.aliasfiletype mutt
>>
>> Can I please have alias expansion back?
> 
> The following patch fixes it for me, please give it a try.
> 
> Since this fix is simply copy&pasting some code from the config_settings path
> someone with better perl understanding might wnat to refactor it
> (Junio/Jacob)?
> 
> -- Cord
> 
> 
> Signed-off-by: Cord Seele <cowose@gmail.com>

Tested-by: Michael J Gruber <git@drmicha.warpmail.net>


Thanks. (Though I'm still wondering what this is about overall.)


> ---
>  git-send-email.perl |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 91607c5..6885dfa 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -337,8 +337,16 @@ sub read_config {
>  	}
>  
>  	foreach my $setting (keys %config_path_settings) {
> -		my $target = $config_path_settings{$setting}->[0];
> -		$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
> +		my $target = $config_path_settings{$setting};
> +		if (ref($target) eq "ARRAY") {
> +			unless (@$target) {
> +				my @values = Git::config_path(@repo, "$prefix.$setting");
> +				@$target = @values if (@values && defined $values[0]);
> +			}
> +		}
> +		else {
> +			$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
> +		}
>  	}
>  
>  	foreach my $setting (keys %config_settings) {

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fix alias expansion with new Git::config_path()
  2011-10-14 14:30   ` Michael J Gruber
@ 2011-10-14 14:42     ` Cord Seele
  2011-10-14 15:05       ` Junio C Hamano
  2011-10-14 16:38     ` Jakub Narebski
  1 sibling, 1 reply; 11+ messages in thread
From: Cord Seele @ 2011-10-14 14:42 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List, Junio C Hamano, Jakub Narebski

On Fri 14 Oct 2011 16:30:25 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:

> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>

Great.

> Thanks. (Though I'm still wondering what this is about overall.)

to make '~/' work in sendemail.aliasesfile

-- Cord

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fix alias expansion with new Git::config_path()
  2011-10-14 14:42     ` Cord Seele
@ 2011-10-14 15:05       ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2011-10-14 15:05 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List, Jakub Narebski

Cord Seele <cowose@googlemail.com> writes:

> On Fri 14 Oct 2011 16:30:25 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
>
>> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
>
> Great.
>
>> Thanks. (Though I'm still wondering what this is about overall.)
>
> to make '~/' work in sendemail.aliasesfile

That is not an explanation.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] fix alias expansion with new Git::config_path()
  2011-10-14 14:30   ` Michael J Gruber
  2011-10-14 14:42     ` Cord Seele
@ 2011-10-14 16:38     ` Jakub Narebski
  2011-10-14 18:13       ` Junio C Hamano
  1 sibling, 1 reply; 11+ messages in thread
From: Jakub Narebski @ 2011-10-14 16:38 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List, Junio C Hamano

Michael J Gruber wrote:
> Cord Seele venit, vidit, dixit 14.10.2011 16:25:
>> On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
>> 
>>> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
>>> broke the expansion of aliases for me:
[...]
>>> Reverting cec5dae brings my aliases back. [...]
[...]
>> 
>> The following patch fixes it for me, please give it a try.
>> 
>> Since this fix is simply copy&pasting some code from the config_settings path
>> someone with better perl understanding might wnat to refactor it
>> (Junio/Jacob)?

[missing commit message]

>> Signed-off-by: Cord Seele <cowose@gmail.com>
> 
> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
> 
> Thanks. (Though I'm still wondering what this is about overall.)

There were a few issues that were responsible for this error:

1. %config_bool_settings and %config_settings despite similar name have
   different semantic.  

   %config_bool_settings values are arrays where the first element is
   (reference to) the variable to set, and second element is default
   value... which admittedly is a bit cryptic.  More readable if more
   verbose option would be to use hash reference, e.g.:

	my %config_bool_settings = (
	    "thread" => { variable => \$thread, default => 1},
	    [...]

   Or something like that.

   %config_settings values are either either reference to scalar variable
   or reference to array.  In second case it means that option (or config
   option) is multi-valued.  BTW. this is similar to what Getopt::Long does.

2. In cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
   the setting "aliasesfile" was moved from %config_settings to newly
   introduced %config_path_settings.  But the loop that parses settings
   from %config_path_settings was copy'n'pasted *wrongly* from
   %config_bool_settings instead of from %config_settings.

   It looks like cec5dae author cargo-culted this change...

3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
   didn't add test for alias expansion to t9001-send-email.sh

>> ---
>>  git-send-email.perl |   12 ++++++++++--
>>  1 files changed, 10 insertions(+), 2 deletions(-)
>> 
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 91607c5..6885dfa 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -337,8 +337,16 @@ sub read_config {
>>  	}
>>  
>>  	foreach my $setting (keys %config_path_settings) {
>> -		my $target = $config_path_settings{$setting}->[0];
>> -		$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
>> +		my $target = $config_path_settings{$setting};
>> +		if (ref($target) eq "ARRAY") {
>> +			unless (@$target) {
>> +				my @values = Git::config_path(@repo, "$prefix.$setting");
>> +				@$target = @values if (@values && defined $values[0]);
>> +			}
>> +		}
>> +		else {
>> +			$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
>> +		}
>>  	}
>>  
>>  	foreach my $setting (keys %config_settings) {

Or a bit simpler (though still duplicated somewhat code with
%config_settings) case:

diff --git i/git-send-email.perl w/git-send-email.perl
index 91607c5..eed241e 100755
--- i/git-send-email.perl
+++ w/git-send-email.perl
@@ -337,8 +337,13 @@ sub read_config {
 	}
 
 	foreach my $setting (keys %config_path_settings) {
-		my $target = $config_path_settings{$setting}->[0];
-		$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+		my $target = $config_path_settings{$setting};
+		if (ref($target) eq "ARRAY" && !@$target) {
+			my @values = Git::config_path(@repo, "$prefix.$setting");
+			@$target = @values if (@values && defined $values[0]);
+		} elsif (!defined $$target) {
+			$$target = Git::config_path(@repo, "$prefix.$setting");
+		}
 	}
 
 	foreach my $setting (keys %config_settings) {

P.S. Junio, does t9001 pass for you?  For me it fails very strangely on
some tests: 

  not ok - 21 reject long lines
  not ok - 22 no patch was sent
  not ok - 28 In-Reply-To without --chain-reply-to
  not ok - 29 In-Reply-To with --chain-reply-to
  not ok - 39 sendemail.cccmd
  not ok - 49 --suppress-cc=bodycc
  not ok - 51 --suppress-cc=cc
  not ok - 56 confirm by default (due to cc)
  not ok - 70 warning with an implicit --chain-reply-to
  # failed 9 among 93 test(s)
-- 
Jakub Narebski
Poland

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] fix alias expansion with new Git::config_path()
  2011-10-14 16:38     ` Jakub Narebski
@ 2011-10-14 18:13       ` Junio C Hamano
  2011-10-14 18:49         ` [PATCH] send-email: Fix %config_path_settings handling Jakub Narebski
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2011-10-14 18:13 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Michael J Gruber, Git Mailing List

Jakub Narebski <jnareb@gmail.com> writes:

> P.S. Junio, does t9001 pass for you?

It seems to.

Thanks for a detailed write-up. I'd appreciate a final fix in an
apply-able patch form.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] send-email: Fix %config_path_settings handling
  2011-10-14 18:13       ` Junio C Hamano
@ 2011-10-14 18:49         ` Jakub Narebski
  2011-10-14 19:26           ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Narebski @ 2011-10-14 18:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, Git Mailing List, Cord Seele, Cord Seele

From: Cord Seele <cowose@gmail.com>

cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30) broke
the expansion of aliases.

This was caused by treating %config_path_settings, newly introduced in
said patch, like %config_bool_settings instead of like %config_settings.
Copy from %config_settings, making it more readable.


Nb. there were a few issues that were responsible for this error:

1. %config_bool_settings and %config_settings despite similar name have
   different semantic.

   %config_bool_settings values are arrays where the first element is
   (reference to) the variable to set, and second element is default
   value... which admittedly is a bit cryptic.  More readable if more
   verbose option would be to use hash reference, e.g.:

        my %config_bool_settings = (
            "thread" => { variable => \$thread, default => 1},
            [...]

   Or something like that.

   %config_settings values are either either reference to scalar variable
   or reference to array.  In second case it means that option (or config
   option) is multi-valued.  BTW. this is similar to what Getopt::Long does.

2. In cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
   the setting "aliasesfile" was moved from %config_settings to newly
   introduced %config_path_settings.  But the loop that parses settings
   from %config_path_settings was copy'n'pasted *wrongly* from
   %config_bool_settings instead of from %config_settings.

   It looks like cec5dae author cargo-culted this change...

3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
   didn't add test for alias expansion to t9001-send-email.sh

Signed-off-by: Cord Seele <cowose@gmail.com>
Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio C Hamano wrote:

> Thanks for a detailed write-up. I'd appreciate a final fix in an
> apply-able patch form.

Something like this?

Nb. I was not sure whether to keep Cord authorship...

 git-send-email.perl |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 91607c5..41807b6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -337,8 +337,15 @@ sub read_config {
 	}
 
 	foreach my $setting (keys %config_path_settings) {
-		my $target = $config_path_settings{$setting}->[0];
-		$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+		my $target = $config_path_settings{$setting};
+		if (ref($target) eq "ARRAY" && !@$target) {
+			# multi-valued and not set
+			my @values = Git::config_path(@repo, "$prefix.$setting");
+			@$target = @values if (@values && defined $values[0]);
+		} elsif (!defined $$target) {
+			# multi-valued and not set
+			$$target = Git::config_path(@repo, "$prefix.$setting");
+		}
 	}
 
 	foreach my $setting (keys %config_settings) {
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: Fix %config_path_settings handling
  2011-10-14 18:49         ` [PATCH] send-email: Fix %config_path_settings handling Jakub Narebski
@ 2011-10-14 19:26           ` Junio C Hamano
  2011-10-14 20:53             ` Jakub Narebski
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2011-10-14 19:26 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Michael J Gruber, Git Mailing List, Cord Seele, Cord Seele

Jakub Narebski <jnareb@gmail.com> writes:

> From: Cord Seele <cowose@gmail.com>
>    value... which admittedly is a bit cryptic.  More readable if more
>    verbose option would be to use hash reference, e.g.:
>
>         my %config_bool_settings = (
>             "thread" => { variable => \$thread, default => 1},
>             [...]
>
>    Or something like that.

Do you really want to leave this "Or something like that" here?

> 3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
>    didn't add test for alias expansion to t9001-send-email.sh

I was hoping that an updated patch to have a new test or two here...

> Signed-off-by: Cord Seele <cowose@gmail.com>
> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

Is this the version tested by Michael?

> +		my $target = $config_path_settings{$setting};
> +		if (ref($target) eq "ARRAY" && !@$target) {
> +			# multi-valued and not set
> +			my @values = Git::config_path(@repo, "$prefix.$setting");
> +			@$target = @values if (@values && defined $values[0]);
> +		} elsif (!defined $$target) {
> +			# multi-valued and not set
> +			$$target = Git::config_path(@repo, "$prefix.$setting");
> +		}

If the target is an array ref and for whatever reason the array is already
populated, wouldn't you check "if (!defined $$target)" with this change?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: Fix %config_path_settings handling
  2011-10-14 19:26           ` Junio C Hamano
@ 2011-10-14 20:53             ` Jakub Narebski
  2011-10-14 23:23               ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Narebski @ 2011-10-14 20:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael J Gruber, Git Mailing List, Cord Seele, Cord Seele

From: Cord Seele <cowose@gmail.com>

cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30) broke
the expansion of aliases.

This was caused by treating %config_path_settings, newly introduced in
said patch, like %config_bool_settings instead of like %config_settings.
Copy from %config_settings, making it more readable.

While at it add basic test for expansion of aliases, and for path
expansion, which would catch this error.


Nb. there were a few issues that were responsible for this error:

1. %config_bool_settings and %config_settings despite similar name have
   different semantic.

   %config_bool_settings values are arrays where the first element is
   (reference to) the variable to set, and second element is default
   value... which admittedly is a bit cryptic.  More readable if more
   verbose option would be to use hash reference, e.g.:

        my %config_bool_settings = (
            "thread" => { variable => \$thread, default => 1},
            [...]

   %config_settings values are either either reference to scalar variable
   or reference to array.  In second case it means that option (or config
   option) is multi-valued.  BTW. this is similar to what Getopt::Long does.

2. In cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
   the setting "aliasesfile" was moved from %config_settings to newly
   introduced %config_path_settings.  But the loop that parses settings
   from %config_path_settings was copy'n'pasted *wrongly* from
   %config_bool_settings instead of from %config_settings.

   It looks like cec5dae author cargo-culted this change...

3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
   didn't add test for alias expansion to t9001-send-email.sh

Signed-off-by: Cord Seele <cowose@gmail.com>
Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > From: Cord Seele <cowose@gmail.com>
> >    value... which admittedly is a bit cryptic.  More readable if more
> >    verbose option would be to use hash reference, e.g.:
> >
> >         my %config_bool_settings = (
> >             "thread" => { variable => \$thread, default => 1},
> >             [...]
> >
> >    Or something like that.
> 
> Do you really want to leave this "Or something like that" here?

Removed.  

"e.g." should be enough.

> > 3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
> >    didn't add test for alias expansion to t9001-send-email.sh
> 
> I was hoping that an updated patch to have a new test or two here...

Done.  I thought to add it in separate patch...
 
> > Signed-off-by: Cord Seele <cowose@gmail.com>
> > Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
> > Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> 
> Is this the version tested by Michael?

This one is.

[cut previous version]

 git-send-email.perl   |   12 ++++++++++--
 t/t9001-send-email.sh |   28 ++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 91607c5..6885dfa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -337,8 +337,16 @@ sub read_config {
 	}
 
 	foreach my $setting (keys %config_path_settings) {
-		my $target = $config_path_settings{$setting}->[0];
-		$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+		my $target = $config_path_settings{$setting};
+		if (ref($target) eq "ARRAY") {
+			unless (@$target) {
+				my @values = Git::config_path(@repo, "$prefix.$setting");
+				@$target = @values if (@values && defined $values[0]);
+			}
+		}
+		else {
+			$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+		}
 	}
 
 	foreach my $setting (keys %config_settings) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 579ddb7..87b4acc 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1168,4 +1168,32 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '
 	test -n "$(ls msgtxt*)"
 '
 
+test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
+	clean_fake_sendmail &&
+	echo "alias sbd  somebody@example.org" >.mailrc &&
+	git config --replace-all sendemail.aliasesfile "$(pwd)/.mailrc" &&
+	git config sendemail.aliasfiletype mailrc &&
+	git send-email \
+	  --from="Example <nobody@example.com>" \
+	  --to=sbd \
+	  --smtp-server="$(pwd)/fake.sendmail" \
+	  outdir/0001-*.patch \
+	  2>errors >out &&
+	grep "^!somebody@example\.org!$" commandline1
+'
+
+test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
+	clean_fake_sendmail &&
+	echo "alias sbd  someone@example.org" >~/.mailrc &&
+	git config --replace-all sendemail.aliasesfile "~/.mailrc" &&
+	git config sendemail.aliasfiletype mailrc &&
+	git send-email \
+	  --from="Example <nobody@example.com>" \
+	  --to=sbd \
+	  --smtp-server="$(pwd)/fake.sendmail" \
+	  outdir/0001-*.patch \
+	  2>errors >out &&
+	grep "^!someone@example\.org!$" commandline1
+'
+
 test_done
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: Fix %config_path_settings handling
  2011-10-14 20:53             ` Jakub Narebski
@ 2011-10-14 23:23               ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2011-10-14 23:23 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Michael J Gruber, Git Mailing List, Cord Seele, Cord Seele

Jakub Narebski <jnareb@gmail.com> writes:

> From: Cord Seele <cowose@gmail.com>
> ...
> Signed-off-by: Cord Seele <cowose@gmail.com>
> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

Thanks.

> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 579ddb7..87b4acc 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1168,4 +1168,32 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '
>  	test -n "$(ls msgtxt*)"
>  '
>  
> +test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
> +	clean_fake_sendmail &&
> +	echo "alias sbd  somebody@example.org" >.mailrc &&
> +	git config --replace-all sendemail.aliasesfile "$(pwd)/.mailrc" &&
> +	git config sendemail.aliasfiletype mailrc &&
> +	git send-email \
> +	  --from="Example <nobody@example.com>" \
> +	  --to=sbd \
> +	  --smtp-server="$(pwd)/fake.sendmail" \
> +	  outdir/0001-*.patch \
> +	  2>errors >out &&
> +	grep "^!somebody@example\.org!$" commandline1
> +'
> +
> +test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
> +	clean_fake_sendmail &&
> +	echo "alias sbd  someone@example.org" >~/.mailrc &&
> +	git config --replace-all sendemail.aliasesfile "~/.mailrc" &&
> +	git config sendemail.aliasfiletype mailrc &&
> +	git send-email \
> +	  --from="Example <nobody@example.com>" \
> +	  --to=sbd \
> +	  --smtp-server="$(pwd)/fake.sendmail" \
> +	  outdir/0001-*.patch \
> +	  2>errors >out &&
> +	grep "^!someone@example\.org!$" commandline1
> +'
> +
>  test_done

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2011-10-14 23:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-14 12:29 [BUG] send-email: alias expansion broken Michael J Gruber
2011-10-14 14:25 ` [PATCH] fix alias expansion with new Git::config_path() Cord Seele
2011-10-14 14:30   ` Michael J Gruber
2011-10-14 14:42     ` Cord Seele
2011-10-14 15:05       ` Junio C Hamano
2011-10-14 16:38     ` Jakub Narebski
2011-10-14 18:13       ` Junio C Hamano
2011-10-14 18:49         ` [PATCH] send-email: Fix %config_path_settings handling Jakub Narebski
2011-10-14 19:26           ` Junio C Hamano
2011-10-14 20:53             ` Jakub Narebski
2011-10-14 23:23               ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).