@matheusbsbs / matheus-bagliano / commits / 56d26ade97

ref-filter.c: really don't sort when using --no-sort

When '--no-sort' is passed to 'for-each-ref', 'tag', and 'branch', the printed refs are still sorted by ascending refname. Change the handling of sort options in these commands so that '--no-sort' to truly disables sorting. '--no-sort' does not disable sorting in these commands is because their option parsing does not distinguish between "the absence of '--sort'" (and/or values for tag.sort & branch.sort) and '--no-sort'. Both result in an empty 'sorting_options' string list, which is parsed by 'ref_sorting_options()' to create the 'struct ref_sorting *' for the command. If the string list is empty, 'ref_sorting_options()' interprets that as "the absence of '--sort'" and returns the default ref sorting structure (equivalent to "refname" sort). To handle '--no-sort' properly while preserving the "refname" sort in the "absence of --sort'" case, first explicitly add "refname" to the string list *before* parsing options. This alone doesn't actually change any behavior, since 'compare_refs()' already falls back on comparing refnames if two refs are equal w.r.t all other sort keys. Now that the string list is populated by default, '--no-sort' is the only way to empty the 'sorting_options' string list. Update 'ref_sorting_options()' to return a NULL 'struct ref_sorting *' if the string list is empty, and add a condition to 'ref_array_sort()' to skip the sort altogether if the sort structure is NULL. Note that other functions using 'struct ref_sorting *' do not need any changes because they already ignore NULL values. Finally, remove the condition around sorting in 'ls-remote', since it's no longer necessary. Unlike 'for-each-ref' et. al., it does *not* do any sorting by default. This default is preserved by simply leaving its sort key string list empty before parsing options; if no additional sort keys are set, 'struct ref_sorting *' is NULL and sorting is skipped. Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Victoria Dye committed Nov 14, 2023 at 19:53 UTC 56d26ade97135614ccc60cb215d6c9cb22babfb1

8 files changed +153 -26

builtin/branch.c

@@ -767,7 +767,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)

767 if (argc == 2 && !strcmp(argv[1], "-h"))

768 usage_with_options(builtin_branch_usage, options);

769

770 + /*

771 + * Try to set sort keys from config. If config does not set any,

772 + * fall back on default (refname) sorting.

773 + */

774 git_config(git_branch_config, &sorting_options);

775 + if (!sorting_options.nr)

776 + string_list_append(&sorting_options, "refname");

777

778 track = git_branch_track;

779

builtin/for-each-ref.c

@@ -67,6 +67,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)

68 git_config(git_default_config, NULL);

70 + /* Set default (refname) sorting */

71 + string_list_append(&sorting_options, "refname");

72 +

73 parse_options(argc, argv, prefix, opts, for_each_ref_usage, 0);

74 if (maxcount < 0) {

75 error("invalid --count argument: `%d'", maxcount);

builtin/ls-remote.c

+4 -7

@@ -58,6 +58,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)

58 struct transport *transport;

59 const struct ref *ref;

60 struct ref_array ref_array;

61 + struct ref_sorting *sorting;

62 struct string_list sorting_options = STRING_LIST_INIT_DUP;

64 struct option options[] = {

@@ -141,13 +142,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)

142 item->symref = xstrdup_or_null(ref->symref);

143 }

144

144 - if (sorting_options.nr) {

145 - struct ref_sorting *sorting;

146 -

147 - sorting = ref_sorting_options(&sorting_options);

148 - ref_array_sort(sorting, &ref_array);

149 - ref_sorting_release(sorting);

150 - }

145 + sorting = ref_sorting_options(&sorting_options);

146 + ref_array_sort(sorting, &ref_array);

147

148 for (i = 0; i < ref_array.nr; i++) {

149 const struct ref_array_item *ref = ref_array.items[i];

@@ -157,6 +153,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)

153 status = 0; /* we found something */

154 }

155

156 + ref_sorting_release(sorting);

157 ref_array_clear(&ref_array);

158 if (transport_disconnect(transport))

159 status = 1;

builtin/tag.c

@@ -501,7 +501,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)

