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@xxxxxxxxx>
Helped-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: René Scharfe <l.s.r@xxxxxx>
---
builtin/branch.c | 2 +-
ref-filter.c | 50 ++++++++++++++++++++++++----------------
ref-filter.h | 5 ----
t/t3203-branch-output.sh | 28 ++++++++++++++++++++++
4 files changed, 59 insertions(+), 26 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 6e7b0cfddb..fbb9536282 100644
--- 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 bf5534605e..c3957473e5 100644
--- 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 754038ab07..5f3dd6c931 100644
--- 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 500c9d0e72..a6bd88a58d 100755
--- 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
'
--
2.48.1
