On Mon, Dec 11, 2023 at 07:52:28PM -0500, Jeff King wrote:

On Thu, Dec 07, 2023 at 09:14:36AM +0100, Patrick Steinhardt wrote:

quoted

Thanks for working on this topic! I've left a couple of comments, most
of which are about whether we should retain previous behaviour where we
generate a warning instead of raising an error for unknown values.

Thanks for taking a look. I see what you're saying about the warnings,
but IMHO it's not worth the extra complexity. Returning early means the
existing code can proceed without worrying about NULLs. Though I suppose
we could have a "warn_error_nonbool()" which issues a warning and
returns 0.

Still, the "return config_error_nonbool()" pattern is pretty
well-established and used for most options. I would go so far as to say
the ones that warn for invalid values are the odd ones out, and probably
should be returning errors as well (though doing so now may not be worth
the trouble and risk of annoyance).

And certainly there should be no regressions in this series; every case
is currently a segfault, so returning an error is a strict improvement.
I'd just as soon stay strict there, as it's easier to loosen later if we
choose than to tighten.

Fair enough, I'm perfectly fine with this reasoning. Thanks!

Patrick