On Sun, Dec 13, 2015 at 2:15 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> 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.
This example is actually fine, the next one "%(contents)" is more of
the problem,
the check is done in grab_sub_body_contents() where previously "contents.all"
would be enough to check if we need to add contents value. Now the first enum
value is selected. Maybe have a "NOT_VALID" field in the enum.
--
Regards,
Karthik Nayak
--
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