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

[Eric Sunshine <sunshine@sunshineco.com>]

On Tuesday, July 28, 2015, Karthik Nayak <karthik.188@gmail.com> wrote:
> Add support to sort by version using the "v:refname" and
> "version:refname" option. This is achieved by using the 'versioncmp()'
> function as the comparing function for qsort.
>
> This option is included to support sorting by versions in `git tag -l`
> which will eventaully be ported to use ref-filter APIs.
>
> Add documentation and tests for the same.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 45dd7f8..2b60aee 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -151,6 +151,9 @@ For sorting purposes, fields with numeric values sort in numeric
>  order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
>  All other fields are used to sort in their byte-value order.
>
> +There is also an option to sort by versions, this can be done by using
> +the fieldname `version:refname` or in short `v:refname`.

Nit: "or in short" reads a bit oddly. Perhaps:

    ...`version:refname` or its alias `v:refname`.

or

    ...`version:refname` or the short-form `v:refname`.

(I rather prefer the "alias" alternative.)

>  In any case, a field name that refers to a field inapplicable to
>  the object referred by the ref does not cause an error.  It
>  returns an empty string instead.
> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
> index 19ac480..68688a9 100755
> --- a/t/t6302-for-each-ref-filter.sh
> +++ b/t/t6302-for-each-ref-filter.sh
> @@ -97,4 +97,40 @@ test_expect_success 'padding to the right using `padright`' '
> +test_expect_success 'version sort' '
> +       git for-each-ref --sort=version:refname --format="%(refname:short)" refs/tags/ | grep "foo" >actual &&
> +       cat >expect <<-\EOF &&
> +       foo1.3
> +       foo1.6
> +       foo1.10
> +       EOF
> +       test_cmp expect actual
> +'
> +
> +test_expect_success 'version sort (shortened)' '
> +       git for-each-ref --sort=v:refname --format="%(refname:short)" refs/tags/ | grep "foo" >actual &&
> +       cat >expect <<-\EOF &&
> +       foo1.3
> +       foo1.6
> +       foo1.10
> +       EOF
> +       test_cmp expect actual

Nit: In the earlier review when I suggested using "v:refname" for one
of the tests in order to exercise it (in addition to
"version:refname"), I didn't mean that you had to add another
(identical) test but rather that you could have one of the existing
tests use "v:refname". (Not a big deal. You can leave this as is if
you like. I just wanted to clarify.)

> +'
> +
> +test_expect_success 'reverse version sort' '
> +       git for-each-ref --sort=-version:refname --format="%(refname:short)" refs/tags/ | grep "foo" >actual &&
> +       cat >expect <<-\EOF &&
> +       foo1.10
> +       foo1.6
> +       foo1.3
> +       EOF
> +       test_cmp expect actual
> +'
> +
>  test_done
> --
> 2.4.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html