Jeff King <peff@xxxxxxxx> writes:
> On Tue, Jun 13, 2017 at 01:42:02PM +0200, Johannes Schindelin wrote:
>
>> > As you probably guessed, I had tried that first and then figured that if
>> > I needed to keep the config_key_is_valid() test anyway, I could just as
>> > well keep the strbuf around for later use.
>> >
>> > Will change the code,
>>
>> Alas, I won't change the code after all.
>>
>> 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?