ref-filter: sort numerically when ":size" is used

Atoms like "raw" and "contents" have a ":size" option which can be used to know the size of the data. Since these atoms have the cmp_type FIELD_STR, they are sorted alphabetically from 'a' to 'z' and '0' to '9'. Meaning, even when the ":size" option is used and what we ultimatlely have is numbers, we still sort alphabetically. For example, consider the the following case in a repo refname contents:size raw:size ======= ============= ======== refs/heads/branch1 1130 1210 refs/heads/master 300 410 refs/tags/v1.0 140 260 Sorting with "--format="%(refname) %(contents:size) --sort=contents:size" would give refs/heads/branch1 1130 refs/tags/v1.0.0 140 refs/heads/master 300 which is an alphabetic sort, while what one might really expect is refs/tags/v1.0.0 140 refs/heads/master 300 refs/heads/branch1 1130 which is a numeric sort (that is, a "$ sort -n file" as opposed to a "$ sort file", where "file" contains only the "contents:size" or "raw:size" info, each of which is on a newline). Same is the case with "--sort=raw:size". So, sort numerically whenever the sort is done with "contents:size" or "raw:size" and do it the normal alphabetic way when "contents" or "raw" are used with some other option (they are FIELD_STR anyways). Helped-by: Jeff King <peff@peff.net> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Kousik Sanagavarapu committed Sep 2, 2023 at 14:30 UTC 6d79cd8474b7bb4979f2a7544fd736bed190261a
2 files changed +26 -10
ref-filter.c
+13 -8
@@ -582,9 +582,10 @@ static int contents_atom_parser(struct ref_format *format, struct used_atom *ato
582 atom->u.contents.option = C_BARE;
583 else if (!strcmp(arg, "body"))
584 atom->u.contents.option = C_BODY;
585 - else if (!strcmp(arg, "size"))
585 + else if (!strcmp(arg, "size")) {
586 + atom->type = FIELD_ULONG;
587 atom->u.contents.option = C_LENGTH;
587 - else if (!strcmp(arg, "signature"))
588 + } else if (!strcmp(arg, "signature"))
589 atom->u.contents.option = C_SIG;
590 else if (!strcmp(arg, "subject"))
591 atom->u.contents.option = C_SUB;
@@ -690,9 +691,10 @@ static int raw_atom_parser(struct ref_format *format UNUSED,
691 {
692 if (!arg)
693 atom->u.raw_data.option = RAW_BARE;
693 - else if (!strcmp(arg, "size"))
694 + else if (!strcmp(arg, "size")) {
695 + atom->type = FIELD_ULONG;
696 atom->u.raw_data.option = RAW_LENGTH;
695 - else
697 + } else
698 return err_bad_arg(err, "raw", arg);
699 return 0;
700 }
@@ -1857,7 +1859,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
1859 v->s = xmemdupz(buf, buf_size);
1860 v->s_size = buf_size;
1861 } else if (atom->u.raw_data.option == RAW_LENGTH) {
1860 - v->s = xstrfmt("%"PRIuMAX, (uintmax_t)buf_size);
1862 + v->value = buf_size;
1863 + v->s = xstrfmt("%"PRIuMAX, v->value);
1864 }
1865 continue;
1866 }
@@ -1883,9 +1886,10 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
1886 v->s = strbuf_detach(&sb, NULL);
1887 } else if (atom->u.contents.option == C_BODY_DEP)
1888 v->s = xmemdupz(bodypos, bodylen);
1886 - else if (atom->u.contents.option == C_LENGTH)
1887 - v->s = xstrfmt("%"PRIuMAX, (uintmax_t)strlen(subpos));
1888 - else if (atom->u.contents.option == C_BODY)
1889 + else if (atom->u.contents.option == C_LENGTH) {
1890 + v->value = strlen(subpos);
1891 + v->s = xstrfmt("%"PRIuMAX, v->value);
1892 + } else if (atom->u.contents.option == C_BODY)
1893 v->s = xmemdupz(bodypos, nonsiglen);
1894 else if (atom->u.contents.option == C_SIG)
1895 v->s = xmemdupz(sigpos, siglen);
@@ -2265,6 +2269,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
2269
2270 v->s_size = ATOM_SIZE_UNSPECIFIED;
2271 v->handler = append_atom;
2272 + v->value = 0;
2273 v->atom = atom;
2274
2275 if (*name == '*') {
t/t6300-for-each-ref.sh
+13 -2
@@ -1017,16 +1017,16 @@ test_expect_success 'Verify sorts with raw' '
1017 test_expect_success 'Verify sorts with raw:size' '
1018 cat >expected <<-EOF &&
1019 refs/myblobs/blob8
1020 - refs/myblobs/first
1020 refs/myblobs/blob7
1022 - refs/heads/main
1021 refs/myblobs/blob4
1022 refs/myblobs/blob1
1023 refs/myblobs/blob2
1024 refs/myblobs/blob3
1025 refs/myblobs/blob5
1026 refs/myblobs/blob6
1027 + refs/myblobs/first
1028 refs/mytrees/first
1029 + refs/heads/main
1030 EOF
1031 git for-each-ref --format="%(refname)" --sort=raw:size \
1032 refs/heads/main refs/myblobs/ refs/mytrees/first >actual &&
@@ -1138,6 +1138,17 @@ test_expect_success 'for-each-ref --format compare with cat-file --batch' '
1138 test_cmp expected actual
1139 '
1140
1141 +test_expect_success 'verify sorts with contents:size' '
1142 + cat >expect <<-\EOF &&
1143 + refs/heads/main
1144 + refs/heads/newtag
1145 + refs/heads/ambiguous
1146 + EOF
1147 + git for-each-ref --format="%(refname)" \
1148 + --sort=contents:size refs/heads/ >actual &&
1149 + test_cmp expect actual
1150 +'
1151 +
1152 test_expect_success 'set up multiple-sort tags' '
1153 for when in 100000 200000
1154 do