Re: [add-default-config 2/5] adding default to color

From: Jeff King <peff@peff.net>
Date: Nov 13, 2017, 03:55 UTC
Message-ID: <20171113035516.446uznhfl6xlfvmi@sigill.intra.peff.net>
In-Reply-To: <xmqqvaiealsv.fsf@gitster.mtv.corp.google.com>

On Mon, Nov 13, 2017 at 12:40:16PM +0900, Junio C Hamano wrote:

Show 20 quoted lines

> As an aside.  Over time we accumulated quite a many actions that are
> all mutually exclusive by nature.  I have a feeling that we might be
> better off to move away from this implementation.  The only thing
> that we are getting from the current one-bit-in-a-flag-word is that
> we can name the variable "actions" (instead of "action") to pretend
> as if we can be given more than one, and then having to check its
> value with HAS_MULTI_BITS(actions) to confuse ourselves.
> 
> Instead, perhaps we should introduce an "enum action" that includes
> ACTION_UNSPECIFIED that is the initial value for the "action"
> variable, which gets set to ACTION_GET, etc. with OPT_SET_INT().  If
> we really care about erroring out when given
> 
> 	$ git config --add --get foo.bar
> 
> instead of the "last one wins" semantics, we can use OPT_CMDMODE.
> 
> The above is of course outside the scope of this series, and I am
> not sure if it should be done as a preparatory or a follow-up
> clean-up.

Yes, I agree that it's a little confusing, and that an enum is a better representation. The TYPE constants have the same problem.

I _think_ we could use OPT_CMDMODE() for those, too. Despite the name, there is nothing in the parse-options error message that would be inappropriate for something that isn't a cmdmode. Though I care a lot less about "--bool --int" reporting an error (instead of last-one-wins) than I do about "--get --set".

-Peff