Re: [PATCH v3 08/15] ref-filter: introduce color_atom_parser()

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 5, 2016, 21:06 UTC
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:

Show 9 quoted lines

> @@ -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.