On Tue, Jun 13, 2017 at 09:40:49AM -0700, Junio C Hamano wrote: > >> It is really tempting to avoid the extra strbuf, but then the error > >> message would change from > >> > >> error: missing value for 'alias.br' > >> > >> to > >> > >> error: missing value for 'br' > >> > >> which is of course no good at all. > >> > >> And since I already have to keep that strbuf, I'll simply keep the > >> config_key_is_valid() guard, too (because why not). > > > > Oof, yeah, that is definitely worse. I'm fine with keeping both parts. > > When you replace Dscho's "compare 'var' with 'alias.br' that is in > strbuf naively with the "skip-prefix and compare with br" without > changing anything else, i.e. > > if (skip_prefix(var, "alias.", &key) && !strcmp(key, data->key)) > return git_config_string((const char **)&data->v, key, value); > > it would cause the "br" to be fed to git_config_string() and result > in problem reported for "br", not "alias.br". > > But this can be trivially fixed by passing "var" instead of "key" to > git_config_string(), no? Am I mistaken? Right, I wasn't thinking. That is a perfectly good solution, and matches the usual mechanism in other config callbacks. -Peff