Re: [PATCH v2 2/2] config: Add hashtable for config parsing & retrieval

From: Jeff King <peff@peff.net>
Date: Jun 17, 2014, 05:46 UTC
Message-ID: <20140617054645.GD29957@sigill.intra.peff.net>
In-Reply-To: <20140617053436.GB29957@sigill.intra.peff.net>

On Tue, Jun 17, 2014 at 01:34:36AM -0400, Jeff King wrote:

Show 10 quoted lines

> You need some mechanism to store entries that are NULL. It may be enough
> to silently convert them into the string "true" inside the cached
> storage. But there may be callers who treat NULL specially (e.g., a
> tri-state true/false/auto that treats a bare boolean as "auto"); you'd
> need to check.
> 
> The other alternative is to use something besides string_list that can
> handle a NULL.  You may also need to give some thought to how such NULLs
> would be handled by git_config_get_string() (since a NULL there also
> means "not found").

After reading your other patch and thinking about config_error_nonbool, I don't think it would be right to silently convert this to "true".

E.g., imagine config like:

  [alias]
  foo

If we interpret alias.foo as a boolean, it means "true". But since alias.* variables are not booleans, the current alias code will produce an error (with config_error_nonbool). Silently converting it to true in the config layer would mean the alias code sees only "true", and rather than generating an error, it would run the command "true".

-Peff