From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 05 Jan 2016 21:06:42 GMT
Subject: Re: [PATCH v3 08/15] ref-filter: introduce color_atom_parser()
Message-ID: <xmqqr3hvivml.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1451980994-26865-9-git-send-email-Karthik.188@gmail.com>

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

> @@ -90,7 +105,7 @@ static struct {
>  	{ "symref" },
>  	{ "flag" },
>  	{ "HEAD" },
> -	{ "color" },
> +	{ "color", FIELD_STR, color_atom_parser },
>  	{ "align" },
>  	{ "end" },
>  };

This is minor, as I do not think anybody sane would say
"for-each-ref --sort=color" and expect anything sensible, but

I think we shouldn't mark these bogus attributes [*1*] as FIELD_STR
(and it is not FIELD_ULONG, either).  Perhaps add a new FIELD_BOGUS
(or some better name to make it clear that this is not a "value"
that belongs to the ref and can be used to sort, e.g. "FAKE") value
and mark them as such?

That would allow us to error out when they are specified as a
sorting criteria.


[Footnote]

*1* Bogus from the point of view of "what are the various attributes
specific to these items that the command is iterating over,
i.e. refs (or the objects at the tips of these refs)", as color,
align, etc. are not attributes per refs.