Re: [PATCH v4 06/12] ref-filter: introduce color_atom_parser()

On Sat, Feb 6, 2016 at 4:20 PM, Karthik Nayak [off-list ref] wrote:

On Fri, Feb 5, 2016 at 3:55 AM, Eric Sunshine [off-list ref] wrote:

quoted

On Sun, Jan 31, 2016 at 12:42 PM, Karthik Nayak [off-list ref] wrote:

quoted

Introduce color_atom_parser() which will parse a "color" atom and
store its color in the "used_atom" structure for further usage in
populate_value().

Signed-off-by: Karthik Nayak <redacted>
---

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

@@ -29,10 +29,21 @@ typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 static struct used_atom {
        const char *name;
        cmp_type type;
+       union {
+               char color[COLOR_MAXLEN];
+       } u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
 static int need_color_reset_at_eol;

+static void color_atom_parser(struct used_atom *atom, const char *color_value)
+{
+       if (!color_value)
+               die(_("expected format: %%(color:<color>)"));
+       if (color_parse(color_value, atom->u.color) < 0)
+               die(_("invalid color value: %s"), atom->u.color);

Shouldn't this be:

    die(_("invalid color value: %s"), color_value);

?

Oops. You're right, it should.
Also the error is reported already in color_parse(...), so seems duplicated.

e.g.

git for-each-ref  --format="%(color:sfadf)%(align:middle,30)%(refname)%(end)"
error: invalid color value: sfadf
fatal: invalid color value: sfadf

What would be an ideal way around this?

Maybe it has already been discussed a lot and I missed the discussion,
but if possible the argument, the parameter or the atom itself might
just be ignored with a warning instead of dying when an atom argument,
format or parameter is not recognized, because in the next Git
versions we might want to add new arguments, formats and parameter and
it would be sad if old versions of Git die when those new things are
passed to them.