Subject: [PATCH v2 0/3] Add new "describe" atom

From: Kousik Sanagavarapu <[email-masked]>
Date: Sat, 15 Jul 2023 00:50:25 +0530

Hi,
This patch series addresses the previous comments and so now we can do
for example

	git for-each-ref --format="%(describe:tags=yes,abbrev=14)"

PATCH 1/3 - This is a new commit which introduces two new functions for
	    handling multiple options in ref-filter.

	    There are two ways to do this
	    - We change the functions in pretty so that they can be used
	      generally and not only for placeholders.
	    - We introduce corresponding functions in ref-filter for
	      handling atoms.

	    This patch follows the second approach but the first
	    approach is also good because we don't duplicate the code.
	    Or maybe there is a much better approach that I don't see.

PATCH 2/3 - Changes are made so that we can handle multiple options and
	    also the related docs are a nested description list.

PATCH 3/3 - This commit is left unchanged.

Kousik Sanagavarapu (3):
  ref-filter: add multiple-option parsing functions
  ref-filter: add new "describe" atom
  t6300: run describe atom tests on a different repo

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

Range-diff against v1:

-:  ---------- > 1:  50497067a3 ref-filter: add multiple-option parsing
functions
1:  9e3e652659 ! 2:  f6f882884c ref-filter: add new "describe" atom
    @@ Documentation/git-for-each-ref.txt: ahead-behind:<committish>::
        commits ahead and behind, respectively, when comparing the
output
        ref to the `<committish>` specified in the format.
      
    -+describe[:options]:: human-readable name, like
    ++describe[:options]:: Human-readable name, like
     +               link-git:git-describe[1]; empty string for
     +               undescribable commits. The `describe` string may be
     +               followed by a colon and zero or more
comma-separated
     +               options. Descriptions can be inconsistent when tags
     +               are added or removed at the same time.
     ++
    -+** tags=<bool-value>: Instead of only considering annotated tags,
consider
    -+                lightweight tags as well.
    -+** abbrev=<number>: Instead of using the default number of
hexadecimal digits
    -+              (which will vary according to the number of objects
in the
    -+              repository with a default of 7) of the abbreviated
    -+              object name, use <number> digits, or as many digits
as
    -+              needed to form a unique object name.
    -+** match=<pattern>: Only consider tags matching the given
`glob(7)` pattern,
    -+              excluding the "refs/tags/" prefix.
    -+** exclude=<pattern>: Do not consider tags matching the given
`glob(7)`
    -+                pattern,excluding the "refs/tags/" prefix.
    ++--
    ++tags=<bool-value>;; Instead of only considering annotated tags,
consider
    ++              lightweight tags as well; see the corresponding
option
    ++              in linkgit:git-describe[1] for details.
    ++abbrev=<number>;; Use at least <number> hexadecimal digits; see
    ++            the corresponding option in linkgit:git-describe[1]
    ++            for details.
    ++match=<pattern>;; Only consider tags matching the given `glob(7)`
pattern,
    ++            excluding the "refs/tags/" prefix; see the
corresponding
    ++            option in linkgit:git-describe[1] for details.
    ++exclude=<pattern>;; Do not consider tags matching the given
`glob(7)`
    ++              pattern, excluding the "refs/tags/" prefix; see the
    ++              corresponding option in linkgit:git-describe[1] for
    ++              details.
    ++--
     +
      In addition to the above, for commit and tag objects, the header
      field names (`tree`, `parent`, `object`, `type`, and `tag`) can
    @@ Documentation/git-for-each-ref.txt: ahead-behind:<committish>::
     
      ## ref-filter.c ##
     @@
    + #include "alloc.h"
    + #include "environment.h"
    + #include "gettext.h"
    ++#include "config.h"
      #include "gpg-interface.h"
      #include "hex.h"
      #include "parse-options.h"
    @@ ref-filter.c: static struct used_atom {
                        enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
                } email_option;
     +          struct {
    -+                  enum { D_BARE, D_TAGS, D_ABBREV, D_EXCLUDE,
    -+                         D_MATCH } option;
    -+                  unsigned int tagbool;
    -+                  unsigned int length;
    -+                  char *pattern;
    ++                  enum { D_BARE, D_TAGS, D_ABBREV,
    ++                         D_EXCLUDE, D_MATCH } option;
    ++                  const char **args;
     +          } describe;
                struct refname_atom refname;
                char *head;
    @@ ref-filter.c: static int contents_atom_parser(struct ref_format
*format, struct
        return 0;
      }
      
    -+static int parse_describe_option(const char *arg)
    -+{
    -+  if (!arg)
    -+          return D_BARE;
    -+  else if (starts_with(arg, "tags"))
    -+          return D_TAGS;
    -+  else if (starts_with(arg, "abbrev"))
    -+          return D_ABBREV;
    -+  else if(starts_with(arg, "exclude"))
    -+          return D_EXCLUDE;
    -+  else if (starts_with(arg, "match"))
    -+          return D_MATCH;
    -+  return -1;
    -+}
    -+
     +static int describe_atom_parser(struct ref_format *format UNUSED,
     +                          struct used_atom *atom,
     +                          const char *arg, struct strbuf *err)
     +{
    -+  int opt = parse_describe_option(arg);
    ++  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;
    ++
    ++          if (!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;
     +
    -+  switch (opt) {
    -+  case D_BARE:
    -+          break;
    -+  case D_TAGS:
    -+          /*
    -+           * It is also possible to just use describe:tags, which
    -+           * is just treated as describe:tags=1
    -+           */
    -+          if (skip_prefix(arg, "tags=", &arg)) {
    -+                  if (strtoul_ui(arg, 10,
&atom->u.describe.tagbool))
    -+                          return strbuf_addf_ret(err, -1,
_("boolean value "
    -+                                          "expected
describe:tags=%s"), arg);
    ++                                  if (!arglen)
    ++                                          ret = -1;
    ++                                  if (strtol(argval, &endptr, 10)
< 0)
    ++                                          ret = -1;
    ++                                  if (endptr - argval != arglen)
    ++                                          ret = -1;
     +
    -+          } else {
    -+                  atom->u.describe.tagbool = 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;
    ++                  }
     +          }
    -+          break;
    -+  case D_ABBREV:
    -+          skip_prefix(arg, "abbrev=", &arg);
    -+          if (strtoul_ui(arg, 10, &atom->u.describe.length))
    -+                  return strbuf_addf_ret(err, -1, _("positive
value "
    -+                                         "expected
describe:abbrev=%s"), arg);
    -+          break;
    -+  case D_EXCLUDE:
    -+          skip_prefix(arg, "exclude=", &arg);
    -+          atom->u.describe.pattern = xstrdup(arg);
    -+          break;
    -+  case D_MATCH:
    -+          skip_prefix(arg, "match=", &arg);
    -+          atom->u.describe.pattern = xstrdup(arg);
    -+          break;
    -+  default:
    -+          return err_bad_arg(err, "describe", arg);
    -+          break;
    ++          if (!found)
    ++                  break;
     +  }
    -+  atom->u.describe.option = opt;
    ++  atom->u.describe.args = strvec_detach(&args);
     +  return 0;
     +}
     +
    @@ ref-filter.c: static void append_lines(struct strbuf *out, const
char *buf, unsi
     +
     +  for (i = 0; i < used_atom_cnt; i++) {
     +          struct used_atom *atom = &used_atom[i];
    ++          enum atom_type type = atom->atom_type;
     +          const char *name = atom->name;
     +          struct atom_value *v = &val[i];
    -+          int opt;
     +
     +          struct child_process cmd = CHILD_PROCESS_INIT;
     +          struct strbuf out = STRBUF_INIT;
     +          struct strbuf err = STRBUF_INIT;
     +
    ++          if (type != ATOM_DESCRIBE)
    ++                  continue;
    ++
     +          if (!!deref != (*name == '*'))
     +                  continue;
     +          if (deref)
    @@ ref-filter.c: static void append_lines(struct strbuf *out, const
char *buf, unsi
     +          else
     +                  name++;
     +
    -+          opt = parse_describe_option(name);
    -+          if (opt < 0)
    -+                  continue;
    -+
     +          cmd.git_cmd = 1;
     +          strvec_push(&cmd.args, "describe");
    -+
    -+          switch(opt) {
-:  ---------- > 1:  50497067a3 ref-filter: add multiple-option parsing
functions
1:  9e3e652659 ! 2:  f6f882884c ref-filter: add new "describe" atom
    @@ Documentation/git-for-each-ref.txt: ahead-behind:<committish>::
        commits ahead and behind, respectively, when comparing the
output
        ref to the `<committish>` specified in the format.
      
    -+describe[:options]:: human-readable name, like
    ++describe[:options]:: Human-readable name, like
     +               link-git:git-describe[1]; empty string for
     +               undescribable commits. The `describe` string may be
     +               followed by a colon and zero or more
comma-separated
     +               options. Descriptions can be inconsistent when tags
     +               are added or removed at the same time.
     ++
    -+** tags=<bool-value>: Instead of only considering annotated tags,
consider
    -+                lightweight tags as well.
    -+** abbrev=<number>: Instead of using the default number of
hexadecimal digits
    -+              (which will vary according to the number of objects
in the
    -+              repository with a default of 7) of the abbreviated
    -+              object name, use <number> digits, or as many digits
as
    -+              needed to form a unique object name.
    -+** match=<pattern>: Only consider tags matching the given
`glob(7)` pattern,
    -+              excluding the "refs/tags/" prefix.
    -+** exclude=<pattern>: Do not consider tags matching the given
`glob(7)`
    -+                pattern,excluding the "refs/tags/" prefix.
    ++--
    ++tags=<bool-value>;; Instead of only considering annotated tags,
consider
    ++              lightweight tags as well; see the corresponding
option
    ++              in linkgit:git-describe[1] for details.
    ++abbrev=<number>;; Use at least <number> hexadecimal digits; see
    ++            the corresponding option in linkgit:git-describe[1]
    ++            for details.
    ++match=<pattern>;; Only consider tags matching the given `glob(7)`
pattern,
    ++            excluding the "refs/tags/" prefix; see the
corresponding
    ++            option in linkgit:git-describe[1] for details.
    ++exclude=<pattern>;; Do not consider tags matching the given
`glob(7)`
    ++              pattern, excluding the "refs/tags/" prefix; see the
    ++              corresponding option in linkgit:git-describe[1] for
    ++              details.
    ++--
     +
      In addition to the above, for commit and tag objects, the header
      field names (`tree`, `parent`, `object`, `type`, and `tag`) can
    @@ Documentation/git-for-each-ref.txt: ahead-behind:<committish>::
     
      ## ref-filter.c ##
     @@
    + #include "alloc.h"
    + #include "environment.h"
    + #include "gettext.h"
    ++#include "config.h"
      #include "gpg-interface.h"
      #include "hex.h"
      #include "parse-options.h"
    @@ ref-filter.c: static struct used_atom {
                        enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
                } email_option;
     +          struct {
    -+                  enum { D_BARE, D_TAGS, D_ABBREV, D_EXCLUDE,
    -+                         D_MATCH } option;
    -+                  unsigned int tagbool;
    -+                  unsigned int length;
    -+                  char *pattern;
    ++                  enum { D_BARE, D_TAGS, D_ABBREV,
    ++                         D_EXCLUDE, D_MATCH } option;
    ++                  const char **args;
     +          } describe;
                struct refname_atom refname;
                char *head;
    @@ ref-filter.c: static int contents_atom_parser(struct ref_format
*format, struct
        return 0;
      }
      
    -+static int parse_describe_option(const char *arg)
    -+{
    -+  if (!arg)
    -+          return D_BARE;
    -+  else if (starts_with(arg, "tags"))
    -+          return D_TAGS;
    -+  else if (starts_with(arg, "abbrev"))
    -+          return D_ABBREV;
    -+  else if(starts_with(arg, "exclude"))
    -+          return D_EXCLUDE;
    -+  else if (starts_with(arg, "match"))
    -+          return D_MATCH;
    -+  return -1;
    -+}
    -+
     +static int describe_atom_parser(struct ref_format *format UNUSED,
     +                          struct used_atom *atom,
     +                          const char *arg, struct strbuf *err)
     +{
    -+  int opt = parse_describe_option(arg);
    ++  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;
    ++
    ++          if (!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;
     +
    -+  switch (opt) {
    -+  case D_BARE:
    -+          break;
    -+  case D_TAGS:
    -+          /*
    -+           * It is also possible to just use describe:tags, which
    -+           * is just treated as describe:tags=1
    -+           */
    -+          if (skip_prefix(arg, "tags=", &arg)) {
    -+                  if (strtoul_ui(arg, 10,
&atom->u.describe.tagbool))
    -+                          return strbuf_addf_ret(err, -1,
_("boolean value "
    -+                                          "expected
describe:tags=%s"), arg);
    ++                                  if (!arglen)
    ++                                          ret = -1;
    ++                                  if (strtol(argval, &endptr, 10)
< 0)
    ++                                          ret = -1;
    ++                                  if (endptr - argval != arglen)
    ++                                          ret = -1;
     +
    -+          } else {
    -+                  atom->u.describe.tagbool = 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,
...skipping...
    -+          case D_BARE:
    -+                  break;
    -+          case D_TAGS:
    -+                  if (atom->u.describe.tagbool)
    -+                          strvec_push(&cmd.args, "--tags");
    -+                  else
    -+                          strvec_push(&cmd.args, "--no-tags");
    -+                  break;
    -+          case D_ABBREV:
    -+                  strvec_pushf(&cmd.args, "--abbrev=%d",
    -+                               atom->u.describe.length);
    -+                  break;
    -+          case D_EXCLUDE:
    -+                  strvec_pushf(&cmd.args, "--exclude=%s",
    -+                               atom->u.describe.pattern);
    -+                  break;
    -+          case D_MATCH:
    -+                  strvec_pushf(&cmd.args, "--match=%s",
    -+                               atom->u.describe.pattern);
    -+                  break;
    -+          }
    -+
    ++          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'"));
2:  43cd3eef3c = 3:  a5122bf5e2 t6300: run describe atom tests on a
different repo
 fivlite  BR describe4  ~ | Documents | git  git checkout master
Switched to branch 'master'
Your branch is up to date with 'upstream/master'.
 fivlite  BR master  ~ | Documents | git  git range-diff
9748a6820043..describe4 master..describe5
-:  ---------- > 1:  50497067a3 ref-filter: add multiple-option parsing
functions
1:  9e3e652659 ! 2:  f6f882884c ref-filter: add new "describe" atom
    @@ Documentation/git-for-each-ref.txt: ahead-behind:<committish>::
        commits ahead and behind, respectively, when comparing the
output
        ref to the `<committish>` specified in the format.
      
    -+describe[:options]:: human-readable name, like
    ++describe[:options]:: Human-readable name, like
     +               link-git:git-describe[1]; empty string for
     +               undescribable commits. The `describe` string may be
     +               followed by a colon and zero or more
comma-separated
     +               options. Descriptions can be inconsistent when tags
     +               are added or removed at the same time.
     ++
    -+** tags=<bool-value>: Instead of only considering annotated tags,
consider
    -+                lightweight tags as well.
    -+** abbrev=<number>: Instead of using the default number of
hexadecimal digits
    -+              (which will vary according to the number of objects
in the
    -+              repository with a default of 7) of the abbreviated
    -+              object name, use <number> digits, or as many digits
as
    -+              needed to form a unique object name.
    -+** match=<pattern>: Only consider tags matching the given
`glob(7)` pattern,
    -+              excluding the "refs/tags/" prefix.
    -+** exclude=<pattern>: Do not consider tags matching the given
`glob(7)`
    -+                pattern,excluding the "refs/tags/" prefix.
    ++--
    ++tags=<bool-value>;; Instead of only considering annotated tags,
consider
    ++              lightweight tags as well; see the corresponding
option
    ++              in linkgit:git-describe[1] for details.
    ++abbrev=<number>;; Use at least <number> hexadecimal digits; see
    ++            the corresponding option in linkgit:git-describe[1]
    ++            for details.
    ++match=<pattern>;; Only consider tags matching the given `glob(7)`
pattern,
    ++            excluding the "refs/tags/" prefix; see the
corresponding
...skipping...
    -+          case D_BARE:
    -+                  break;
    -+          case D_TAGS:
    -+                  if (atom->u.describe.tagbool)
    -+                          strvec_push(&cmd.args, "--tags");
    -+                  else
    -+                          strvec_push(&cmd.args, "--no-tags");
    -+                  break;
    -+          case D_ABBREV:
    -+                  strvec_pushf(&cmd.args, "--abbrev=%d",
    -+                               atom->u.describe.length);
    -+                  break;
    -+          case D_EXCLUDE:
    -+                  strvec_pushf(&cmd.args, "--exclude=%s",
    -+                               atom->u.describe.pattern);
    -+                  break;
    -+          case D_MATCH:
    -+                  strvec_pushf(&cmd.args, "--match=%s",
    -+                               atom->u.describe.pattern);
    -+                  break;
    -+          }
    -+
    ++          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'"));
2:  43cd3eef3c = 3:  a5122bf5e2 t6300: run describe atom tests on a
different repo

-- 
2.41.0.321.g26b82700c0.dirty