Re: [PATCH v2 07/11] ref-filter: introduce align_atom_parser()

On Fri, Dec 18, 2015 at 11:20 AM, Eric Sunshine [off-list ref] wrote:

On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak [off-list ref] wrote:

quoted

Introduce align_atom_parser() which will parse an "align" atom and
store the required alignment position and width in the "use_atom"
structure for further usage in 'populate_value()'.

Helped-by: Ramsay Jones [off-list ref]
Signed-off-by: Karthik Nayak <redacted>
---

diff --git a/ref-filter.c b/ref-filter.c

@@ -63,6 +69,49 @@ static void color_atom_parser(struct used_atom *atom)
+static void align_atom_parser(struct used_atom *atom)
+{
+       [...]
+       match_atom_name(atom->str, "align", &buf);
+       if (!buf)
+               die(_("expected format: %%(align:<width>,<position>)"));
+       [...]
+}

@@ -880,35 +924,7 @@ static void populate_value(struct ref_array_item *ref)
                                v->s = " ";
                        continue;
                } else if (match_atom_name(name, "align", &valp)) {

Hmm, align_atom_parser() has already been called before we get here,
right? And it has already invoked match_atom_name() and died if the
argument to "align:" was missing, correct? If so, then this invocation
of match_atom_name() in populate_value() is unnecessary and should be
replaced with a simpler starts_with("align:").

Plus, 'valp' is not used, aside from this unnecessary
match_atom_name(), thus it can be removed as well.

WIll do. Thanks for noticing this. I missed it.

-- 
Regards,
Karthik Nayak