Hi Peff,
On Sat, 10 Jun 2017, Jeff King wrote:
> On Thu, Jun 08, 2017 at 09:53:50PM +0200, Johannes Schindelin wrote:
>
> > -char *alias_lookup(const char *alias)
> > [...]
> > {
> > - char *v = NULL;
> > - struct strbuf key = STRBUF_INIT;
> > - strbuf_addf(&key, "alias.%s", alias);
> > - if (git_config_key_is_valid(key.buf))
> > - git_config_get_string(key.buf, &v);
> > - strbuf_release(&key);
> > - return v;
> > + struct strbuf key;
> > + char *v;
> > +};
> > [...]
> > +char *alias_lookup(const char *alias, struct strbuf *cdup_dir)
> > +{
> > + struct config_alias_data data = { STRBUF_INIT, NULL };
> > +
> > + strbuf_addf(&data.key, "alias.%s", alias);
> > + if (git_config_key_is_valid(data.key.buf))
> > + read_early_config(config_alias_cb, &data, cdup_dir);
> > + strbuf_release(&data.key);
> > +
> > + return data.v;
> > }
>
> Two optional cleanups here:
>
> 1. We don't need to call config_key_is_valid when using a callback. We
> only needed that to prevent the configset machinery from issuing a
> warning. It does save us reading the config entirely when the
> program name is syntactically invalid as an alias, but that's a
> pretty rare case.
It may be a pretty rare case, or it may not be. I do not want to think
hard about this, so I just wanted to keep that test.
But since you suggested it, I will simply blame all the fallout (if any)
on you.
;-)
> 2. Now that we're not using the configset machinery, we don't need to
> have the alias name as a full string. Instead of using the strbuf,
> we could just pass the "alias" string itself and do:
>
> if (skip_prefix(var, "alias.", &key) && !strcmp(key, data->key))
>
> in the callback.
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,
Dscho
