Subject: [PATCH v3 0/2] Add new "describe" atom

From: Kousik Sanagavarapu <[email-masked]>
Date: Wed, 19 Jul 2023 21:45:04 +0530

Hi,
Thanks Junio for the review on the previous version of this series. I
have made the changes according to the comments left.

PATCH 1/2 - Left unchanged expect for small changes in the commit
	    message for more clarity.

PATCH 2/2 - We now parse the arguments in a seperate function
	    `describe_atom_option_parser()` call this in
	    `describe_atom_parser()` instead to populate
	    `atom->u.describe_args`. This splitting of the function
	    helps err at the right places.

	    I've also squashed the third commit in the previous version
	    into this commit.

Kousik Sanagavarapu (2):
  ref-filter: add multiple-option parsing functions
  ref-filter: add new "describe" atom

 Documentation/git-for-each-ref.txt |  23 ++++
 ref-filter.c                       | 189 +++++++++++++++++++++++++++++
 t/t6300-for-each-ref.sh            | 114 +++++++++++++++++
 3 files changed, 326 insertions(+)

Range-diff against v2:

1:  50497067a3 ! 1:  08f3be1631 ref-filter: add multiple-option parsing functions
    @@ Commit message
                 match_placeholder_arg_value()
                 match_placeholder_bool_arg()
     
    -    were added in pretty (4f732e0fd7 (pretty: allow %(trailers) options
    -    with explicit value, 2019-01-29)) to parse multiple options in an
    +    were added in pretty 4f732e0fd7 (pretty: allow %(trailers) options
    +    with explicit value, 2019-01-29) to parse multiple options in an
         argument to --pretty. For example,
     
                 git log --pretty="%(trailers:key=Signed-Off-By,separator=%x2C )"
     
         will output all the trailers matching the key and seperates them by
    -    commas per commit.
    +    a comma followed by a space per commit.
     
         Add similar functions,
     
                 match_atom_arg_value()
                 match_atom_bool_arg()
     
    -    in ref-filter. A particular use of this can be seen in the subsequent
    -    commit where we parse the options given to a new atom "describe".
    +    in ref-filter.
    +
    +    There is no atom yet that can use these functions in ref-filter, but we
    +    are going to add a new %(describe) atom in a subsequent commit where we
    +    parse options like tags=<bool-value> or match=<pattern> given to it.
     
         Mentored-by: Christian Couder <[email-masked]>
         Mentored-by: Hariom Verma <[email-masked]>
