Tanay Abhra <tanay...@gmail.com> writes:

> API functions:
>
> 1. int git_config_get_value(char *k, char **v): The most generic function, 
> traverses
> the usual config functions for values. (internally uses the the_config_set 
> struct)

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.


> 2. int git_configset_get_value_from(const char *key, const char **value,
>                               const char *filename, struct config_set *cs)

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).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to