Re: [PATCH v3 2/3] config: add hashtable for config parsing & retrieval

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Hmm, maybe. The "... take advantage of the new code" refers to the
> possibility (or otherwise) of re-using your work to update these
> "older API" functions to the new API style. (also, see Junio's response).

I agree that, while caching the usual config files is the most
important, allowing other users of the config code to use non-callback
functions would be nice too.

Not really for efficiency, but because I find it far more natural to ask
"what's the value of config key XYZ" to the code than to ask "can you
parse all config keys in the file, let me know, and I'll tell you later
which ones I'm interested in".

> [In order to do this, I would have expected to see one hash table
> for each file/blob, so the singleton object took me by surprise.]

The singleton could be fine as long as it is optional. We can have
an API looking like:

int git_config_get_string(const char *key, const char **value) {
	return git_config_get_string_from(the singleton config cache,
                                          key, value);
}

int git_config_get_string_from(struct hashmap *map, const char *key, const char **value);

(or take a structure representing "a set of config files" instead of
directly the hashmap)

Now, the good news is that such API can be written later, without
breaking the API. So I'd say it's fine to keep the code as-is for now,
and make it more general later. No strong opinion, though.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/