send-email: Fix %config_path_settings handling

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Cord Seele committed Oct 14, 2011 at 22:53 UTC 463b0ea22b5b9a882e8140d0308433d8cbd0d1fe
2 files changed +38 -2
git-send-email.perl
+10 -2
@@ -337,8 +337,16 @@ sub read_config {
337 }
338
339 foreach my $setting (keys %config_path_settings) {
340 - my $target = $config_path_settings{$setting}->[0];
341 - $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
340 + my $target = $config_path_settings{$setting};
341 + if (ref($target) eq "ARRAY") {
342 + unless (@$target) {
343 + my @values = Git::config_path(@repo, "$prefix.$setting");
344 + @$target = @values if (@values && defined $values[0]);
345 + }
346 + }
347 + else {
348 + $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
349 + }
350 }
351
352 foreach my $setting (keys %config_settings) {
t/t9001-send-email.sh
+28
@@ -1168,4 +1168,32 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '
1168 test -n "$(ls msgtxt*)"
1169 '
1170
1171 +test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
1172 + clean_fake_sendmail &&
1173 + echo "alias sbd somebody@example.org" >.mailrc &&
1174 + git config --replace-all sendemail.aliasesfile "$(pwd)/.mailrc" &&
1175 + git config sendemail.aliasfiletype mailrc &&
1176 + git send-email \
1177 + --from="Example <nobody@example.com>" \
1178 + --to=sbd \
1179 + --smtp-server="$(pwd)/fake.sendmail" \
1180 + outdir/0001-*.patch \
1181 + 2>errors >out &&
1182 + grep "^!somebody@example\.org!$" commandline1
1183 +'
1184 +
1185 +test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
1186 + clean_fake_sendmail &&
1187 + echo "alias sbd someone@example.org" >~/.mailrc &&
1188 + git config --replace-all sendemail.aliasesfile "~/.mailrc" &&
1189 + git config sendemail.aliasfiletype mailrc &&
1190 + git send-email \
1191 + --from="Example <nobody@example.com>" \
1192 + --to=sbd \
1193 + --smtp-server="$(pwd)/fake.sendmail" \
1194 + outdir/0001-*.patch \
1195 + 2>errors >out &&
1196 + grep "^!someone@example\.org!$" commandline1
1197 +'
1198 +
1199 test_done