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

Kousik Sanagavarapu
2023-07-19T16:24:47Z

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 <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
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