Re: [PATCH v2] ref-filter: sort numerically when ":size" is used

From: Kousik Sanagavarapu <five231003@gmail.com>
Date: Sep 2, 2023, 09:11 UTC
Message-ID: <ZPL8SQTxbqZ3LTCP@five231003>
In-Reply-To: <20230902090155.8978-1-five231003@gmail.com>

On Sat, Sep 02, 2023 at 02:30:39PM +0530, Kousik Sanagavarapu wrote:

Show 5 quoted lines

> 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.

[...]

Show 7 quoted lines

> 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>
> ---

Oops, forgot the range-diff.

Range-diff against v1:

1:  9b561e429b ! 1:  194dcb0b0d ref-filter: sort numerically when ":size" is used
    @@ Commit message
         "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>

      ## ref-filter.c ##
    -@@ ref-filter.c: struct atom_value {
    -   ssize_t s_size;
    -   int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state,
    -                  struct strbuf *err);
    --  uintmax_t value; /* used for sorting when not FIELD_STR */
    -+
    -+  /*
    -+   * Used for sorting when not FIELD_STR or when FIELD_STR but the
    -+   * sort should be numeric and not alphabetic.
    -+   */
    -+  uintmax_t value;
    -+
    -   struct used_atom *atom;
    - };
    -
    +@@ ref-filter.c: static int contents_atom_parser(struct ref_format *format, struct used_atom *ato
    +           atom->u.contents.option = C_BARE;
    +   else if (!strcmp(arg, "body"))
    +           atom->u.contents.option = C_BODY;
    +-  else if (!strcmp(arg, "size"))
    ++  else if (!strcmp(arg, "size")) {
    ++          atom->type = FIELD_ULONG;
    +           atom->u.contents.option = C_LENGTH;
    +-  else if (!strcmp(arg, "signature"))
    ++  } else if (!strcmp(arg, "signature"))
    +           atom->u.contents.option = C_SIG;
    +   else if (!strcmp(arg, "subject"))
    +           atom->u.contents.option = C_SUB;
    +@@ ref-filter.c: static int raw_atom_parser(struct ref_format *format UNUSED,
    + {
    +   if (!arg)
    +           atom->u.raw_data.option = RAW_BARE;
    +-  else if (!strcmp(arg, "size"))
    ++  else if (!strcmp(arg, "size")) {
    ++          atom->type = FIELD_ULONG;
    +           atom->u.raw_data.option = RAW_LENGTH;
    +-  else
    ++  } else
    +           return err_bad_arg(err, "raw", arg);
    +   return 0;
    + }
     @@ ref-filter.c: static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
                                v->s = xmemdupz(buf, buf_size);
                                v->s_size = buf_size;
                                v->s = xmemdupz(buf, buf_size);
                                v->s_size = buf_size;
                                v->s_size = buf_size;
                        } else if (atom->u.raw_data.option == RAW_LENGTH) {
     -                          v->s = xstrfmt("%"PRIuMAX, (uintmax_t)buf_size);
    -+                          v->value = (uintmax_t)buf_size;
    ++                          v->value = buf_size;
     +                          v->s = xstrfmt("%"PRIuMAX, v->value);
                        }
                        continue;
    @@ ref-filter.c: static void grab_sub_body_contents(struct atom_value *val, int der
                        v->s = xmemdupz(bodypos, bodylen);
     -          else if (atom->u.contents.option == C_LENGTH)
     -                  v->s = xstrfmt("%"PRIuMAX, (uintmax_t)strlen(subpos));
    +-          else if (atom->u.contents.option == C_BODY)
     +          else if (atom->u.contents.option == C_LENGTH) {
    -+                  v->value = (uintmax_t)strlen(subpos);
    ++                  v->value = strlen(subpos);
     +                  v->s = xstrfmt("%"PRIuMAX, v->value);
    -+          }
    -           else if (atom->u.contents.option == C_BODY)
    ++          } else if (atom->u.contents.option == C_BODY)
                        v->s = xmemdupz(bodypos, nonsiglen);
                else if (atom->u.contents.option == C_SIG)
    +                   v->s = xmemdupz(sigpos, siglen);
     @@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbuf *err)

                v->s_size = ATOM_SIZE_UNSPECIFIED;
    @@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbu
                v->atom = atom;

                if (*name == '*') {
    -@@ ref-filter.c: static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
    -           cmp_detached_head = 1;
    -   } else if (s->sort_flags & REF_SORTING_VERSION) {
    -           cmp = versioncmp(va->s, vb->s);
    --  } else if (cmp_type == FIELD_STR) {
    -+  } else if (cmp_type == FIELD_STR && !va->value && !vb->value) {
    -           if (va->s_size < 0 && vb->s_size < 0) {
    -                   int (*cmp_fn)(const char *, const char *);
    -                   cmp_fn = s->sort_flags & REF_SORTING_ICASE

      ## t/t6300-for-each-ref.sh ##
     @@ t/t6300-for-each-ref.sh: test_expect_success 'Verify sorts with raw' '