Subject: Re: [PATCH] send-email: Fix %config_path_settings handling

From: Junio C Hamano <[email-masked]>
Date: Fri, 14 Oct 2011 12:26:43 -0700

Jakub Narebski <[email-masked]> writes:

> From: Cord Seele <[email-masked]>
>    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 <[email-masked]>
> Tested-by: Michael J Gruber <[email-masked]>
> Signed-off-by: Jakub Narebski <[email-masked]>

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?