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

Karthik Nayak <karthik.188@gmail.com> writes:

At least you should be able to pre-parse the %(<atom>:<modifier>) construct, instead of doing strcmp() every time populate_value() is called, no? Then your parser would not be doing

with 'valp' that scans over 'name' (which is an element of used_atom[], i.e. a name from valid_atom[] followed by ":<modifier>" for each customization). Instead your used_atom[] would be an array of a data structure that is richer than just a string.

The <modifier> would be split at comma boundary and made into an array of two field structure, as the most general form of it is

    <modifier> ::= <attrval> | <attrval> ',' <modifier>
    <attrval> ::= <attr> '=' <value> | <attr> | <value>

if we recall the discussion we had while designing %(align), i.e.

 * The most general form is "%(align:position=left,width=32)"

 * The value domain of attributes may be distinct, in which case the
   value itself could imply what attr it is talking about,
   e.g. "%(align:32,left)" is sufficiently clear as '32' cannot be
   position and 'left' cannot be width.

And clearly there can be an attr whose presense alone can imply its value (iow, a boolean), even though your %(align) does not yet have such a modifier. It is easy to imagine that you would later want to add %(align:position=left,width=32,truncate) that truncates the value when its display width exceeds '32'. The 'nobracket' above would be an another example of a boolean (whose name is negative, which is not a very good design in general, though).

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;

and "align:position=left,32" would be parsed into

	.str = "align:position=left,32";
        .modifier = { { "position", "left" }, { "32", NULL } };
        .modifier_nr = 2;

when the format string is read, which is done only once.

The looping over all the refs done in populate_value() could just use the pre-parsed representation.

Hmm?