Re: [PATCH 7/9] builtin-checkout-index.c: use parse_options()

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Sverre Rabbelier schrieb:
> On Thu, Jul 24, 2008 at 10:08 PM, Michele Ballabio
> <barra_cuda@katamail.com> wrote:
>> On Thursday 24 July 2008, Johannes Schindelin wrote:
>>> On Wed, 23 Jul 2008, Michele Ballabio wrote:
>>>
>>>> +           { OPTION_CALLBACK, 'f', "force", &state, NULL,
>>>> +             "force overwrite of existing files",
>>>> +             PARSE_OPT_NOARG, parse_state_force_cb, 0 },
>>> I wonder if this could not be written as
>>>
>>>               OPT_BOOLEAN('f', "force", &state.force,
>>>                       "force overwrite of existing files"),
>> I did it that way because 'force' is a bitfield.
> 
> I thought there is an OPT_BIT?

OPT_BIT is for flags and bitmasks, not for bitfields.

Since you can't get the address of a bitfield member, a function that
wants to change its value needs to know its name.  Switching to bitmasks
would make the option parsing code look cleaner, but you'd have to
change all those bitfield accesses to explicit bitmask operations, e.g.:

	if (state.force)
		state.force = 0;

vs.

	if (state.flags & CHECKOUT_FORCE)
		state.flags &= ~CHECKOUT_FORCE;

In the case of struct checkout, though, we could simply make the
bitfield members full ints, because there are only a few instances of
this structure in memory at any given time.  Wasting a few bytes of RAM
in order to gain much simpler code is OK in this case, I think.
OPT_BOOLEAN looks a lot nicer than a callback.

René