* [RFC/PATCH 0/2] Git config cache & special querying api utilizing the cache @ 2014-05-26 17:33 Tanay Abhra 2014-05-26 17:33 ` [RFC/PATCH 1/2] config: Add cache for config value querying Tanay Abhra 2014-05-26 17:33 ` [RFC/PATCH 2/2] config: Add new query functions to the api Tanay Abhra 0 siblings, 2 replies; 7+ messages in thread From: Tanay Abhra @ 2014-05-26 17:33 UTC (permalink / raw) To: git; +Cc: Tanay Abhra, Ramkumar Ramachandra, Matthieu Moy Hi, This is my first patch series for this year's GSoC. My project is "Git Config API improvements". The link of my proposal is appended below [1]. The aim of this patch series is to generate a cache for querying values from the config files in a non callback manner as the current method reads and parses the config files every time a value is queried for. The cache is generated from hooking the update_cache function to the current parsing and callback mechanism in config.c. It is implemented as an hashmap using the hashmap-api with variables and its corresponding values list as its members. The values in the list are sorted in order of increasing priority. The cache is initialised in git_config_early() as it is the first time a `git_config` function is called during program startup. setup_git_directory_gently() calls git_config_early() which in turn reads every config file (local, user and global config files). get_value() in config.c feeds variable and values into the callback function. Using this function as a hook, we update the cache. Also, we add two new functions to the config-api git_config_get_string() and git_config_get_string_multi() for querying in a non callback manner from the cache. I have run the tests and debug the code and it works, but I have to add a few things, 1. Invalidity check: if a config file is written into, update the cache. I am using git_config_set_multivar_in_file() as an update hook. 2. Metadata about the variables and values. I have added only the file from each variable value pair comes in an upcoming series. What else should I add or implement ;is my approach right? [1] https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing Cheers, Tanay Abhra. Tanay Abhra (2): config: Add cache for config value querying config: Add new query functions to the api Documentation/technical/api-config.txt | 19 ++++++ cache.h | 2 + config.c | 105 +++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+) -- 1.9.0.GIT ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC/PATCH 1/2] config: Add cache for config value querying 2014-05-26 17:33 [RFC/PATCH 0/2] Git config cache & special querying api utilizing the cache Tanay Abhra @ 2014-05-26 17:33 ` Tanay Abhra 2014-05-26 20:02 ` Torsten Bögershausen 2014-05-28 9:31 ` Eric Sunshine 2014-05-26 17:33 ` [RFC/PATCH 2/2] config: Add new query functions to the api Tanay Abhra 1 sibling, 2 replies; 7+ messages in thread From: Tanay Abhra @ 2014-05-26 17:33 UTC (permalink / raw) To: git; +Cc: Tanay Abhra, Ramkumar Ramachandra, Matthieu Moy Add an internal cache with the all variable value pairs read from the usual config files(repo specific .git/config, user wide ~/.gitconfig and the global /etc/gitconfig). Also, add two external functions `git_config_get_string` and `git_config_get_string_multi` for querying in an non callback manner from the cache. Signed-off-by: Tanay Abhra <tanayabh@gmail.com> --- cache.h | 2 ++ config.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/cache.h b/cache.h index 107ac61..2038150 100644 --- a/cache.h +++ b/cache.h @@ -1271,6 +1271,8 @@ extern int check_repository_format_version(const char *var, const char *value, v extern int git_env_bool(const char *, int); extern int git_config_system(void); extern int config_error_nonbool(const char *); +extern const char *git_config_get_string(const char *); +extern const struct string_list *git_config_get_string_multi(const char *); #if defined(__GNUC__) && ! defined(__clang__) #define config_error_nonbool(s) (config_error_nonbool(s), -1) #endif diff --git a/config.c b/config.c index a30cb5c..f6515c2 100644 --- a/config.c +++ b/config.c @@ -9,6 +9,8 @@ #include "exec_cmd.h" #include "strbuf.h" #include "quote.h" +#include "hashmap.h" +#include "string-list.h" struct config_source { struct config_source *prev; @@ -37,6 +39,102 @@ static struct config_source *cf; static int zlib_compression_seen; +struct hashmap config_cache; + +struct config_cache_node { + struct hashmap_entry ent; + struct strbuf key; + struct string_list *value_list ; +}; + +static int hashmap_init_v = 0; + +static int config_cache_node_cmp(const struct config_cache_node *e1, + const struct config_cache_node *e2, const char *key) +{ + return strcmp(e1->key.buf, key ? key : e2->key.buf); +} + +static int config_cache_node_cmp_icase(const struct config_cache_node *e1, + const struct config_cache_node *e2, const char* key) +{ + return strcasecmp(e1->key.buf, key ? key : e2->key.buf); +} + +static void config_cache_init(const int icase) +{ + hashmap_init(&config_cache, (hashmap_cmp_fn) (icase ? config_cache_node_cmp_icase + : config_cache_node_cmp), 0); +} + +static void config_cache_free(void) +{ + hashmap_free(&config_cache, 1); +} + +static struct config_cache_node *config_cache_find_entry(const char *key) +{ + struct hashmap_entry k; + hashmap_entry_init(&k, strihash(key)); + return hashmap_get(&config_cache, &k, key); +} + +static struct string_list *config_cache_get_value(const char *key) +{ + struct config_cache_node *e = config_cache_find_entry(key); + if (e == NULL) + return NULL; + else + return (e->value_list); +} + + +static void config_cache_set_value(const char *key, const char *value) +{ + struct config_cache_node *e; + + if (!value) + return; + + e = config_cache_find_entry(key); + if (!e) { + e = malloc(sizeof(*e)); + hashmap_entry_init(e, strihash(key)); + strbuf_init(&(e->key), 1024); + strbuf_addstr(&(e->key),key); + e->value_list = malloc(sizeof(struct string_list)); + e->value_list->strdup_strings = 1; + e->value_list->nr = 0; + e->value_list->alloc = 0; + e->value_list->items = NULL; + e->value_list->cmp = NULL; + string_list_append(e->value_list, value); + hashmap_add(&config_cache, e); + } + else { + if (!unsorted_string_list_has_string((e->value_list), value)) + string_list_append((e->value_list), value); + } +} + +extern const char *git_config_get_string(const char *name) +{ + struct string_list *values; + int num_values; + char *result; + values = config_cache_get_value(name); + if (!values) + return NULL; + num_values = values->nr; + result = values->items[num_values-1].string ; + return result; +} + +extern const struct string_list *git_config_get_string_multi(const char *key) +{ + return config_cache_get_value(key); +} + static int config_file_fgetc(struct config_source *conf) { return fgetc(conf->u.file); @@ -333,6 +431,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) if (!value) return -1; } + config_cache_set_value(name->buf, value); return fn(name->buf, value, data); } @@ -1135,6 +1234,12 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) char *xdg_config = NULL; char *user_config = NULL; + int icase = 1; + if (!hashmap_init_v) { + config_cache_init(icase); + hashmap_init_v = 1; + } + home_config_paths(&user_config, &xdg_config, "config"); if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0)) { -- 1.9.0.GIT ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC/PATCH 1/2] config: Add cache for config value querying 2014-05-26 17:33 ` [RFC/PATCH 1/2] config: Add cache for config value querying Tanay Abhra @ 2014-05-26 20:02 ` Torsten Bögershausen 2014-05-27 13:54 ` Tanay Abhra 2014-05-28 9:31 ` Eric Sunshine 1 sibling, 1 reply; 7+ messages in thread From: Torsten Bögershausen @ 2014-05-26 20:02 UTC (permalink / raw) To: Tanay Abhra, git; +Cc: Ramkumar Ramachandra, Matthieu Moy On 2014-05-26 19.33, Tanay Abhra wrote: I like the idea. Please allow some minor comments (and read them as questions rather then answers) > Add an internal cache with the all variable value pairs read from the usual "cache": The word "cache" is in Git often used for "index" "variable value" can be written as "key value" "usual": I don't think we handle "unusual" config files, (so can we drop the word usual ?) I think the (important) first line can be written like this: >Add a hash table with the all key-value pairs read from the or >Add a hash table to cache all key-value pairs read from the > config files(repo specific .git/config, user wide ~/.gitconfig and the global > /etc/gitconfig). Also, add two external functions `git_config_get_string` and Can we drop "Also" ? > @@ -37,6 +39,102 @@ static struct config_source *cf; > > static int zlib_compression_seen; > > +struct hashmap config_cache; > + > +struct config_cache_node { > + struct hashmap_entry ent; > + struct strbuf key; Do we need a whole strbuf for the key? Or could a "char *key" work as well? (and/or could it be "const char *key" ? > + struct string_list *value_list ; > +static struct string_list *config_cache_get_value(const char *key) > +{ > + struct config_cache_node *e = config_cache_find_entry(key); why "e" ? Will "node" be easier to read ? Or entry ? > +static void config_cache_set_value(const char *key, const char *value) > +{ > + struct config_cache_node *e; > + > + if (!value) > + return; Hm, either NULL could mean "unset==remove" the value, (but we don't do that, do we? Or it could mean a programming or runtime error?, Should there be a warning ? > + > + e = config_cache_find_entry(key); > + if (!e) { > + e = malloc(sizeof(*e)); > + hashmap_entry_init(e, strihash(key)); > + strbuf_init(&(e->key), 1024); > + strbuf_addstr(&(e->key),key); > + e->value_list = malloc(sizeof(struct string_list)); > + e->value_list->strdup_strings = 1; > + e->value_list->nr = 0; > + e->value_list->alloc = 0; > + e->value_list->items = NULL; > + e->value_list->cmp = NULL; When malloc() is replaced by xcalloc() the x = NULL and y = 0 can go away, and the code is shorter and easier to read. > +extern const char *git_config_get_string(const char *name) > +{ > + struct string_list *values; > + int num_values; > + char *result; > + values = config_cache_get_value(name); > + if (!values) > + return NULL; > + num_values = values->nr; > + result = values->items[num_values-1].string ; We could get rid of the variable "int num_values" by simply writing result = values->items[values->nr-1].string; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC/PATCH 1/2] config: Add cache for config value querying 2014-05-26 20:02 ` Torsten Bögershausen @ 2014-05-27 13:54 ` Tanay Abhra 0 siblings, 0 replies; 7+ messages in thread From: Tanay Abhra @ 2014-05-27 13:54 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: git, Ramkumar Ramachandra, Matthieu Moy Hi, On 05/26/2014 01:02 PM, Torsten Bögershausen wrote: >> Add an internal cache with the all variable value pairs read from the usual > "cache": The word "cache" is in Git often used for "index" Okay, point noted. I thought about choosing between "hashmap" and "cache" and chose the later. > "variable value" can be written as "key value" I had used the term "variable" to be consistent with the documentation (api-config.txt). But I think "key" is much clearer. > "usual": I don't think we handle "unusual" config files, > (so can we drop the word usual ?) Okay, noted. > I think the (important) first line can be written like this: > >> Add a hash table with the all key-value pairs read from the > or >> Add a hash table to cache all key-value pairs read from the > >> config files(repo specific .git/config, user wide ~/.gitconfig and the global >> /etc/gitconfig). Also, add two external functions `git_config_get_string` and > Can we drop "Also" ? >> @@ -37,6 +39,102 @@ static struct config_source *cf; >> >> static int zlib_compression_seen; >> >> +struct hashmap config_cache; >> + >> +struct config_cache_node { >> + struct hashmap_entry ent; >> + struct strbuf key; > Do we need a whole strbuf for the key? > Or could a "char *key" work as well? > (and/or could it be "const char *key" ? To maintain consistency with config.c. config.c uses strbuf for both key and value throughout. I found the reason by git-blaming config.c. Key length is flexible so it would be better to use a api construct such as strbuf for it. >> + struct string_list *value_list ; > > > >> +static struct string_list *config_cache_get_value(const char *key) >> +{ >> + struct config_cache_node *e = config_cache_find_entry(key); > why "e" ? Will "node" be easier to read ? Or entry ? Noted. Entry is much better. >> +static void config_cache_set_value(const char *key, const char *value) >> +{ >> + struct config_cache_node *e; >> + >> + if (!value) >> + return; > Hm, either NULL could mean "unset==remove" the value, (but we don't do that, do we? > > Or it could mean a programming or runtime error?, Should there be a warning ? Nope. It is just a check to not save blank values for a key in the hashmap. Removal functionality will come later. NULL==remove is implemented in git_config_set_multivar_in_file(). We are not reading key value pairs from that, just from git_config(). >> + >> + e = config_cache_find_entry(key); >> + if (!e) { >> + e = malloc(sizeof(*e)); >> + hashmap_entry_init(e, strihash(key)); >> + strbuf_init(&(e->key), 1024); >> + strbuf_addstr(&(e->key),key); >> + e->value_list = malloc(sizeof(struct string_list)); >> + e->value_list->strdup_strings = 1; >> + e->value_list->nr = 0; >> + e->value_list->alloc = 0; >> + e->value_list->items = NULL; >> + e->value_list->cmp = NULL; > When malloc() is replaced by xcalloc() the x = NULL and y = 0 can go away, > and the code is shorter and easier to read. Much better, thanks. >> +extern const char *git_config_get_string(const char *name) >> +{ >> + struct string_list *values; >> + int num_values; >> + char *result; >> + values = config_cache_get_value(name); >> + if (!values) >> + return NULL; >> + num_values = values->nr; >> + result = values->items[num_values-1].string ; > We could get rid of the variable "int num_values" by simply writing > result = values->items[values->nr-1].string; > Noted. Cheers, Tanay Abhra. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC/PATCH 1/2] config: Add cache for config value querying 2014-05-26 17:33 ` [RFC/PATCH 1/2] config: Add cache for config value querying Tanay Abhra 2014-05-26 20:02 ` Torsten Bögershausen @ 2014-05-28 9:31 ` Eric Sunshine 1 sibling, 0 replies; 7+ messages in thread From: Eric Sunshine @ 2014-05-28 9:31 UTC (permalink / raw) To: Tanay Abhra Cc: Git List, Ramkumar Ramachandra, Matthieu Moy, Torsten Bögershausen On Mon, May 26, 2014 at 1:33 PM, Tanay Abhra <tanayabh@gmail.com> wrote: > Add an internal cache with the all variable value pairs read from the usual > config files(repo specific .git/config, user wide ~/.gitconfig and the global s/(/ (/ > /etc/gitconfig). Also, add two external functions `git_config_get_string` and > `git_config_get_string_multi` for querying in an non callback manner from the s/an/a/ s/non/non-/ More below. > cache. > > Signed-off-by: Tanay Abhra <tanayabh@gmail.com> > --- > cache.h | 2 ++ > config.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 107 insertions(+) > > diff --git a/cache.h b/cache.h > index 107ac61..2038150 100644 > --- a/cache.h > +++ b/cache.h > @@ -1271,6 +1271,8 @@ extern int check_repository_format_version(const char *var, const char *value, v > extern int git_env_bool(const char *, int); > extern int git_config_system(void); > extern int config_error_nonbool(const char *); > +extern const char *git_config_get_string(const char *); > +extern const struct string_list *git_config_get_string_multi(const char *); > #if defined(__GNUC__) && ! defined(__clang__) > #define config_error_nonbool(s) (config_error_nonbool(s), -1) > #endif > diff --git a/config.c b/config.c > index a30cb5c..f6515c2 100644 > --- a/config.c > +++ b/config.c > @@ -9,6 +9,8 @@ > #include "exec_cmd.h" > #include "strbuf.h" > #include "quote.h" > +#include "hashmap.h" > +#include "string-list.h" > > struct config_source { > struct config_source *prev; > @@ -37,6 +39,102 @@ static struct config_source *cf; > > static int zlib_compression_seen; > > +struct hashmap config_cache; This should be 'static'. > +struct config_cache_node { > + struct hashmap_entry ent; > + struct strbuf key; Upon reading this, I had the same question as Torsten: Why isn't 'key' a plain 'char *' allocated when the entry is created? Your answer: To maintain consistency with config.c. config.c uses strbuf for both key and value throughout. I found the reason by git-blaming config.c. Key length is flexible so it would be better to use a api construct such as strbuf for it. doesn't make sense. The existing callback-based code re-uses the strbuf for each key as it parses the file, however, you are just storing each key once in the hashmap and never altering it, thus a strbuf is overkill. > + struct string_list *value_list ; Why isn't this just a plain 'struct string_list'? Why the extra indirection of a pointer? Drop space before semicolon. > +}; > + > +static int hashmap_init_v = 0; It's not obvious what "_v" is meant to convey. > +static int config_cache_node_cmp(const struct config_cache_node *e1, > + const struct config_cache_node *e2, const char *key) > +{ > + return strcmp(e1->key.buf, key ? key : e2->key.buf); > +} > + > +static int config_cache_node_cmp_icase(const struct config_cache_node *e1, > + const struct config_cache_node *e2, const char* key) > +{ > + return strcasecmp(e1->key.buf, key ? key : e2->key.buf); > +} > + > +static void config_cache_init(const int icase) 'const int' as a function argument is very unusual in this code base. (I found only a single instance of it in log-tree.[ch]:parse_decorate_color_config.) > +{ > + hashmap_init(&config_cache, (hashmap_cmp_fn) (icase ? config_cache_node_cmp_icase > + : config_cache_node_cmp), 0); > +} > + > +static void config_cache_free(void) > +{ > + hashmap_free(&config_cache, 1); > +} Doesn't this leak the strbuf 'key' and string_list 'value_list'? > +static struct config_cache_node *config_cache_find_entry(const char *key) > +{ > + struct hashmap_entry k; > + hashmap_entry_init(&k, strihash(key)); How does this unconditional use of case-insensitive strihash() interact with the 'icase' argument of config_cache_init()? > + return hashmap_get(&config_cache, &k, key); > +} > + > +static struct string_list *config_cache_get_value(const char *key) > +{ > + struct config_cache_node *e = config_cache_find_entry(key); > + if (e == NULL) Rephrase: if (!e) > + return NULL; > + else > + return (e->value_list); Unnecessary parentheses. > +} A purely subject rewrite of the above: return e ? e->value_list : NULL; > +static void config_cache_set_value(const char *key, const char *value) > +{ > + struct config_cache_node *e; > + > + if (!value) > + return; Again, I had the same questions as Torsten. ("Is this supposed to delete the entry?", "Is it a programmer error?", etc.) From a pure reading, it's not obvious why a NULL 'value' is handled this way. The intent might be clearer if the caller instead performs the check, and invokes config_cache_set_value() only if 'value' is non-NULL. > + e = config_cache_find_entry(key); > + if (!e) { > + e = malloc(sizeof(*e)); > + hashmap_entry_init(e, strihash(key)); Same question as above regarding unconditional case-insensitive strihash() versus the 'icase' argument of config_cache_init(). > + strbuf_init(&(e->key), 1024); Why allocate such a large buffer for each key, especially when most keys likely are quite short. Since the key is never altered after being assigned, this seems like considerable waste. Also, unnecessary parentheses. > + strbuf_addstr(&(e->key),key); Unnecessary parentheses. Add space after comma. > + e->value_list = malloc(sizeof(struct string_list)); > + e->value_list->strdup_strings = 1; > + e->value_list->nr = 0; > + e->value_list->alloc = 0; > + e->value_list->items = NULL; > + e->value_list->cmp = NULL; This string_list initialization code begs for a preparatory patch which adds a string_list_init() function (akin to strbuf_init()). > + string_list_append(e->value_list, value); > + hashmap_add(&config_cache, e); > + } > + else { Cuddle the else with the braces: } else { > + if (!unsorted_string_list_has_string((e->value_list), value)) > + string_list_append((e->value_list), value); Unnecessary parentheses (on both lines). Since there is only an 'if' statement in this 'else' block, you could turn it into an 'else if', thus eliminating one level of indentation. > + } > +} > + > +extern const char *git_config_get_string(const char *name) > +{ > + struct string_list *values; > + int num_values; > + char *result; > + values = config_cache_get_value(name); > + if (!values) > + return NULL; > + num_values = values->nr; > + result = values->items[num_values-1].string ; Add spaces around '-'. Drop space before semicolon. > + return result; Why do you need the 'result' variable if you are returning it immediately after it's assigned? return values->items[num_values - 1].string; Or, taking Torsten's recommendation into consideration to drop num_values: return values->items[values->nr - 1].string; > +} > + > +extern const struct string_list *git_config_get_string_multi(const char *key) > +{ > + return config_cache_get_value(key); > +} > + > static int config_file_fgetc(struct config_source *conf) > { > return fgetc(conf->u.file); > @@ -333,6 +431,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) > if (!value) > return -1; > } > + config_cache_set_value(name->buf, value); > return fn(name->buf, value, data); > } > > @@ -1135,6 +1234,12 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) > char *xdg_config = NULL; > char *user_config = NULL; > > + int icase = 1; > + if (!hashmap_init_v) { > + config_cache_init(icase); > + hashmap_init_v = 1; > + } > + > home_config_paths(&user_config, &xdg_config, "config"); > > if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0)) { > -- > 1.9.0.GIT ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC/PATCH 2/2] config: Add new query functions to the api 2014-05-26 17:33 [RFC/PATCH 0/2] Git config cache & special querying api utilizing the cache Tanay Abhra 2014-05-26 17:33 ` [RFC/PATCH 1/2] config: Add cache for config value querying Tanay Abhra @ 2014-05-26 17:33 ` Tanay Abhra 2014-05-28 9:43 ` Eric Sunshine 1 sibling, 1 reply; 7+ messages in thread From: Tanay Abhra @ 2014-05-26 17:33 UTC (permalink / raw) To: git; +Cc: Tanay Abhra, Ramkumar Ramachandra, Matthieu Moy Add explanations for `git_config_get_string_multi` and `git_config_get_string` which utilize the config cache for querying in an non callback manner for a specific variable. Signed-off-by: Tanay Abhra <tanayabh@gmail.com> --- Documentation/technical/api-config.txt | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..33b5b90 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -77,6 +77,25 @@ To read a specific file in git-config format, use `git_config_from_file`. This takes the same callback and data parameters as `git_config`. +Querying for specific variables +------------------------------- + +For programs wanting to query for specific variables in a non callback +manner, the config API provides two functions `git_config_get_string` +and `git_config_get_string_multi`. They both take a single parameter, + +- a variable as the key string for which the corresponding values will + be retrieved and returned. + +They both read value from an internal cache generated previously from +reading the config files. `git_config_get_string` returns the value with +the highest priority(i.e. value in the repo config will be preferred over +value in user wide config for the same variable). + +`git_config_get_string_multi` returns a `string_list` containing all the +values for that particular variable, sorted in order of increasing +priority. + Value Parsing Helpers --------------------- -- 1.9.0.GIT ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC/PATCH 2/2] config: Add new query functions to the api 2014-05-26 17:33 ` [RFC/PATCH 2/2] config: Add new query functions to the api Tanay Abhra @ 2014-05-28 9:43 ` Eric Sunshine 0 siblings, 0 replies; 7+ messages in thread From: Eric Sunshine @ 2014-05-28 9:43 UTC (permalink / raw) To: Tanay Abhra; +Cc: Git List, Ramkumar Ramachandra, Matthieu Moy On Mon, May 26, 2014 at 1:33 PM, Tanay Abhra <tanayabh@gmail.com> wrote: > Subject: config: Add new query functions to the api This sounds as if you are adding new functions to the header file. Saying "... to the api docs" would been clearer. Better: config: document new query functions > Add explanations for `git_config_get_string_multi` and `git_config_get_string` > which utilize the config cache for querying in an non callback manner for a s/an/a/ s/non/non-/ > specific variable. > > Signed-off-by: Tanay Abhra <tanayabh@gmail.com> One might expect these functions to be documented in the same patch which introduces them. Since this patch is so small, it might make sense just to squash it into patch 1. More below. > --- > Documentation/technical/api-config.txt | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt > index 230b3a0..33b5b90 100644 > --- a/Documentation/technical/api-config.txt > +++ b/Documentation/technical/api-config.txt > @@ -77,6 +77,25 @@ To read a specific file in git-config format, use > `git_config_from_file`. This takes the same callback and data parameters > as `git_config`. > > +Querying for specific variables > +------------------------------- All other section headers in this file capitalize each word. > +For programs wanting to query for specific variables in a non callback s/non-/ > +manner, the config API provides two functions `git_config_get_string` > +and `git_config_get_string_multi`. They both take a single parameter, > + > +- a variable as the key string for which the corresponding values will > + be retrieved and returned. > + > +They both read value from an internal cache generated previously from Minor observation: As this file is documenting the API, mentioning private implementation details such, as the "internal cache", may be frowned upon (though probably doesn't matter in practice since this _is_ a technical document). > +reading the config files. `git_config_get_string` returns the value with > +the highest priority(i.e. value in the repo config will be preferred over > +value in user wide config for the same variable). > + > +`git_config_get_string_multi` returns a `string_list` containing all the > +values for that particular variable, sorted in order of increasing > +priority. > + > Value Parsing Helpers > --------------------- > > -- > 1.9.0.GIT ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-05-28 9:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-26 17:33 [RFC/PATCH 0/2] Git config cache & special querying api utilizing the cache Tanay Abhra 2014-05-26 17:33 ` [RFC/PATCH 1/2] config: Add cache for config value querying Tanay Abhra 2014-05-26 20:02 ` Torsten Bögershausen 2014-05-27 13:54 ` Tanay Abhra 2014-05-28 9:31 ` Eric Sunshine 2014-05-26 17:33 ` [RFC/PATCH 2/2] config: Add new query functions to the api Tanay Abhra 2014-05-28 9:43 ` Eric Sunshine
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).