Re: [PATCH v3 6/6] notes-util.c: replace `git_config()` with `git_config_get_value()`

From: Jeff King <peff@peff.net>
Date: Jul 22, 2014, 11:23 UTC
Message-ID: <20140722112311.GB386@peff.net>
In-Reply-To: <1405941145-12120-7-git-send-email-tanayabh@gmail.com>

On Mon, Jul 21, 2014 at 04:12:25AM -0700, Tanay Abhra wrote:

Show 13 quoted lines

> -static int notes_rewrite_config(const char *k, const char *v, void *cb)
> +static void notes_rewrite_config(struct notes_rewrite_cfg *c)
>  {
> -	struct notes_rewrite_cfg *c = cb;
> -	if (starts_with(k, "notes.rewrite.") && !strcmp(k+14, c->cmd)) {
> -		c->enabled = git_config_bool(k, v);
> -		return 0;
> -	} else if (!c->mode_from_env && !strcmp(k, "notes.rewritemode")) {
> +	const char *v;
> +	struct strbuf key = STRBUF_INIT;
> +	strbuf_addf(&key, "notes.rewrite.%s", c->cmd);
> +	git_config_get_bool(key.buf, &c->enabled);
> +	strbuf_release(&key);

I wonder if it is worth teaching the accessors to form such strings themselves, like:

  void git_config_get_bool(int *out, const char *fmt, ...);

so you could do:

  git_config_get_bool(&c->enabled, "notes.rewrite.%s", c->cmd);

The "normal" cases where we do not need any run-time modification could be used as-is (I swapped the parameter order above, but you would not have to do so). But I guess that would require us doing extra work in the common "normal" case to print the string into a buffer, even though it does not have any expansions (or to do a strchr, I guess, to look for "%"). It's probably not worth it considering how few config keys have computed values like this.

-Peff