Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

From: Karthik Nayak <karthik.188@gmail.com>
Date: Oct 11, 2015, 12:48 UTC
Message-ID: <CAOLa=ZQkjMFXVeJ==myQLjyRs6EcejnYnszYKJLyskFufjeqiA@mail.gmail.com>
In-Reply-To: <xmqqfv1jq4jy.fsf@gitster.mtv.corp.google.com>

On Fri, Oct 9, 2015 at 11:59 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 41 quoted lines

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Then used_atom[] could become something like
>>>
>>>     struct {
>>>      const char *str; /* e.g. "align:position=left,32" */
>>>      struct {
>>>              const char *part0; /* everything before '=' */
>>>                 const char *part1; /* optional */
>>>      } *modifier;
>>>         int modifier_nr;
>>>     } *used_atom;
>>
>> If the goal is to prepare as much as possible when parsing the format
>> string, I'd even push it one step further and have stg like
>>
>>      struct {
>>       const char *str; /* e.g. "align:position=left,32" */
>>       union {
>>               struct {
>>                       int position;
>>                       enum { left, right, center } kind;
>>               } align;
>>                 struct {
>>                       ....;
>>                 } objectname;
>>         int modifier_nr;
>>      } *used_atom;
>>
>> Just a thought, I'm not sure how useful this would be, and this may be
>> too much change for this series (so it may deserve a separate topic).
>
> Yes, if we are willing to enrich the element of valid_atom[] array
> with a type-specific parsing functions, we could even do that.  Then
> populate_value() would not have to do any parsing and just do the
> filling.
>
> I was shooting for a middle ground, but certainly with an eye
> towards such an endgame state in the future.

I like the idea's here, I was trying out what you suggested before Matthieu suggestion.

We could have:

static void parse_atom_modifiers(struct used_atom *atom)
{
    const char *sp = NULL;

    atom->modifier = NULL;
    atom->modifier_nr = 0;

    if ((sp = strchr(atom->str, ':'))) {
        while (sp[1]) {
            const char *equal, *comma, *ep;
            int no = atom->modifier_nr;

            atom->modifier_nr++;
            sp++;
            REALLOC_ARRAY(atom->modifier, atom->modifier_nr);

            equal = strchr(sp, '=');
            comma = strchr(sp, ',');

            if (comma)
                ep = comma;
            else
                ep = sp + strlen(sp);

            if (!equal) {
                atom->modifier[no].part0 = xstrndup(sp, ep - sp);
                atom->modifier[no].part1 = NULL;
            } else {
                atom->modifier[no].part0 = xstrndup(sp, equal - sp);
                atom->modifier[no].part1 = xstrndup(equal + 1, ep - equal - 1);
            }
            sp = ep;
        }
    }
}

or something on these lines for what you suggested. We could improve by having a special parsing function for selected atoms and leave this to be default.

Also does it make sense to integrate these changes here? Or would you like to have another series on this?

-- 
Regards,
Karthik Nayak