Subject: Re: [PATCH] refs: parse_hide_refs_config to use parse_config_key

From: Junio C Hamano <[email-masked]>
Date: Fri, 24 Feb 2017 13:24:38 -0800

Jeff King <[email-masked]> 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.