{"thread":{"id":"28677","subject":"[BUG] send-email: alias expansion broken","startedAt":"2011-10-14T12:29:27Z","lastAt":"2011-10-14T23:23:50Z","messageCount":11,"participants":["Michael J Gruber","Cord Seele","Junio C Hamano","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"177637","messageId":"4E982B27.8050807@drmicha.warpmail.net","threadId":"28677","inReplyTo":null,"subject":"[BUG] send-email: alias expansion broken","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T12:29:27Z","receivedAt":"2011-10-14T12:29:27Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)\n\nbroke the expansion of aliases for me:\n\n./git-send-email --cc=junio  --dry-run\n0001-t7800-avoid-arithmetic-expansion-notation.patch\n0001-t7800-avoid-arithmetic-expansion-notation.patch\nWho should the emails appear to be from? [Michael J Gruber\n<git@drmicha.warpmail.net>]\nEmails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>\nDry-OK. Log says:\nSendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio\ngit@drmicha.warpmail.net\nFrom: Michael J Gruber <git@drmicha.warpmail.net>\nTo: git@vger.kernel.org\nCc: junio\n...\n\nHappens with both \"--cc junio\" and \"--cc=junio\".\n\nReverting cec5dae brings my aliases back. Relevant config:\n\ngit config --get-regexp sendemail.alias\\*\nsendemail.aliasesfile /home/mjg/git/gitauthors\nsendemail.aliasfiletype mutt\n\nCan I please have alias expansion back?\n\nNo, I don't know what cec5dae tries to achieve, and I lack the perl fu\nto fix it.\n\nMichael\n"},{"id":"177647","messageId":"20111014142557.GB13680@laptop","threadId":"28677","inReplyTo":"4E982B27.8050807@drmicha.warpmail.net","subject":"[PATCH] fix alias expansion with new Git::config_path()","fromName":"Cord Seele","fromEmail":"cowose@googlemail.com","sentAt":"2011-10-14T14:25:57Z","receivedAt":"2011-10-14T14:25:57Z","isPatch":true,"sender":{"key":"cowose@gmail.com","avatar":null},"body":"On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:\n\n> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)\n> \n> broke the expansion of aliases for me:\n> \n> ./git-send-email --cc=junio  --dry-run\n> 0001-t7800-avoid-arithmetic-expansion-notation.patch\n> 0001-t7800-avoid-arithmetic-expansion-notation.patch\n> Who should the emails appear to be from? [Michael J Gruber\n> <git@drmicha.warpmail.net>]\n> Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>\n> Dry-OK. Log says:\n> Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio\n> git@drmicha.warpmail.net\n> From: Michael J Gruber <git@drmicha.warpmail.net>\n> To: git@vger.kernel.org\n> Cc: junio\n> ...\n> \n> Happens with both \"--cc junio\" and \"--cc=junio\".\n> \n> Reverting cec5dae brings my aliases back. Relevant config:\n> \n> git config --get-regexp sendemail.alias\\*\n> sendemail.aliasesfile /home/mjg/git/gitauthors\n> sendemail.aliasfiletype mutt\n> \n> Can I please have alias expansion back?\n\nThe following patch fixes it for me, please give it a try.\n\nSince this fix is simply copy&pasting some code from the config_settings path\nsomeone with better perl understanding might wnat to refactor it\n(Junio/Jacob)?\n\n-- Cord\n\n\nSigned-off-by: Cord Seele <cowose@gmail.com>\n---\n git-send-email.perl |   12 ++++++++++--\n 1 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 91607c5..6885dfa 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -337,8 +337,16 @@ sub read_config {\n \t}\n \n \tforeach my $setting (keys %config_path_settings) {\n-\t\tmy $target = $config_path_settings{$setting}->[0];\n-\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\tmy $target = $config_path_settings{$setting};\n+\t\tif (ref($target) eq \"ARRAY\") {\n+\t\t\tunless (@$target) {\n+\t\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n+\t\t\t\t@$target = @values if (@values && defined $values[0]);\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\t}\n \t}\n \n \tforeach my $setting (keys %config_settings) {\n-- \n1.7.6.4\n"},{"id":"177648","messageId":"4E984781.6050601@drmicha.warpmail.net","threadId":"28677","inReplyTo":"20111014142557.GB13680@laptop","subject":"Re: [PATCH] fix alias expansion with new Git::config_path()","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T14:30:25Z","receivedAt":"2011-10-14T14:30:25Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Cord Seele venit, vidit, dixit 14.10.2011 16:25:\n> On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:\n> \n>> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)\n>>\n>> broke the expansion of aliases for me:\n>>\n>> ./git-send-email --cc=junio  --dry-run\n>> 0001-t7800-avoid-arithmetic-expansion-notation.patch\n>> 0001-t7800-avoid-arithmetic-expansion-notation.patch\n>> Who should the emails appear to be from? [Michael J Gruber\n>> <git@drmicha.warpmail.net>]\n>> Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>\n>> Dry-OK. Log says:\n>> Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio\n>> git@drmicha.warpmail.net\n>> From: Michael J Gruber <git@drmicha.warpmail.net>\n>> To: git@vger.kernel.org\n>> Cc: junio\n>> ...\n>>\n>> Happens with both \"--cc junio\" and \"--cc=junio\".\n>>\n>> Reverting cec5dae brings my aliases back. Relevant config:\n>>\n>> git config --get-regexp sendemail.alias\\*\n>> sendemail.aliasesfile /home/mjg/git/gitauthors\n>> sendemail.aliasfiletype mutt\n>>\n>> Can I please have alias expansion back?\n> \n> The following patch fixes it for me, please give it a try.\n> \n> Since this fix is simply copy&pasting some code from the config_settings path\n> someone with better perl understanding might wnat to refactor it\n> (Junio/Jacob)?\n> \n> -- Cord\n> \n> \n> Signed-off-by: Cord Seele <cowose@gmail.com>\n\nTested-by: Michael J Gruber <git@drmicha.warpmail.net>\n\n\nThanks. (Though I'm still wondering what this is about overall.)\n\n\n> ---\n>  git-send-email.perl |   12 ++++++++++--\n>  1 files changed, 10 insertions(+), 2 deletions(-)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 91607c5..6885dfa 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -337,8 +337,16 @@ sub read_config {\n>  \t}\n>  \n>  \tforeach my $setting (keys %config_path_settings) {\n> -\t\tmy $target = $config_path_settings{$setting}->[0];\n> -\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n> +\t\tmy $target = $config_path_settings{$setting};\n> +\t\tif (ref($target) eq \"ARRAY\") {\n> +\t\t\tunless (@$target) {\n> +\t\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n> +\t\t\t\t@$target = @values if (@values && defined $values[0]);\n> +\t\t\t}\n> +\t\t}\n> +\t\telse {\n> +\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n> +\t\t}\n>  \t}\n>  \n>  \tforeach my $setting (keys %config_settings) {\n"},{"id":"177650","messageId":"20111014144203.GC13680@laptop","threadId":"28677","inReplyTo":"4E984781.6050601@drmicha.warpmail.net","subject":"Re: [PATCH] fix alias expansion with new Git::config_path()","fromName":"Cord Seele","fromEmail":"cowose@googlemail.com","sentAt":"2011-10-14T14:42:03Z","receivedAt":"2011-10-14T14:42:03Z","isPatch":true,"sender":{"key":"cowose@gmail.com","avatar":null},"body":"On Fri 14 Oct 2011 16:30:25 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:\n\n> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>\n\nGreat.\n\n> Thanks. (Though I'm still wondering what this is about overall.)\n\nto make '~/' work in sendemail.aliasesfile\n\n-- Cord\n"},{"id":"177651","messageId":"7vd3dz1ump.fsf@alter.siamese.dyndns.org","threadId":"28677","inReplyTo":"20111014144203.GC13680@laptop","subject":"Re: [PATCH] fix alias expansion with new Git::config_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-14T15:05:18Z","receivedAt":"2011-10-14T15:05:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Cord Seele <cowose@googlemail.com> writes:\n\n> On Fri 14 Oct 2011 16:30:25 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:\n>\n>> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>\n>\n> Great.\n>\n>> Thanks. (Though I'm still wondering what this is about overall.)\n>\n> to make '~/' work in sendemail.aliasesfile\n\nThat is not an explanation.\n"},{"id":"177655","messageId":"201110141838.19118.jnareb@gmail.com","threadId":"28677","inReplyTo":"4E984781.6050601@drmicha.warpmail.net","subject":"Re: [PATCH] fix alias expansion with new Git::config_path()","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-10-14T16:38:18Z","receivedAt":"2011-10-14T16:38:18Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Michael J Gruber wrote:\n> Cord Seele venit, vidit, dixit 14.10.2011 16:25:\n>> On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:\n>> \n>>> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)\n>>> broke the expansion of aliases for me:\n[...]\n>>> Reverting cec5dae brings my aliases back. [...]\n[...]\n>> \n>> The following patch fixes it for me, please give it a try.\n>> \n>> Since this fix is simply copy&pasting some code from the config_settings path\n>> someone with better perl understanding might wnat to refactor it\n>> (Junio/Jacob)?\n\n[missing commit message]\n\n>> Signed-off-by: Cord Seele <cowose@gmail.com>\n> \n> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>\n> \n> Thanks. (Though I'm still wondering what this is about overall.)\n\nThere were a few issues that were responsible for this error:\n\n1. %config_bool_settings and %config_settings despite similar name have\n   different semantic.  \n\n   %config_bool_settings values are arrays where the first element is\n   (reference to) the variable to set, and second element is default\n   value... which admittedly is a bit cryptic.  More readable if more\n   verbose option would be to use hash reference, e.g.:\n\n\tmy %config_bool_settings = (\n\t    \"thread\" => { variable => \\$thread, default => 1},\n\t    [...]\n\n   Or something like that.\n\n   %config_settings values are either either reference to scalar variable\n   or reference to array.  In second case it means that option (or config\n   option) is multi-valued.  BTW. this is similar to what Getopt::Long does.\n\n2. In cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)\n   the setting \"aliasesfile\" was moved from %config_settings to newly\n   introduced %config_path_settings.  But the loop that parses settings\n   from %config_path_settings was copy'n'pasted *wrongly* from\n   %config_bool_settings instead of from %config_settings.\n\n   It looks like cec5dae author cargo-culted this change...\n\n3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)\n   didn't add test for alias expansion to t9001-send-email.sh\n\n>> ---\n>>  git-send-email.perl |   12 ++++++++++--\n>>  1 files changed, 10 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index 91607c5..6885dfa 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -337,8 +337,16 @@ sub read_config {\n>>  \t}\n>>  \n>>  \tforeach my $setting (keys %config_path_settings) {\n>> -\t\tmy $target = $config_path_settings{$setting}->[0];\n>> -\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n>> +\t\tmy $target = $config_path_settings{$setting};\n>> +\t\tif (ref($target) eq \"ARRAY\") {\n>> +\t\t\tunless (@$target) {\n>> +\t\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n>> +\t\t\t\t@$target = @values if (@values && defined $values[0]);\n>> +\t\t\t}\n>> +\t\t}\n>> +\t\telse {\n>> +\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n>> +\t\t}\n>>  \t}\n>>  \n>>  \tforeach my $setting (keys %config_settings) {\n\nOr a bit simpler (though still duplicated somewhat code with\n%config_settings) case:\n\ndiff --git i/git-send-email.perl w/git-send-email.perl\nindex 91607c5..eed241e 100755\n--- i/git-send-email.perl\n+++ w/git-send-email.perl\n@@ -337,8 +337,13 @@ sub read_config {\n \t}\n \n \tforeach my $setting (keys %config_path_settings) {\n-\t\tmy $target = $config_path_settings{$setting}->[0];\n-\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\tmy $target = $config_path_settings{$setting};\n+\t\tif (ref($target) eq \"ARRAY\" && !@$target) {\n+\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n+\t\t\t@$target = @values if (@values && defined $values[0]);\n+\t\t} elsif (!defined $$target) {\n+\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\");\n+\t\t}\n \t}\n \n \tforeach my $setting (keys %config_settings) {\n\nP.S. Junio, does t9001 pass for you?  For me it fails very strangely on\nsome tests: \n\n  not ok - 21 reject long lines\n  not ok - 22 no patch was sent\n  not ok - 28 In-Reply-To without --chain-reply-to\n  not ok - 29 In-Reply-To with --chain-reply-to\n  not ok - 39 sendemail.cccmd\n  not ok - 49 --suppress-cc=bodycc\n  not ok - 51 --suppress-cc=cc\n  not ok - 56 confirm by default (due to cc)\n  not ok - 70 warning with an implicit --chain-reply-to\n  # failed 9 among 93 test(s)\n-- \nJakub Narebski\nPoland\n"},{"id":"177661","messageId":"7vwrc7zbk2.fsf@alter.siamese.dyndns.org","threadId":"28677","inReplyTo":"201110141838.19118.jnareb@gmail.com","subject":"Re: [PATCH] fix alias expansion with new Git::config_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-14T18:13:17Z","receivedAt":"2011-10-14T18:13:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> P.S. Junio, does t9001 pass for you?\n\nIt seems to.\n\nThanks for a detailed write-up. I'd appreciate a final fix in an\napply-able patch form.\n"},{"id":"177664","messageId":"201110142049.32734.jnareb@gmail.com","threadId":"28677","inReplyTo":"7vwrc7zbk2.fsf@alter.siamese.dyndns.org","subject":"[PATCH] send-email: Fix %config_path_settings handling","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-10-14T18:49:32Z","receivedAt":"2011-10-14T18:49:32Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: Cord Seele <cowose@gmail.com>\n\ncec5dae (use new Git::config_path() for aliasesfile, 2011-09-30) broke\nthe expansion of aliases.\n\nThis was caused by treating %config_path_settings, newly introduced in\nsaid patch, like %config_bool_settings instead of like %config_settings.\nCopy from %config_settings, making it more readable.\n\n\nNb. there were a few issues that were responsible for this error:\n\n1. %config_bool_settings and %config_settings despite similar name have\n   different semantic.\n\n   %config_bool_settings values are arrays where the first element is\n   (reference to) the variable to set, and second element is default\n   value... which admittedly is a bit cryptic.  More readable if more\n   verbose option would be to use hash reference, e.g.:\n\n        my %config_bool_settings = (\n            \"thread\" => { variable => \\$thread, default => 1},\n            [...]\n\n   Or something like that.\n\n   %config_settings values are either either reference to scalar variable\n   or reference to array.  In second case it means that option (or config\n   option) is multi-valued.  BTW. this is similar to what Getopt::Long does.\n\n2. In cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)\n   the setting \"aliasesfile\" was moved from %config_settings to newly\n   introduced %config_path_settings.  But the loop that parses settings\n   from %config_path_settings was copy'n'pasted *wrongly* from\n   %config_bool_settings instead of from %config_settings.\n\n   It looks like cec5dae author cargo-culted this change...\n\n3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)\n   didn't add test for alias expansion to t9001-send-email.sh\n\nSigned-off-by: Cord Seele <cowose@gmail.com>\nTested-by: Michael J Gruber <git@drmicha.warpmail.net>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nJunio C Hamano wrote:\n\n> Thanks for a detailed write-up. I'd appreciate a final fix in an\n> apply-able patch form.\n\nSomething like this?\n\nNb. I was not sure whether to keep Cord authorship...\n\n git-send-email.perl |   11 +++++++++--\n 1 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 91607c5..41807b6 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -337,8 +337,15 @@ sub read_config {\n \t}\n \n \tforeach my $setting (keys %config_path_settings) {\n-\t\tmy $target = $config_path_settings{$setting}->[0];\n-\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\tmy $target = $config_path_settings{$setting};\n+\t\tif (ref($target) eq \"ARRAY\" && !@$target) {\n+\t\t\t# multi-valued and not set\n+\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n+\t\t\t@$target = @values if (@values && defined $values[0]);\n+\t\t} elsif (!defined $$target) {\n+\t\t\t# multi-valued and not set\n+\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\");\n+\t\t}\n \t}\n \n \tforeach my $setting (keys %config_settings) {\n-- \n1.7.6\n"},{"id":"177673","messageId":"7vbotjz85o.fsf@alter.siamese.dyndns.org","threadId":"28677","inReplyTo":"201110142049.32734.jnareb@gmail.com","subject":"Re: [PATCH] send-email: Fix %config_path_settings handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-14T19:26:43Z","receivedAt":"2011-10-14T19:26:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> From: Cord Seele <cowose@gmail.com>\n>    value... which admittedly is a bit cryptic.  More readable if more\n>    verbose option would be to use hash reference, e.g.:\n>\n>         my %config_bool_settings = (\n>             \"thread\" => { variable => \\$thread, default => 1},\n>             [...]\n>\n>    Or something like that.\n\nDo you really want to leave this \"Or something like that\" here?\n\n> 3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)\n>    didn't add test for alias expansion to t9001-send-email.sh\n\nI was hoping that an updated patch to have a new test or two here...\n\n> Signed-off-by: Cord Seele <cowose@gmail.com>\n> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>\n> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n\nIs this the version tested by Michael?\n\n> +\t\tmy $target = $config_path_settings{$setting};\n> +\t\tif (ref($target) eq \"ARRAY\" && !@$target) {\n> +\t\t\t# multi-valued and not set\n> +\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n> +\t\t\t@$target = @values if (@values && defined $values[0]);\n> +\t\t} elsif (!defined $$target) {\n> +\t\t\t# multi-valued and not set\n> +\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\");\n> +\t\t}\n\nIf the target is an array ref and for whatever reason the array is already\npopulated, wouldn't you check \"if (!defined $$target)\" with this change?\n"},{"id":"177678","messageId":"201110142253.32695.jnareb@gmail.com","threadId":"28677","inReplyTo":"7vbotjz85o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] send-email: Fix %config_path_settings handling","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-10-14T20:53:31Z","receivedAt":"2011-10-14T20:53:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: Cord Seele <cowose@gmail.com>\n\ncec5dae (use new Git::config_path() for aliasesfile, 2011-09-30) broke\nthe expansion of aliases.\n\nThis was caused by treating %config_path_settings, newly introduced in\nsaid patch, like %config_bool_settings instead of like %config_settings.\nCopy from %config_settings, making it more readable.\n\nWhile at it add basic test for expansion of aliases, and for path\nexpansion, which would catch this error.\n\n\nNb. there were a few issues that were responsible for this error:\n\n1. %config_bool_settings and %config_settings despite similar name have\n   different semantic.\n\n   %config_bool_settings values are arrays where the first element is\n   (reference to) the variable to set, and second element is default\n   value... which admittedly is a bit cryptic.  More readable if more\n   verbose option would be to use hash reference, e.g.:\n\n        my %config_bool_settings = (\n            \"thread\" => { variable => \\$thread, default => 1},\n            [...]\n\n   %config_settings values are either either reference to scalar variable\n   or reference to array.  In second case it means that option (or config\n   option) is multi-valued.  BTW. this is similar to what Getopt::Long does.\n\n2. In cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)\n   the setting \"aliasesfile\" was moved from %config_settings to newly\n   introduced %config_path_settings.  But the loop that parses settings\n   from %config_path_settings was copy'n'pasted *wrongly* from\n   %config_bool_settings instead of from %config_settings.\n\n   It looks like cec5dae author cargo-culted this change...\n\n3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)\n   didn't add test for alias expansion to t9001-send-email.sh\n\nSigned-off-by: Cord Seele <cowose@gmail.com>\nTested-by: Michael J Gruber <git@drmicha.warpmail.net>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nJunio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > From: Cord Seele <cowose@gmail.com>\n> >    value... which admittedly is a bit cryptic.  More readable if more\n> >    verbose option would be to use hash reference, e.g.:\n> >\n> >         my %config_bool_settings = (\n> >             \"thread\" => { variable => \\$thread, default => 1},\n> >             [...]\n> >\n> >    Or something like that.\n> \n> Do you really want to leave this \"Or something like that\" here?\n\nRemoved.  \n\n\"e.g.\" should be enough.\n\n> > 3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)\n> >    didn't add test for alias expansion to t9001-send-email.sh\n> \n> I was hoping that an updated patch to have a new test or two here...\n\nDone.  I thought to add it in separate patch...\n \n> > Signed-off-by: Cord Seele <cowose@gmail.com>\n> > Tested-by: Michael J Gruber <git@drmicha.warpmail.net>\n> > Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n> \n> Is this the version tested by Michael?\n\nThis one is.\n\n[cut previous version]\n\n git-send-email.perl   |   12 ++++++++++--\n t/t9001-send-email.sh |   28 ++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 91607c5..6885dfa 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -337,8 +337,16 @@ sub read_config {\n \t}\n \n \tforeach my $setting (keys %config_path_settings) {\n-\t\tmy $target = $config_path_settings{$setting}->[0];\n-\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\tmy $target = $config_path_settings{$setting};\n+\t\tif (ref($target) eq \"ARRAY\") {\n+\t\t\tunless (@$target) {\n+\t\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n+\t\t\t\t@$target = @values if (@values && defined $values[0]);\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\t}\n \t}\n \n \tforeach my $setting (keys %config_settings) {\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 579ddb7..87b4acc 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1168,4 +1168,32 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '\n \ttest -n \"$(ls msgtxt*)\"\n '\n \n+test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '\n+\tclean_fake_sendmail &&\n+\techo \"alias sbd  somebody@example.org\" >.mailrc &&\n+\tgit config --replace-all sendemail.aliasesfile \"$(pwd)/.mailrc\" &&\n+\tgit config sendemail.aliasfiletype mailrc &&\n+\tgit send-email \\\n+\t  --from=\"Example <nobody@example.com>\" \\\n+\t  --to=sbd \\\n+\t  --smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t  outdir/0001-*.patch \\\n+\t  2>errors >out &&\n+\tgrep \"^!somebody@example\\.org!$\" commandline1\n+'\n+\n+test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '\n+\tclean_fake_sendmail &&\n+\techo \"alias sbd  someone@example.org\" >~/.mailrc &&\n+\tgit config --replace-all sendemail.aliasesfile \"~/.mailrc\" &&\n+\tgit config sendemail.aliasfiletype mailrc &&\n+\tgit send-email \\\n+\t  --from=\"Example <nobody@example.com>\" \\\n+\t  --to=sbd \\\n+\t  --smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t  outdir/0001-*.patch \\\n+\t  2>errors >out &&\n+\tgrep \"^!someone@example\\.org!$\" commandline1\n+'\n+\n test_done\n-- \n1.7.6\n"},{"id":"177701","messageId":"7v62jrxim1.fsf@alter.siamese.dyndns.org","threadId":"28677","inReplyTo":"201110142253.32695.jnareb@gmail.com","subject":"Re: [PATCH] send-email: Fix %config_path_settings handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-14T23:23:50Z","receivedAt":"2011-10-14T23:23:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> From: Cord Seele <cowose@gmail.com>\n> ...\n> Signed-off-by: Cord Seele <cowose@gmail.com>\n> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>\n> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n\nThanks.\n\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 579ddb7..87b4acc 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -1168,4 +1168,32 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '\n>  \ttest -n \"$(ls msgtxt*)\"\n>  '\n>  \n> +test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '\n> +\tclean_fake_sendmail &&\n> +\techo \"alias sbd  somebody@example.org\" >.mailrc &&\n> +\tgit config --replace-all sendemail.aliasesfile \"$(pwd)/.mailrc\" &&\n> +\tgit config sendemail.aliasfiletype mailrc &&\n> +\tgit send-email \\\n> +\t  --from=\"Example <nobody@example.com>\" \\\n> +\t  --to=sbd \\\n> +\t  --smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t  outdir/0001-*.patch \\\n> +\t  2>errors >out &&\n> +\tgrep \"^!somebody@example\\.org!$\" commandline1\n> +'\n> +\n> +test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '\n> +\tclean_fake_sendmail &&\n> +\techo \"alias sbd  someone@example.org\" >~/.mailrc &&\n> +\tgit config --replace-all sendemail.aliasesfile \"~/.mailrc\" &&\n> +\tgit config sendemail.aliasfiletype mailrc &&\n> +\tgit send-email \\\n> +\t  --from=\"Example <nobody@example.com>\" \\\n> +\t  --to=sbd \\\n> +\t  --smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t  outdir/0001-*.patch \\\n> +\t  2>errors >out &&\n> +\tgrep \"^!someone@example\\.org!$\" commandline1\n> +'\n> +\n>  test_done\n"}]}