Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> FWIW, the code looks OK here. It is a shame to duplicate the policy
> found in git_config_parse_key(), though.
>
> I wonder if we could make a master version of that which canonicalizes
> in-place, and then just wrap it for the git_config_parse_key()
> interface. Actually, I guess the function you just wrote _is_ that inner
> function, as long as it learned about the "quiet" flag.

Hmm, I obviously missed an opportunity.  I thought about doing a
similar thing with the policy in parse-source, but that side didn't
seem worth doing, as the config-parse-source callgraph is quite a
mess (as it has to parse the .ini like format with line breaks and
comments, not the simple "<string>[.<string>].<string>" thing, it
cannot quite avoid it), and it needs to take advantage of _some_
policy to parse the pieces.

We could loosen the policy implemented by config-parse-key interface
(e.g. change config-parse-source to let anything that begins with a
non-whitespace continue to be processed with get_value(), instead of
only allowing string that begins with isalpha(); similarly loosen
get_value() to allow any non-dot non-space string, not just
iskeychar() bytes) and first turn what is read into the simple
"<string>[.<string>].<string>" format, and then reuse the new
"master" logic to validate.  That would allow us to update the
"master" logic to make it tighter or looser to some degree, but the
source parser still needs to hardcode _some_ policy (e.g. the first
level and the last level names begin with a non-space) that allows
it to guess what "<string>" pieces the contents being parsed from
the .ini looking format meant to express.

But you are right.  config-parse-key does have the simpler string
that can just be given to the canonicalize thing and we should be
able to reuse it.