Re: [PATCH v5 1/2] config: let git_config_parse_key() validate quietly

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

Harald Nordgren <haraldnordgren@gmail.com> writes:

>> Perhaps the updated "git_config_parse_key()" in this patch should be
>> renamed to be a file-scape static internal helper, and the existing
>> "git_config_parse_key()" should become a thin wrapper around that
>> new helper function, retaining the current external interface,
>> requiring no changes to existing callers.
>
> I want to remember a discussion on one of my earlier topics, a few
> months back, where someone else suggested instead of introducing two
> thin wrappers over a helper, we should update the callers instead.
>
> But for me either way is fine, maybe here it makes more sense, because
> of the repeated NULL/0/1 parameters.

If the "quiet" and "store_key" setting were independent, then I
wouldn't have made such a suggestion.  But I got an impression that
with the updated code, there wasn't a valid use case to ask to
quietly store the discovered key.

An ideal refactoring would have been a low level helper function
that only yields error code, and git_config_parse_key() would call
it and react to the returned error code, stores the discovered key,
and produces error message on its own.  Then such an "always quiet"
helper can be used for the purpose of the new caller, without having
to have "if (!quiet)" sprinkled all over.  But that is certainly
cumbersome to arrange.