501

502 setup_ref_filter_porcelain_msg();

503

504 + /*

505 + * Try to set sort keys from config. If config does not set any,

506 + * fall back on default (refname) sorting.

507 + */

508 git_config(git_tag_config, &sorting_options);

509 + if (!sorting_options.nr)

510 + string_list_append(&sorting_options, "refname");

511

512 memset(&opt, 0, sizeof(opt));

513 filter.lines = -1;

ref-filter.c

+3 -16

@@ -3058,7 +3058,8 @@ void ref_sorting_set_sort_flags_all(struct ref_sorting *sorting,

3058

3059 void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)

3060 {

3061 - QSORT_S(array->items, array->nr, compare_refs, sorting);

3061 + if (sorting)

3062 + QSORT_S(array->items, array->nr, compare_refs, sorting);

3063 }

3064

3065 static void append_literal(const char *cp, const char *ep, struct ref_formatting_state *state)

@@ -3164,18 +3165,6 @@ static int parse_sorting_atom(const char *atom)

3165 return res;

3166 }

3167

3167 -/* If no sorting option is given, use refname to sort as default */

3168 -static struct ref_sorting *ref_default_sorting(void)

3169 -{

3170 - static const char cstr_name[] = "refname";

3171 -

3172 - struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));

3173 -

3174 - sorting->next = NULL;

3175 - sorting->atom = parse_sorting_atom(cstr_name);

3176 - return sorting;

3177 -}

3178 -

3168 static void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg)

