On Fri, Oct 9, 2015 at 11:59 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:
>
>> Junio C Hamano <gitster@xxxxxxxxx> 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
--
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