ref-filter: move ahead-behind bases into used_atom

verify_ref_format() parses a ref-filter format string and stores
recognized items in the static array "used_atom".  For
"ahead-behind:<committish>" it stores the committish part in a
string_list member "bases" of struct ref_format.

ref_sorting_options() also parses bare ref-filter format items and
stores stores recognized ones in "used_atom" as well.  The committish
parts go to a dummy struct ref_format in parse_sorting_atom(), though,
and are leaked and forgotten.

If verify_ref_format() is called before ref_sorting_options(), like in
git for-each-ref, then all works well if the sort key is included in the
format string.  If it isn't then sorting cannot work as the committishes
are missing.

If ref_sorting_options() is called first, like in git branch, then we
have the additional issue that if the sort key is included in the format
string then filter_ahead_behind() can't see its committish, will not
generate any results for it and thus it will be expanded to an empty
string.

Fix those issues by replacing the string_list with a field in used_atom
for storing the committish.  This way it can be shared for handling both
ref-filter format strings and sorting options in the same command.

Reported-by: Ross Goldberg <ross.goldberg@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/branch.c b/builtin/branch.c
index 6e7b0cfddbd6075d6692317d030f8d50472ad84e..fbb9536282dcaafc0f435d3886715f602dacef52 100644 (file)
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -473,7 +473,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
        if (verify_ref_format(format))
                die(_("unable to parse format string"));
 
-       filter_ahead_behind(the_repository, format, &array);
+       filter_ahead_behind(the_repository, &array);
        ref_array_sort(sorting, &array);
 
        if (column_active(colopts)) {

diff --git a/ref-filter.c b/ref-filter.c
index 23054694c2c96014ef411a225c6c941eb8d2d56c..250e412c47181ae706237e865f5eccd56d97d8b1 100644 (file)
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -235,6 +235,9 @@ static struct used_atom {
                        enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
                               S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL } option;
                } signature;
+               struct {
+                       struct commit *commit;
+               } base;
                struct strvec describe_args;
                struct refname_atom refname;
                char *head;
@@ -891,18 +894,15 @@ static int rest_atom_parser(struct ref_format *format UNUSED,
        return 0;
 }
 
