Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > diff --git a/config.h b/config.h > index 7606246531a..7dd62ca81bf 100644 > --- a/config.h > +++ b/config.h > @@ -465,6 +465,9 @@ void git_configset_clear(struct config_set *cs); > * value in the 'dest' pointer. > */ > > +RESULT_MUST_BE_USED > +int git_configset_get(struct config_set *cs, const char *key); IIRC, feedback on v4 [1] mentioned that since git_configset_get() can return negative values, it probably shouldn't come under this comment: /* * These functions return 1 if not found, and 0 if found, leaving the found * value in the 'dest' pointer. */ I think moving it to before the comment would suffice, maybe with a pointer to the corresponding repo_* or git_*. 1. https://lore.kernel.org/git/xmqqv8kjpqoe.fsf@gitster.g/ > @@ -485,6 +488,14 @@ int git_configset_get_pathname(struct config_set *cs, const char *key, const cha > /* Functions for reading a repository's config */ > struct repository; > void repo_config(struct repository *repo, config_fn_t fn, void *data); > + > +/** > + * Run only the discover part of the repo_config_get_*() functions > + * below, in addition to 1 if not found, returns negative values on > + * error (e.g. if the key itself is invalid). > + */ > +RESULT_MUST_BE_USED > +int repo_config_get(struct repository *repo, const char *key); > int repo_config_get_value(struct repository *repo, > const char *key, const char **value); > const struct string_list *repo_config_get_value_multi(struct repository *repo, > @@ -521,8 +532,15 @@ void git_protected_config(config_fn_t fn, void *data); > * manner, the config API provides two functions `git_config_get_value` > * and `git_config_get_value_multi`. They both read values from an internal > * cache generated previously from reading the config files. > + * > + * For those git_config_get*() functions that aren't documented, > + * consult the corresponding repo_config_get*() function's > + * documentation. > */ After rereading config.h, I really appreciate comments like this that try to control the documentation load. We have configset*, repo* and git*, _and_ the comments are spread out hapzardly around config.h with no pointers to the corresponding comments. I think we're overdue for reorganization, and this sort of comment helps a lot with that. As a suggestion, it seems like the git_config_get*() functions are actually the better documented ones - nearly all of them have comments, whereas the repo_config_get_*() ones typically don't, so maybe adding the comment to git_config_get() instead of repo_config_get() would be better for this series: ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 ---- diff --git a/config.h b/config.h index 7dd62ca81b..aa9bdf8df4 100644 --- a/config.h +++ b/config.h @@ -489,10 +489,10 @@ int git_configset_get_pathname(struct config_set *cs, const char *key, const cha struct repository; void repo_config(struct repository *repo, config_fn_t fn, void *data); -/** - * Run only the discover part of the repo_config_get_*() functions - * below, in addition to 1 if not found, returns negative values on - * error (e.g. if the key itself is invalid). +/* + * These repo_config_get*() functions each correspond to to a git_config_get*() + * function. Consult the corresponding git_config_get*() documentation for more + * information. */ RESULT_MUST_BE_USED int repo_config_get(struct repository *repo, const char *key); @@ -532,12 +532,13 @@ void git_protected_config(config_fn_t fn, void *data); * manner, the config API provides two functions `git_config_get_value` * and `git_config_get_value_multi`. They both read values from an internal * cache generated previously from reading the config files. - * - * For those git_config_get*() functions that aren't documented, - * consult the corresponding repo_config_get*() function's - * documentation. */ +/** + * Run only the discover part of the repo_config_get_*() functions + * below, in addition to 1 if not found, returns negative values on + * error (e.g. if the key itself is invalid). + */ RESULT_MUST_BE_USED int git_config_get(const char *key); ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 ---- Though in the long run, I'd prefer having the docs on the more "general" APIs (configset_*, repo_*) instead of the more "specific" ones (git_*). Perhaps you had a similar intent while making this change, but I think this might be better left as a cleanup. As an aside, I really appreciate your effort in sticking with the config interface work. I think it's grown quite unruly, and it's worth trying to tame it.
