Re: [PATCH] merge: use skip_prefix to parse config key

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 10, 2020, 16:44 UTC
Message-ID: <xmqqo8rzjnnb.fsf@gitster.c.googlers.com>
In-Reply-To: <20200410155827.GA71011@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 6 quoted lines

> In general, parsing subsections accurately involves looking from both
> the start and the end of the string, pulling out the section and key and
> leaving the rest in the middle. But I think we can get away with this
> left-to-right parsing because we're only interested in matching a
> _specific_ subsection name, and a specific key. So there are no cases it
> will handle incorrectly.

In other words, if k were "branch.A.B.mergeoptions", it can only be the 'branch.*.mergeoptions' variable attached to branch "A.B", but when checking for branch=="A", the first two skip_prefix() would pass and the only thing that protects us from misparsing is that "B.mergeoptions" is not what we are looking for.

Show 12 quoted lines

> The more general form would be:
>
>   const char *subsection, *key;
>   int subsection_len;
>
>   if (!parse_config_key("branch", &subsection, &subsection_len, &key) &&
>       subsection_len == strlen(branch) && !strncmp(subsection, branch) &&
>       !strcmp(key, "mergeoptions"))
>          ...
>
> but that's a bit more awkward (it would be less so if we had a helper
> function for comparing a NUL-terminated string against a ptr/len pair).

Yes, but even with such a helper, i.e.

	if (branch &&
	    !parse_config_key("branch", &sub, &sublen, &key) &&
	    !spanstrcmp(sub, sublen, branch) &&
	    !strcmp(key, "mergeoptions"))

what Martin wrote, especially if it is reflowed to match the above, i.e.

	if (branch &&
	    skip_prefix(key, "branch.", &sub) &&
	    skip_prefix(sub, branch, &sub) &&
	    !strcmp(sub, ".mergeoptions")

I find it just as, if not more, easy to read.

Where the parse_config_key() helper shines, I think, is when we do not have the middle level to compare against, and in that case, we must work only from the given key, scanning from both ends for dot.

Thanks.