-static int ahead_behind_atom_parser(struct ref_format *format,
-                                   struct used_atom *atom UNUSED,
+static int ahead_behind_atom_parser(struct ref_format *format UNUSED,
+                                   struct used_atom *atom,
                                    const char *arg, struct strbuf *err)
 {
-       struct string_list_item *item;
-
        if (!arg)
                return strbuf_addf_ret(err, -1, _("expected format: %%(ahead-behind:<committish>)"));
 
-       item = string_list_append(&format->bases, arg);
-       item->util = lookup_commit_reference_by_name(arg);
-       if (!item->util)
+       atom->u.base.commit = lookup_commit_reference_by_name(arg);
+       if (!atom->u.base.commit)
                die("failed to find '%s'", arg);
 
        return 0;
@@ -3084,22 +3084,30 @@ static void reach_filter(struct ref_array *array,
 }
 
 void filter_ahead_behind(struct repository *r,
-                        struct ref_format *format,
                         struct ref_array *array)
 {
        struct commit **commits;
-       size_t commits_nr = format->bases.nr + array->nr;
+       size_t bases_nr, commits_nr;
 
-       if (!format->bases.nr || !array->nr)
+       if (!array->nr)
                return;
 
-       ALLOC_ARRAY(commits, commits_nr);
-       for (size_t i = 0; i < format->bases.nr; i++)
-               commits[i] = format->bases.items[i].util;
+       for (size_t i = bases_nr = 0; i < used_atom_cnt; i++) {
+               if (used_atom[i].atom_type == ATOM_AHEADBEHIND)
+                       bases_nr++;
+       }
+       if (!bases_nr)
+               return;
 
-       ALLOC_ARRAY(array->counts, st_mult(format->bases.nr, array->nr));
+       ALLOC_ARRAY(commits, st_add(bases_nr, array->nr));
+       for (size_t i = 0, j = 0; i < used_atom_cnt; i++) {
+               if (used_atom[i].atom_type == ATOM_AHEADBEHIND)
+                       commits[j++] = used_atom[i].u.base.commit;
+       }
 
-       commits_nr = format->bases.nr;
+       ALLOC_ARRAY(array->counts, st_mult(bases_nr, array->nr));
+
+       commits_nr = bases_nr;
        array->counts_nr = 0;
        for (size_t i = 0; i < array->nr; i++) {
                const char *name = array->items[i]->refname;
@@ -3108,8 +3116,8 @@ void filter_ahead_behind(struct repository *r,
                if (!commits[commits_nr])
                        continue;
 
-               CALLOC_ARRAY(array->items[i]->counts, format->bases.nr);
-               for (size_t j = 0; j < format->bases.nr; j++) {
+               CALLOC_ARRAY(array->items[i]->counts, bases_nr);
+               for (size_t j = 0; j < bases_nr; j++) {
                        struct ahead_behind_count *count;
                        count = &array->counts[array->counts_nr++];
                        count->tip_index = commits_nr;
@@ -3277,9 +3285,12 @@ static inline int can_do_iterative_format(struct ref_filter *filter,
         * - filtering on reachability
         * - including ahead-behind information in the formatted output
         */
+       for (size_t i = 0; i < used_atom_cnt; i++) {
+               if (used_atom[i].atom_type == ATOM_AHEADBEHIND)
+                       return 0;
+       }
        return !(filter->reachable_from ||
                 filter->unreachable_from ||
-                format->bases.nr ||
                 format->is_base_tips.nr);
 }
 
@@ -3303,7 +3314,7 @@ void filter_and_format_refs(struct ref_filter *filter, unsigned int type,
        } else {
                struct ref_array array = { 0 };
                filter_refs(&array, filter, type);
-               filter_ahead_behind(the_repository, format, &array);
+               filter_ahead_behind(the_repository, &array);
                filter_is_base(the_repository, format, &array);
                ref_array_sort(sorting, &array);
                print_formatted_ref_array(&array, format);
@@ -3647,7 +3658,6 @@ void ref_format_init(struct ref_format *format)
 
 void ref_format_clear(struct ref_format *format)
 {
-       string_list_clear(&format->bases, 0);
        string_list_clear(&format->is_base_tips, 0);
        ref_format_init(format);
 }

diff --git a/ref-filter.h b/ref-filter.h
index 754038ab078669d82e4fff10514e46803cc265b7..5f3dd6c93189151ccfcbd9a01006eff5e4fa72aa 100644 (file)
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -99,9 +99,6 @@ struct ref_format {
        /* Internal state to ref-filter */
        int need_color_reset_at_eol;
 
-       /* List of bases for ahead-behind counts. */
-       struct string_list bases;
-
        /* List of bases for is-base indicators. */
        struct string_list is_base_tips;
 
@@ -117,7 +114,6 @@ struct ref_format {
 }
 #define REF_FORMAT_INIT {             \
        .use_color = -1,              \
-       .bases = STRING_LIST_INIT_DUP, \
        .is_base_tips = STRING_LIST_INIT_DUP, \
 }
 
@@ -205,7 +201,6 @@ struct ref_array_item *ref_array_push(struct ref_array *array,
  * If this is not called, then any ahead-behind atoms will be blank.
  */
 void filter_ahead_behind(struct repository *r,
-                        struct ref_format *format,
                         struct ref_array *array);
 
 /*

diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 500c9d0e727a2989ef0c37c71890d057f0d90819..a6bd88a58d0a8c0cfe90be384ac36a39ac372b82 100755 (executable)
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -368,6 +368,34 @@ test_expect_success 'git branch --format with ahead-behind' '
        test_cmp expect actual
 '
 
+test_expect_success 'git branch `--sort=[-]ahead-behind` option' '
+       cat >expect <<-\EOF &&
+       (HEAD detached from fromtag) 0 0
+       refs/heads/ambiguous 0 0
+       refs/heads/branch-two 0 0
+       refs/heads/branch-one 1 0
+       refs/heads/main 1 0
+       refs/heads/ref-to-branch 1 0
+       refs/heads/ref-to-remote 1 0
+       EOF
+       git branch --format="%(refname) %(ahead-behind:HEAD)" \
+               --sort=refname --sort=ahead-behind:HEAD >actual &&
+       test_cmp expect actual &&
+
+       cat >expect <<-\EOF &&
+       (HEAD detached from fromtag) 0 0
+       refs/heads/branch-one 1 0
+       refs/heads/main 1 0
+       refs/heads/ref-to-branch 1 0
+       refs/heads/ref-to-remote 1 0
+       refs/heads/ambiguous 0 0
+       refs/heads/branch-two 0 0
+       EOF
+       git branch --format="%(refname) %(ahead-behind:HEAD)" \
+               --sort=refname --sort=-ahead-behind:HEAD >actual &&
+       test_cmp expect actual
+'
+
 test_expect_success 'git branch with --format=%(rest) must fail' '
        test_must_fail git branch --format="%(rest)" >actual
 '