Re: [PATCH 1/6] config: add helper function for parsing key names

From: Jeff King <peff@peff.net>
Date: Jan 15, 2013, 16:04 UTC
Message-ID: <20130115160422.GC21815@sigill.intra.peff.net>
In-Reply-To: <7v8v7veixc.fsf@alter.siamese.dyndns.org>

On Mon, Jan 14, 2013 at 10:08:47AM -0800, Junio C Hamano wrote:

Show 7 quoted lines

> > +extern int match_config_key(const char *var,
> > +		     const char *section,
> > +		     const char **subsection, int *subsection_len,
> > +		     const char **key);
> > +
> 
> I agree with Jonathan about the naming s/match/parse/.

I see this is marked for re-roll in WC. I'm happy to re-roll it with the suggestions from Jonathan, but before I do, did you have any comment on the "struct config_key" alternative I sent as a follow-up?

You said:

Show 9 quoted lines

> After looking at the callers in your later patches, I think the
> counted interface to subsection is probably fine.  The caller can
> check !subsection to see if it is a two- or three- level name, and
> 
>     if (parse_config_key(var, "submodule", &name, &namelen,  &key) < 0 ||
> 	!name)
> 	return 0;
> 
> is very easy to follow (that is the result of your 5th step).

but I wasn't sure if that was "it is not worth the trouble of the other one" or "I did not yet read the other one".

-Peff