From: Junio C Hamano <gitster@pobox•com>
To: Jakub Narebski <jnareb@gmail•com>
Cc: Michael J Gruber <git@drmicha•warpmail.net>,
Git Mailing List <git@vger•kernel.org>,
Cord Seele <cowose@gmail•com>, Cord Seele <cowose@googlemail•com>
Subject: Re: [PATCH] send-email: Fix %config_path_settings handling
Date: Fri, 14 Oct 2011 12:26:43 -0700 [thread overview]
Message-ID: <7vbotjz85o.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <201110142049.32734.jnareb@gmail.com> (Jakub Narebski's message of "Fri, 14 Oct 2011 20:49:32 +0200")
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?
next prev parent reply other threads:[~2011-10-14 19:26 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2011-10-14 20:53 ` Jakub Narebski
2011-10-14 23:23 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vbotjz85o.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=cowose@gmail$(echo .)com \
--cc=cowose@googlemail$(echo .)com \
--cc=git@drmicha$(echo .)warpmail.net \
--cc=git@vger$(echo .)kernel.org \
--cc=jnareb@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox