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

Kousik Sanagavarapu
2023-07-14T19:43:19Z

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