Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Matthieu Moy <hidden>
Date: 2016-06-15 23:01:45

Junio C Hamano [off-list ref] writes:
When the submodule script that uses "git config -f .gitmodules" is
converted into C, if the updated config API is ready, it may be able
to do something like these in a single program:

	const char *url;
	struct config_set *gm_config;

        /* read from $GIT_DIR/config */
        url = git_config_get_string("remote.origin.url");

        /*
         * allow us to read from .gitmodules; the parameters are
         * list of files that make up the configset, perhaps.
         */
	gm_config = git_configset_new(".gitmodules", NULL);


        if (!git_configset_get_bool(gm_config, "submodule.frotz.ignore")) {
		/* do a lot of stuff for the submodule */
                ...
	}

        /* when we are done with the configset */
        git_configset_clear(gm_config);
Isn't that a bit overkill? Why not just let the caller manage a hashmap
directly instead of a config_set? Your code could become

  struct hashmap *gm_config;
  config_cache_init(&gm_config);
  config_cache_read_from_file(".gitmodule", gm_config);
  /* possibly more calls to
     config_cache_read_from_file("some-other-file", ...). */
  
and then

  if (!git_configset_get_bool(gm_config, "submodule.frotz.ignore")) {
     ...
  

Perhaps a bit more complicated to use, but much simpler to implement.
Since there are very few callers, I'd say it's better to keep the
implementation simple.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help