On Sun, Dec 13, 2015 at 3:32 AM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>> On Sun, Dec 13, 2015 at 6:23 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>>> On Wed, Nov 11, 2015 at 2:44 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>>>> Introduce remote_ref_atom_parser() which will parse the '%(upstream)'
>>>> and '%(push)' atoms and store information into the 'used_atom'
>>>> structure based on the modifiers used along with the corresponding
>>>> atom.
>>>>
>>>> Signed-off-by: Karthik Nayak <Karthik.188@xxxxxxxxx>
>>>> ---
>>>> diff --git a/ref-filter.c b/ref-filter.c
>>>> @@ -37,6 +37,11 @@ static struct used_atom {
>>>> union {
>>>> const char *color;
>>>> struct align align;
>>>> + struct {
>>>> + unsigned int shorten : 1,
>>>> + track : 1,
>>>> + trackshort : 1;
>>>> + } remote_ref;
>>>
>>> Are 'shorten', 'track', and 'trackshort' mutually exclusive? If so, a
>>> simple enum would be clearer than bitfields:
>>>
>>> union {
>>> const char *color;
>>> struct align align;
>>> enum { RR_PLAIN, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
>>> remote_ref;
>>> };
>>>
>>> Or something.
>
> There's also a slight problem with using enum's with the current implementation.
> The problem is the enum is set to 0 by default (since we use memset).
> so the first value is set by default, not something we'd want.
I'm afraid I don't see the problem. Doesn't the RR_PLAIN in the
example cover this case?
> So either we stick to the structure
> with unsigned bits or we introduce a pseudo value in the enum. I
> prefer the former.
It's not a pseudo-value, but rather just one of the mutually exclusive states.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html