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@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
>
> Tested-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx>
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html