From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Stefan Beller <sbeller@google.com>, git@vger.kernel.org
Subject: Re: [PATCH] refs: parse_hide_refs_config to use parse_config_key
Date: Fri, 24 Feb 2017 13:24:38 -0800 [thread overview]
Message-ID: <xmqqvarzz3hl.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170224212025.hcxusnrxijzb5qox@sigill.intra.peff.net> (Jeff King's message of "Fri, 24 Feb 2017 16:20:25 -0500")
Jeff King <peff@peff.net> writes:
> On Fri, Feb 24, 2017 at 01:18:48PM -0800, Junio C Hamano wrote:
>
>> > While I'm thinking about it, here are patches to do that. The third one
>> > I'd probably squash into yours (after ordering it to the end).
>> >
>> > [1/3]: parse_config_key: use skip_prefix instead of starts_with
>> > [2/3]: parse_config_key: allow matching single-level config
>> > [3/3]: parse_hide_refs_config: tell parse_config_key we don't want a subsection
>>
>> While you were doing that, I was grepping the call sites for
>> parse_config_key() and made sure that all of them are OK when fed
>> two level names. Most of them follow this pattern:
>>
>> if (parse_config_key(k, "diff", &name, &namelen, &type) || !name)
>> return -1;
>>
>> and ones that do not immediately check !name does either eventually
>> do so or have separate codepaths for handlihng two- and three-level
>> names.
>
> Yeah, I did that, too. :)
>
> I don't think there are any other sites to convert. And I don't think we
> can make the "!name" case easier (because some call-sites do want to
> handle both types). And it's not like it gets much easier anyway (unlike
> the opposite case, you _do_ need to declare the extra variables.
Yeah, because the rejection of !name as an error is not the only
reason these call sites have &name and &namelen---they want to use
that subsection name after that if() statement rejects malformed
input, so we cannot really lose them.
Thanks. All three looked good.
prev parent reply other threads:[~2017-02-24 21:25 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-24 20:33 [PATCH] refs: parse_hide_refs_config to use parse_config_key Stefan Beller
2017-02-24 20:39 ` Jeff King
2017-02-24 20:43 ` Stefan Beller
2017-02-24 20:47 ` Jeff King
2017-02-24 20:47 ` Junio C Hamano
2017-02-24 21:06 ` Jeff King
2017-02-24 21:07 ` [PATCH 1/3] parse_config_key: use skip_prefix instead of starts_with Jeff King
2017-02-24 21:08 ` [PATCH 2/3] parse_config_key: allow matching single-level config Jeff King
2017-02-24 21:20 ` Junio C Hamano
2017-02-24 21:25 ` Junio C Hamano
2017-02-24 21:26 ` Jeff King
2017-02-24 21:08 ` [PATCH 3/3] parse_hide_refs_config: tell parse_config_key we don't want a subsection Jeff King
2017-02-24 23:28 ` Stefan Beller
2017-02-24 21:18 ` [PATCH] refs: parse_hide_refs_config to use parse_config_key Junio C Hamano
2017-02-24 21:20 ` Jeff King
2017-02-24 21:24 ` Junio C Hamano [this message]
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=xmqqvarzz3hl.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.