Re: [PATCH/RFC 00/10] ref-filter: use parsing functions

> On Sun, Dec 13, 2015 at 11:10 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> If the intention is to rid that inner loop of much of the expensive
>> processing, then wouldn't we want to introduce an enum of valid atoms
>> which is to be a member of 'struct used_atom', and have
>> populate_value() switch on the enum value rather than doing all the
>> expensive strcmp()s and starts_with()?
>>
>>     enum atom_type {
>>         AT_REFNAME,
>>         AT_OBJECTTYPE,
>>         ...
>>         AT_COLOR,
>>         AT_ALIGN
>>     };
>>
>>     static struct used_atom {
>>         enum atom_type atom;
>>         cmp_type cmp;
>>         union {
>>             char *color; /* parsed color */
>>             struct align align;
>>             enum { ... } remote_ref;
>>             struct {
>>                 enum { ... } portion;
>>                 unsigned int nlines;
>>             } contents;
>>             int short_objname;
>>         } u;
>>     } *used_atom;
>>
>> In fact, the 'cmp_type cmp' field can be dropped altogether since it
>> can just as easily be looked up when needed by keeping 'enum
>> atom_type' and valid_atoms[] in-sync and indexing into valid_atoms[]
>> by atom_type:
>>
>>     struct used_atom *a = ...;
>>     cmp_type cmp = valid_atoms[a->atom].cmp_type;
>>
>> As a bonus, an 'enum atom_type' would resolve the problem of how
>> starts_with() is abused in populate_value() for certain cases
>> (assuming I'm understanding the logic), such as how matching of
>> "color" could incorrectly match some yet-to-be-added atom named
>> "colorize".
>
> This actually makes sense to me, especially how we can eliminate most of
> the starts_with() in populate_value(). Well then I guess I'll work on this and
> see what I can come up with, Thanks for the review :)