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

Jeff King [off-list ref] writes:

quoted

@@ -47,6 +48,7 @@ static int show_origin;
 #define ACTION_GET_COLOR (1<<13)
 #define ACTION_GET_COLORBOOL (1<<14)
 #define ACTION_GET_URLMATCH (1<<15)
+#define ACTION_GET_COLORORDEFAULT (1<<16)

I'm not sure I understand this part, though. Providing a default should
be something that goes along with a "get" action, but isn't its own
action.

I agree that it is not.

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.

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

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