On Sat, Feb 6, 2016 at 4:20 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
> On Fri, Feb 5, 2016 at 3:55 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>> On Sun, Jan 31, 2016 at 12:42 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>>> 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 <Karthik.188@xxxxxxxxx>
>>> ---
>>> 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.
--
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