2:  f6f882884c ! 2:  742a79113c ref-filter: add new "describe" atom
    @@ Documentation/git-for-each-ref.txt: ahead-behind:<committish>::
     
      ## ref-filter.c ##
     @@
    - #include "alloc.h"
    + #include "git-compat-util.h"
      #include "environment.h"
      #include "gettext.h"
     +#include "config.h"
    @@ ref-filter.c: enum atom_type {
        ATOM_BODY,
        ATOM_TRAILERS,
     @@ ref-filter.c: static struct used_atom {
    -           struct email_option {
    -                   enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
    -           } email_option;
    -+          struct {
    -+                  enum { D_BARE, D_TAGS, D_ABBREV,
    -+                         D_EXCLUDE, D_MATCH } option;
    -+                  const char **args;
    -+          } describe;
    +                   enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
    +                          S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL } option;
    +           } signature;
    ++          const char **describe_args;
                struct refname_atom refname;
                char *head;
        } u;
    @@ ref-filter.c: static int contents_atom_parser(struct ref_format *format, struct
        return 0;
      }
      
    ++static int describe_atom_option_parser(struct strvec *args, const char **arg,
    ++                                 struct strbuf *err)
    ++{
    ++  const char *argval;
    ++  size_t arglen = 0;
    ++  int optval = 0;
    ++
    ++  if (match_atom_bool_arg(*arg, "tags", arg, &optval)) {
    ++          if (!optval)
    ++                  strvec_push(args, "--no-tags");
    ++          else
    ++                  strvec_push(args, "--tags");
    ++          return 1;
    ++  }
    ++
    ++  if (match_atom_arg_value(*arg, "abbrev", arg, &argval, &arglen)) {
    ++          char *endptr;
    ++
    ++          if (!arglen)
    ++                  return strbuf_addf_ret(err, -1,
    ++                                         _("argument expected for %s"),
    ++                                         "describe:abbrev");
    ++          if (strtol(argval, &endptr, 10) < 0)
    ++                  return strbuf_addf_ret(err, -1,
    ++                                         _("positive value expected %s=%s"),
    ++                                         "describe:abbrev", argval);
    ++          if (endptr - argval != arglen)
    ++                  return strbuf_addf_ret(err, -1,
    ++                                         _("cannot fully parse %s=%s"),
    ++                                         "describe:abbrev", argval);
    ++
    ++          strvec_pushf(args, "--abbrev=%.*s", (int)arglen, argval);
    ++          return 1;
    ++  }
    ++
    ++  if (match_atom_arg_value(*arg, "match", arg, &argval, &arglen)) {
    ++          if (!arglen)
    ++                  return strbuf_addf_ret(err, -1,
    ++                                         _("value expected %s="),
    ++                                         "describe:match");
    ++
    ++          strvec_pushf(args, "--match=%.*s", (int)arglen, argval);
    ++          return 1;
    ++  }
    ++
    ++  if (match_atom_arg_value(*arg, "exclude", arg, &argval, &arglen)) {
    ++          if (!arglen)
    ++                  return strbuf_addf_ret(err, -1,
    ++                                         _("value expected %s="),
    ++                                         "describe:exclude");
    ++
    ++          strvec_pushf(args, "--exclude=%.*s", (int)arglen, argval);
    ++          return 1;
    ++  }
    ++
    ++  return 0;
    ++}
    ++
     +static int describe_atom_parser(struct ref_format *format UNUSED,
     +                          struct used_atom *atom,
     +                          const char *arg, struct strbuf *err)
     +{
    -+  const char *describe_opts[] = {
    -+          "",
    -+          "tags",
    -+          "abbrev",
    -+          "match",
    -+          "exclude",
    -+          NULL
    -+  };
    -+
     +  struct strvec args = STRVEC_INIT;
    ++
     +  for (;;) {
     +          int found = 0;
    -+          const char *argval;
    -+          size_t arglen = 0;
    -+          int optval = 0;
    -+          int opt;
    ++          const char *bad_arg = NULL;
     +
    -+          if (!arg)
    ++          if (!arg || !*arg)
     +                  break;
     +
    -+          for (opt = D_BARE; !found && describe_opts[opt]; opt++) {
    -+                  switch(opt) {
    -+                  case D_BARE:
    -+                          /*
    -+                           * Do nothing. This is the bare describe
    -+                           * atom and we already handle this above.
    -+                           */
    -+                          break;
    -+                  case D_TAGS:
    -+                          if (match_atom_bool_arg(arg, describe_opts[opt],
    -+                                                  &arg, &optval)) {
    -+                                  if (!optval)
    -+                                          strvec_pushf(&args, "--no-%s",
    -+                                                       describe_opts[opt]);
    -+                                  else
    -+                                          strvec_pushf(&args, "--%s",
    -+                                                       describe_opts[opt]);
    -+                                  found = 1;
    -+                          }
    -+                          break;
    -+                  case D_ABBREV:
    -+                          if (match_atom_arg_value(arg, describe_opts[opt],
    -+                                                   &arg, &argval, &arglen)) {
    -+                                  char *endptr;
    -+                                  int ret = 0;
    -+
    -+                                  if (!arglen)
    -+                                          ret = -1;
    -+                                  if (strtol(argval, &endptr, 10) < 0)
    -+                                          ret = -1;
    -+                                  if (endptr - argval != arglen)
    -+                                          ret = -1;
    -+
    -+                                  if (ret)
    -+                                          return strbuf_addf_ret(err, ret,
    -+                                                          _("positive value expected describe:abbrev=%s"), argval);
    -+                                  strvec_pushf(&args, "--%s=%.*s",
    -+                                               describe_opts[opt],
    -+                                               (int)arglen, argval);
    -+                                  found = 1;
    -+                          }
    -+                          break;
    -+                  case D_MATCH:
    -+                  case D_EXCLUDE:
    -+                          if (match_atom_arg_value(arg, describe_opts[opt],
    -+                                                   &arg, &argval, &arglen)) {
    -+                                  if (!arglen)
    -+                                          return strbuf_addf_ret(err, -1,
    -+                                                          _("value expected describe:%s="), describe_opts[opt]);
    -+                                  strvec_pushf(&args, "--%s=%.*s",
    -+                                               describe_opts[opt],
    -+                                               (int)arglen, argval);
    -+                                  found = 1;
    -+                          }
    -+                          break;
    -+                  }
    -+          }
    -+          if (!found)
    ++          bad_arg = arg;
    ++          found = describe_atom_option_parser(&args, &arg, err);
    ++          if (found < 0)
    ++                  return found;
    ++          if (!found) {
    ++                  if (bad_arg && *bad_arg)
    ++                          return err_bad_arg(err, "describe", bad_arg);
     +                  break;
    ++          }
     +  }
    -+  atom->u.describe.args = strvec_detach(&args);
    ++  atom->u.describe_args = strvec_detach(&args);
     +  return 0;
     +}
     +
    @@ ref-filter.c: static void append_lines(struct strbuf *out, const char *buf, unsi
     +
     +          if (!!deref != (*name == '*'))
     +                  continue;
    -+          if (deref)
    -+                  name++;
    -+
    -+          if (!skip_prefix(name, "describe", &name) ||
    -+              (*name && *name != ':'))
    -+                      continue;
    -+          if (!*name)
    -+                  name = NULL;
    -+          else
    -+                  name++;
     +
     +          cmd.git_cmd = 1;
     +          strvec_push(&cmd.args, "describe");
    -+          strvec_pushv(&cmd.args, atom->u.describe.args);
    ++          strvec_pushv(&cmd.args, atom->u.describe_args);
     +          strvec_push(&cmd.args, oid_to_hex(&commit->object.oid));
     +          if (pipe_command(&cmd, NULL, 0, &out, 0, &err, 0) < 0) {
     +                  error(_("failed to run 'describe'"));
    @@ ref-filter.c: static void grab_values(struct atom_value *val, int deref, struct
                break;
        case OBJ_COMMIT:
                grab_commit_values(val, deref, obj);
    -           grab_sub_body_contents(val, deref, data);
    +@@ ref-filter.c: static void grab_values(struct atom_value *val, int deref, struct object *obj, s
                grab_person("author", val, deref, buf);
                grab_person("committer", val, deref, buf);
    +           grab_signature(val, deref, obj);
     +          grab_describe_values(val, deref, obj);
                break;
        case OBJ_TREE:
    @@ t/t6300-for-each-ref.sh: test_expect_success 'color.ui=always does not override
        test_cmp expected.bare actual
      '
      
    -+test_expect_success 'describe atom vs git describe' '
    -+  test_when_finished "rm -rf describe-repo" &&
    -+
    ++test_expect_success 'setup for describe atom tests' '
     +  git init describe-repo &&
     +  (
     +          cd describe-repo &&
    @@ t/t6300-for-each-ref.sh: test_expect_success 'color.ui=always does not override
     +          git tag tagone &&
     +
     +          test_commit --no-tag two &&
    -+          git tag -a -m "tag two" tagtwo &&
    ++          git tag -a -m "tag two" tagtwo
    ++  )
    ++'
     +
    -+          git for-each-ref refs/tags/ --format="%(objectname)" >obj &&
    ++test_expect_success 'describe atom vs git describe' '
    ++  (
    ++          cd describe-repo &&
    ++
    ++          git for-each-ref --format="%(objectname)" \
    ++                  refs/tags/ >obj &&
     +          while read hash
     +          do
     +                  if desc=$(git describe $hash)
    @@ t/t6300-for-each-ref.sh: test_expect_success 'color.ui=always does not override
     +'
     +
     +test_expect_success 'describe:tags vs describe --tags' '
    -+  test_when_finished "git tag -d tagname" &&
    -+  git tag tagname &&
    -+  git describe --tags >expect &&
    -+  git for-each-ref --format="%(describe:tags)" refs/heads/ >actual &&
    -+  test_cmp expect actual
    ++  (
    ++          cd describe-repo &&
    ++          git describe --tags >expect &&
    ++          git for-each-ref --format="%(describe:tags)" \
    ++                          refs/heads/master >actual &&
    ++          test_cmp expect actual
    ++  )
     +'
     +
     +test_expect_success 'describe:abbrev=... vs describe --abbrev=...' '
    -+  test_when_finished "git tag -d tagname" &&
    -+
    -+  # Case 1: We have commits between HEAD and the most
    -+  #         recent tag reachable from it
    -+  test_commit --no-tag file &&
    -+  git describe --abbrev=14 >expect &&
    -+  git for-each-ref --format="%(describe:abbrev=14)" \
    -+          refs/heads/ >actual &&
    -+  test_cmp expect actual &&
    -+
    -+  # Make sure the hash used is atleast 14 digits long
    -+  sed -e "s/^.*-g\([0-9a-f]*\)$/\1/" <actual >hexpart &&
    -+  test 15 -le $(wc -c <hexpart) &&
    -+
    -+  # Case 2: We have a tag at HEAD, describe directly gives
    -+  #         the name of the tag
    -+  git tag -a -m tagged tagname &&
    -+  git describe --abbrev=14 >expect &&
    -+  git for-each-ref --format="%(describe:abbrev=14)" \
    -+          refs/heads/ >actual &&
    -+  test_cmp expect actual &&
    -+  test tagname = $(cat actual)
    ++  (
    ++          cd describe-repo &&
    ++
    ++          # Case 1: We have commits between HEAD and the most
    ++          #         recent tag reachable from it
    ++          test_commit --no-tag file &&
    ++          git describe --abbrev=14 >expect &&
    ++          git for-each-ref --format="%(describe:abbrev=14)" \
    ++                  refs/heads/master >actual &&
    ++          test_cmp expect actual &&
    ++
    ++          # Make sure the hash used is atleast 14 digits long
    ++          sed -e "s/^.*-g\([0-9a-f]*\)$/\1/" <actual >hexpart &&
    ++          test 15 -le $(wc -c <hexpart) &&
    ++
    ++          # Case 2: We have a tag at HEAD, describe directly gives
    ++          #         the name of the tag
    ++          git tag -a -m tagged tagname &&
    ++          git describe --abbrev=14 >expect &&
    ++          git for-each-ref --format="%(describe:abbrev=14)" \
    ++                  refs/heads/master >actual &&
    ++          test_cmp expect actual &&
    ++          test tagname = $(cat actual)
    ++  )
     +'
     +
     +test_expect_success 'describe:match=... vs describe --match ...' '
    -+  test_when_finished "git tag -d tag-match" &&
    -+  git tag -a -m "tag match" tag-match &&
    -+  git describe --match "*-match" >expect &&
    -+  git for-each-ref --format="%(describe:match="*-match")" \
    -+          refs/heads/ >actual &&
    -+  test_cmp expect actual
    ++  (
    ++          cd describe-repo &&
    ++          git tag -a -m "tag foo" tag-foo &&
    ++          git describe --match "*-foo" >expect &&
    ++          git for-each-ref --format="%(describe:match="*-foo")" \
    ++                  refs/heads/master >actual &&
    ++          test_cmp expect actual
    ++  )
     +'
     +
     +test_expect_success 'describe:exclude:... vs describe --exclude ...' '
    -+  test_when_finished "git tag -d tag-exclude" &&
    -+  git tag -a -m "tag exclude" tag-exclude &&
    -+  git describe --exclude "*-exclude" >expect &&
    -+  git for-each-ref --format="%(describe:exclude="*-exclude")" \
    -+          refs/heads/ >actual &&
    -+  test_cmp expect actual
    ++  (
    ++          cd describe-repo &&
    ++          git tag -a -m "tag bar" tag-bar &&
    ++          git describe --exclude "*-bar" >expect &&
    ++          git for-each-ref --format="%(describe:exclude="*-bar")" \
    ++                  refs/heads/master >actual &&
    ++          test_cmp expect actual
    ++  )
    ++'
    ++
    ++test_expect_success 'deref with describe atom' '
    ++  (
    ++          cd describe-repo &&
    ++          cat >expect <<-\EOF &&
    ++
    ++          tagname
    ++          tagname
    ++          tagname
    ++
    ++          tagtwo
    ++          EOF
    ++          git for-each-ref --format="%(*describe)" >actual &&
    ++          test_cmp expect actual
    ++  )
     +'
     +
      cat >expected <<\EOF
3:  a5122bf5e2 < -:  ---------- t6300: run describe atom tests on a different repo

-- 
2.41.0.378.g42703afc1f.dirty