On Sun, Dec 13, 2015 at 3:32 AM, Karthik Nayak <karthik....@gmail.com> wrote:
>> On Sun, Dec 13, 2015 at 6:23 AM, Eric Sunshine <sunsh...@sunshineco.com> 
>> wrote:
>>> On Wed, Nov 11, 2015 at 2:44 PM, Karthik Nayak <karthik....@gmail.com> 
>>> 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....@gmail.com>
>>>> ---
>>>> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to