Re: [PATCH/RFC] config: Add config sets for config parsing & retrieval

Tanay Abhra <tanayabh@gmail.com> writes:

This may also want to be accompanied by a set of more type-specific API functions, e.g.

     long git_config_get_long(char *key);
     unsigned long git_config_get_ulong(char *key);
     char *git_config_get_string(char *key);
     
but we would need to know how the error reporting should look like
for them.

You would also want to add a bit more before we can call the API to be complete. Here are two that comes to my mind:

 - Multi-valued variables

   int git_config_get_multi(char *k, char ***v, int *cnt);

   which may allocate an array of "char *" and return its address in
   *v, while returning its length in *cnt.  Alternatively, we can
   report the length of the array as the return value, signalling an
   error with a negative return value, without having the *cnt
   argument.

 - Iterating over keys

   Existing git_config() function signature can be kept for
   iterating over keys, I would think, even if we start caching the
   previous read of the config files from the filesystem.

Having to give "filename" each and every time I want to know about a single variable does not make any sense to me. I would have expected the API to be more like this:

	struct config_set *cs = git_configset_init();
        char *value;

        git_configset_from_file(cs, ".gitmodule");
        git_configset_get_value(cs, "submodule.kernel.update", &value);

There will need to be a set of "config-set" variant of functions that parallel the "work on the_config_set" variant above, e.g.

	long git_configset_get_long(struct config_set *, char *);
	int git_configset_get_multi(struct config_set *, char *, char ***, int *);
	int git_configset_config(struct config_set *, config_fn_t, void *);

The last one is the parallel to the traditional git_config() but it walks over a specific config_set.

And once this is done, you wouldn't have any separate implementation of git_config_get_value(). it will be a series of macros:

	#define git_config_get_value(k, v) \
        	git_configset_get_value(&the_config_set, (k), (v))

just like all the *_cache() API that used to be functions are now implemented as macros built around corresponding *_index() API functions (defined in cache.h).