Re: [PATCH v4 05/10] ref-filter: add support to sort by version

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jul 27, 2015, 17:03 UTC
Message-ID: <CAOLa=ZQrBbgeYjnVaLf4EV+S1aRYBjJ-G==K_1J8XbGUCp5mmg@mail.gmail.com>
In-Reply-To: <xmqqvbd5ipy1.fsf@gitster.dls.corp.google.com>

On Mon, Jul 27, 2015 at 8:54 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 34 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> On Sun, Jul 26, 2015 at 4:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> Without looking at the callers, s->version looks like a misdesign
>>> that should be updated to use the same cmp_type mechanism?  That
>>> would lead to even more obvious construct that is easy to enhance,
>>> i.e.
>>>
>>>         switch (cmp_type) {
>>>         case CMP_VERSION:
>>>                 ...
>>>         case CMP_STRING:
>>>                 ...
>>>         case CMP_NUMBER:
>>>                 ...
>>>         }
>>>
>>> I dunno.
>>>
>>> Other than that (and the structure of that "format-state" stuff we
>>> discussed separately), the series was a pleasant read.
>>>
>>> Thanks.
>>
>> That was the previous design, but Duy asked me to do this so
>> that we could support all atoms. And I agree with him on this.
>>
>> http://article.gmane.org/gmane.comp.version-control.git/273888
>
> I am not objecting, but $gmane/273888 does not immediately read, at
> least to me, as suggesting using a mechanism different from cmp_type
> but a dedicated field s->version.  Puzzled...
>

What I mean was since "version"/"v" aren't atoms as such, their processing is done before we parse through atoms and fill used_atoms and used_atom_type. cmp_type is obtained from the used_atom_type, which isn't filled by "version"/"v". Hence the dedicated field, just like reverse.

-- 
Regards,
Karthik Nayak