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

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jul 10, 2015, 10:52 UTC
Message-ID: <CAOLa=ZRs560_YBVMTpfKBvB1BF6ts+X_JGFTzhp7A_+YsjwqqQ@mail.gmail.com>
In-Reply-To: <vpqfv4x792z.fsf@anie.imag.fr>

On Thu, Jul 9, 2015 at 6:59 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 12 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Add support to sort by version using the "v:refname" and
>> "version:refname" option. This is achieved by using the
>> 'version_cmp()' function as the comparing function for qsort.
>
> You should elaborate on why you need this. Given the context, I can
> guess that you will need this to implement tag, but for example I first
> wondered why you needed both version: and v:, but I guess it comes from
> the fact that 'git tag --sort' can take version:refname or v:refname.
>
> I think this deserves a test and documentation in for-each-ref.txt.

I'll add it to "for-each-ref.txt" documentation. About the tests, there are already tests for the same in git-tag.txt and that's the only reason I did not repeat the tests in for-each-ref.

Show 14 quoted lines

>
> As-is, the code is a bit hard to understand. I first saw you were
> allowing
>
>   git for-each-ref --format '%(version:refname)'
>
> (which you are, but only as a side effect), and then understood that
> this was also allowing
>
>   git for-each-ref --sort version:refname
>
> A test would have shown me this immediately. Some hints in the commit
> message would clearly have helped too.
>

Most of the sorting options have side effects in "--format", but yeah will add details in the commit message.

-- 
Regards,
Karthik Nayak