3169 {

3170 struct ref_sorting *s;

@@ -3199,9 +3188,7 @@ struct ref_sorting *ref_sorting_options(struct string_list *options)

3188 struct string_list_item *item;

3189 struct ref_sorting *sorting = NULL, **tail = &sorting;

3190

3202 - if (!options->nr) {

3203 - sorting = ref_default_sorting();

3204 - } else {

3191 + if (options->nr) {

3192 for_each_string_list_item(item, options)

3193 parse_ref_sorting(tail, item->string);

3194 }

t/t3200-branch.sh

+65 -3

@@ -1558,9 +1558,10 @@ test_expect_success 'tracking with unexpected .fetch refspec' '

1558

1559 test_expect_success 'configured committerdate sort' '

1560 git init -b main sort &&

1561 + test_config -C sort branch.sort "committerdate" &&

1562 +

1563 (

1564 cd sort &&

1563 - git config branch.sort committerdate &&

1565 test_commit initial &&

1566 git checkout -b a &&

1567 test_commit a &&

@@ -1580,9 +1581,10 @@ test_expect_success 'configured committerdate sort' '

1581 '

1582

1583 test_expect_success 'option override configured sort' '

1584 + test_config -C sort branch.sort "committerdate" &&

1585 +

1586 (

1587 cd sort &&

1585 - git config branch.sort committerdate &&

1588 git branch --sort=refname >actual &&

1589 cat >expect <<-\EOF &&

1590 a

@@ -1594,10 +1596,70 @@ test_expect_success 'option override configured sort' '

1596 )

1597 '

1598

1599 +test_expect_success '--no-sort cancels config sort keys' '

1600 + test_config -C sort branch.sort "-refname" &&

1601 +

1602 + (

1603 + cd sort &&

1604 +

1605 + # objecttype is identical for all of them, so sort falls back on

1606 + # default (ascending refname)

1607 + git branch \

1608 + --no-sort \

1609 + --sort="objecttype" >actual &&

1610 + cat >expect <<-\EOF &&

1611 + a

1612 + * b

1613 + c

1614 + main

1615 + EOF

1616 + test_cmp expect actual

1617 + )

1618 +

1619 +'

1620 +

1621 +test_expect_success '--no-sort cancels command line sort keys' '

1622 + (

1623 + cd sort &&

1624 +

1625 + # objecttype is identical for all of them, so sort falls back on

1626 + # default (ascending refname)

1627 + git branch \

1628 + --sort="-refname" \

1629 + --no-sort \

1630 + --sort="objecttype" >actual &&

1631 + cat >expect <<-\EOF &&

1632 + a

1633 + * b

1634 + c

1635 + main

1636 + EOF

1637 + test_cmp expect actual

1638 + )

1639 +'

1640 +

1641 +test_expect_success '--no-sort without subsequent --sort prints expected branches' '

1642 + (

1643 + cd sort &&

1644 +

1645 + # Sort the results with `sort` for a consistent comparison

1646 + # against expected

1647 + git branch --no-sort | sort >actual &&

1648 + cat >expect <<-\EOF &&

1649 + a

1650 + c

1651 + main

1652 + * b

1653 + EOF

1654 + test_cmp expect actual

1655 + )

1656 +'

1657 +

1658 test_expect_success 'invalid sort parameter in configuration' '

1659 + test_config -C sort branch.sort "v:notvalid" &&

1660 +

1661 (

1662 cd sort &&

1600 - git config branch.sort "v:notvalid" &&

1663

1664 # this works in the "listing" mode, so bad sort key

1665 # is a dying offence.

t/t6300-for-each-ref.sh

+21

@@ -1224,6 +1224,27 @@ test_expect_success '--no-sort cancels the previous sort keys' '

1224 test_cmp expected actual

1225 '

1226

1227 +test_expect_success '--no-sort without subsequent --sort prints expected refs' '

1228 + cat >expected <<-\EOF &&

1229 + refs/tags/multi-ref1-100000-user1

1230 + refs/tags/multi-ref1-100000-user2

1231 + refs/tags/multi-ref1-200000-user1

1232 + refs/tags/multi-ref1-200000-user2

1233 + refs/tags/multi-ref2-100000-user1

1234 + refs/tags/multi-ref2-100000-user2

1235 + refs/tags/multi-ref2-200000-user1

1236 + refs/tags/multi-ref2-200000-user2

1237 + EOF

1238 +

1239 + # Sort the results with `sort` for a consistent comparison against

1240 + # expected

1241 + git for-each-ref \

1242 + --format="%(refname)" \

1243 + --no-sort \

1244 + "refs/tags/multi-*" | sort >actual &&

1245 + test_cmp expected actual

1246 +'

1247 +

1248 test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '

1249 test_when_finished "git checkout main" &&

1250 git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&

t/t7004-tag.sh

+45

@@ -1862,6 +1862,51 @@ test_expect_success 'option override configured sort' '

1862 test_cmp expect actual

1863 '

1864

1865 +test_expect_success '--no-sort cancels config sort keys' '

1866 + test_config tag.sort "-refname" &&

1867 +

1868 + # objecttype is identical for all of them, so sort falls back on

1869 + # default (ascending refname)

1870 + git tag -l \

1871 + --no-sort \

1872 + --sort="objecttype" \

1873 + "foo*" >actual &&

1874 + cat >expect <<-\EOF &&

1875 + foo1.10

1876 + foo1.3

1877 + foo1.6

1878 + EOF

1879 + test_cmp expect actual

1880 +'

1881 +

1882 +test_expect_success '--no-sort cancels command line sort keys' '

1883 + # objecttype is identical for all of them, so sort falls back on

1884 + # default (ascending refname)

1885 + git tag -l \

1886 + --sort="-refname" \

1887 + --no-sort \

1888 + --sort="objecttype" \

1889 + "foo*" >actual &&

1890 + cat >expect <<-\EOF &&

1891 + foo1.10

1892 + foo1.3

1893 + foo1.6

1894 + EOF

1895 + test_cmp expect actual

1896 +'

1897 +

1898 +test_expect_success '--no-sort without subsequent --sort prints expected tags' '

1899 + # Sort the results with `sort` for a consistent comparison against

1900 + # expected

1901 + git tag -l --no-sort "foo*" | sort >actual &&

1902 + cat >expect <<-\EOF &&

1903 + foo1.10

1904 + foo1.3

1905 + foo1.6

1906 + EOF

1907 + test_cmp expect actual

1908 +'

1909 +

1910 test_expect_success 'invalid sort parameter on command line' '

1911 test_must_fail git tag -l --sort=notvalid "foo*" >actual

1912 '