[PATCH] ref-filter: share bases and is_base_tips between formatting and sorting
verify_ref_format() parses a ref-filter format string and stores recognized items in the static array "used_atom". For "ahead-behind:<committish>" and "is-base:<committish>" it stores the committish part in string_lists that are part of struct ref_format.
ref_sorting_options() also parses bare ref-filter format items and also stores recognized ones in "used_atom". 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() and filter_is_base() can't see their committishes, will not generate any results for them and thus they will for expanded to empty strings.
Fix those issues by making the string_lists static, like their sibling "used_atom". This way they can all be shared for handling both ref-filter format strings and sorting options in the same command. And since struct ref_format no longer contains any allocated members, remove the now unnecessary ref_format_init() and ref_format_clear().
Reported-by: Ross Goldberg <ross.goldberg@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
builtin/branch.c | 3 +-
builtin/for-each-ref.c | 1 -
builtin/tag.c | 1 -
builtin/verify-tag.c | 1 -
ref-filter.c | 70 ++++++++++++++++++----------------------
ref-filter.h | 13 --------
t/t3203-branch-output.sh | 28 ++++++++++++++++
t/t6600-test-reach.sh | 29 +++++++++++++++++
8 files changed, 89 insertions(+), 57 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 6e7b0cfddb..9a29de5bf1 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)) {
@@ -884,7 +884,6 @@ int cmd_branch(int argc,
string_list_clear(&output, 0);
ref_sorting_release(sorting);
ref_filter_clear(&filter);
- ref_format_clear(&format);
ret = 0;
goto out;
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 715745a262..8085ebd8fe 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -108,7 +108,6 @@ int cmd_for_each_ref(int argc,
filter_and_format_refs(&filter, flags, sorting, &format);
ref_filter_clear(&filter);
- ref_format_clear(&format);
ref_sorting_release(sorting);
strvec_clear(&vec);
return 0;
diff --git a/builtin/tag.c b/builtin/tag.c
index c4bd145831..e8a344b926 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -698,7 +698,6 @@ int cmd_tag(int argc,
cleanup:
ref_sorting_release(sorting);
ref_filter_clear(&filter);
- ref_format_clear(&format);
strbuf_release(&buf);
strbuf_release(&ref);
strbuf_release(&reflog_msg);
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index a7f20618ff..f6b97048a5 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -69,6 +69,5 @@ int cmd_verify_tag(int argc,
if (format.format)
pretty_print_ref(name, &oid, &format);
}
- ref_format_clear(&format);
return had_error;
}
diff --git a/ref-filter.c b/ref-filter.c
index 23054694c2..aef142e105 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -242,6 +242,12 @@ static struct used_atom {
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
+/* List of bases for ahead-behind counts. */
+static struct string_list bases = STRING_LIST_INIT_DUP;
+
+/* List of bases for is-base indicators. */
+static struct string_list is_base_tips = STRING_LIST_INIT_DUP;
+
/*
* Expand string, append it to strbuf *sb, then return error code ret.
* Allow to save few lines of code.
@@ -891,7 +897,7 @@ static int rest_atom_parser(struct ref_format *format UNUSED,
return 0;
}
-static int ahead_behind_atom_parser(struct ref_format *format,
+static int ahead_behind_atom_parser(struct ref_format *format UNUSED,
struct used_atom *atom UNUSED,
const char *arg, struct strbuf *err)
{
@@ -900,7 +906,7 @@ static int ahead_behind_atom_parser(struct ref_format *format,
if (!arg)
return strbuf_addf_ret(err, -1, _("expected format: %%(ahead-behind:<committish>)"));
- item = string_list_append(&format->bases, arg);
+ item = string_list_append(&bases, arg);
item->util = lookup_commit_reference_by_name(arg);
if (!item->util)
die("failed to find '%s'", arg);
@@ -908,7 +914,7 @@ static int ahead_behind_atom_parser(struct ref_format *format,
return 0;
}
-static int is_base_atom_parser(struct ref_format *format,
+static int is_base_atom_parser(struct ref_format *format UNUSED,
struct used_atom *atom UNUSED,
const char *arg, struct strbuf *err)
{
@@ -917,7 +923,7 @@ static int is_base_atom_parser(struct ref_format *format,
if (!arg)
return strbuf_addf_ret(err, -1, _("expected format: %%(is-base:<committish>)"));
- item = string_list_append(&format->is_base_tips, arg);
+ item = string_list_append(&is_base_tips, arg);
item->util = lookup_commit_reference_by_name(arg);
if (!item->util)
die("failed to find '%s'", arg);
@@ -3024,6 +3030,8 @@ void ref_array_clear(struct ref_array *array)
}
FREE_AND_NULL(used_atom);
used_atom_cnt = 0;
+ string_list_clear(&bases, 0);
+ string_list_clear(&is_base_tips, 0);
if (ref_to_worktree_map.worktrees) {
hashmap_clear_and_free(&(ref_to_worktree_map.map),
@@ -3084,22 +3092,21 @@ 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 commits_nr = bases.nr + array->nr;
- if (!format->bases.nr || !array->nr)
+ if (!bases.nr || !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 = 0; i < bases.nr; i++)
+ commits[i] = bases.items[i].util;
- ALLOC_ARRAY(array->counts, st_mult(format->bases.nr, array->nr));
+ ALLOC_ARRAY(array->counts, st_mult(bases.nr, array->nr));
- commits_nr = format->bases.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 +3115,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;
@@ -3125,14 +3132,13 @@ void filter_ahead_behind(struct repository *r,
}
void filter_is_base(struct repository *r,
- struct ref_format *format,
struct ref_array *array)
{
struct commit **bases;
size_t bases_nr = 0;
struct ref_array_item **back_index;
- if (!format->is_base_tips.nr || !array->nr)
+ if (!is_base_tips.nr || !array->nr)
return;
CALLOC_ARRAY(back_index, array->nr);
@@ -3142,7 +3148,7 @@ void filter_is_base(struct repository *r,
const char *name = array->items[i]->refname;
struct commit *c = lookup_commit_reference_by_name_gently(name, 1);
- CALLOC_ARRAY(array->items[i]->is_base, format->is_base_tips.nr);
+ CALLOC_ARRAY(array->items[i]->is_base, is_base_tips.nr);
if (!c)
continue;
@@ -3152,15 +3158,15 @@ void filter_is_base(struct repository *r,
bases_nr++;
}
- for (size_t i = 0; i < format->is_base_tips.nr; i++) {
- struct commit *tip = format->is_base_tips.items[i].util;
+ for (size_t i = 0; i < is_base_tips.nr; i++) {
+ struct commit *tip = is_base_tips.items[i].util;
int base_index = get_branch_base_for_tip(r, tip, bases, bases_nr);
if (base_index < 0)
continue;
/* Store the string for use in output later. */
- back_index[base_index]->is_base[i] = xstrdup(format->is_base_tips.items[i].string);
+ back_index[base_index]->is_base[i] = xstrdup(is_base_tips.items[i].string);
}
free(back_index);
@@ -3252,8 +3258,7 @@ struct ref_sorting {
};
static inline int can_do_iterative_format(struct ref_filter *filter,
- struct ref_sorting *sorting,
- struct ref_format *format)
+ struct ref_sorting *sorting)
{
/*
* Reference backends sort patterns lexicographically by refname, so if
@@ -3279,15 +3284,15 @@ static inline int can_do_iterative_format(struct ref_filter *filter,
*/
return !(filter->reachable_from ||
filter->unreachable_from ||
- format->bases.nr ||
- format->is_base_tips.nr);
+ bases.nr ||
+ is_base_tips.nr);
}
void filter_and_format_refs(struct ref_filter *filter, unsigned int type,
struct ref_sorting *sorting,
struct ref_format *format)
{
- if (can_do_iterative_format(filter, sorting, format)) {
+ if (can_do_iterative_format(filter, sorting)) {
int save_commit_buffer_orig;
struct ref_filter_and_format_cbdata ref_cbdata = {
.filter = filter,
@@ -3303,8 +3308,8 @@ 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_is_base(the_repository, format, &array);
+ filter_ahead_behind(the_repository, &array);
+ filter_is_base(the_repository, &array);
ref_array_sort(sorting, &array);
print_formatted_ref_array(&array, format);
ref_array_clear(&array);
@@ -3638,16 +3643,3 @@ void ref_filter_clear(struct ref_filter *filter)
free_commit_list(filter->unreachable_from);
ref_filter_init(filter);
}
-
-void ref_format_init(struct ref_format *format)
-{
- struct ref_format blank = REF_FORMAT_INIT;
- memcpy(format, &blank, sizeof(blank));
-}
-
-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..013d4cfa64 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -99,12 +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;
-
struct {
int max_count;
int omit_empty;
@@ -117,8 +111,6 @@ struct ref_format {
}
#define REF_FORMAT_INIT { \
.use_color = -1, \
- .bases = STRING_LIST_INIT_DUP, \
- .is_base_tips = STRING_LIST_INIT_DUP, \
}
/* Macros for checking --merged and --no-merged options */
@@ -205,7 +197,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);
/*
@@ -215,13 +206,9 @@ void filter_ahead_behind(struct repository *r,
* If this is not called, then any is-base atoms will be blank.
*/
void filter_is_base(struct repository *r,
- struct ref_format *format,
struct ref_array *array);
void ref_filter_init(struct ref_filter *filter);
void ref_filter_clear(struct ref_filter *filter);
-void ref_format_init(struct ref_format *format);
-void ref_format_clear(struct ref_format *format);
-
#endif /* REF_FILTER_H */
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
'
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index 2591f8b8b3..6638d1aa1d 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -733,4 +733,33 @@ test_expect_success 'for-each-ref is-base:multiple' '
--format="%(refname)[%(is-base:commit-2-3)-%(is-base:commit-6-5)]" --stdin
'
+test_expect_success 'for-each-ref is-base: --sort' '
+ cat >input <<-\EOF &&
+ refs/heads/commit-1-1
+ refs/heads/commit-4-2
+ refs/heads/commit-4-4
+ refs/heads/commit-8-4
+ EOF
+
+ cat >expect <<-\EOF &&
+ refs/heads/commit-1-1
+ refs/heads/commit-4-4
+ refs/heads/commit-8-4
+ refs/heads/commit-4-2
+ EOF
+ run_all_modes git for-each-ref \
+ --format="%(refname)" --stdin \
+ --sort=refname --sort=is-base:commit-2-3 &&
+
+ cat >expect <<-\EOF &&
+ refs/heads/commit-4-2
+ refs/heads/commit-1-1
+ refs/heads/commit-4-4
+ refs/heads/commit-8-4
+ EOF
+ run_all_modes git for-each-ref \
+ --format="%(refname)" --stdin \
+ --sort=refname --sort=-is-base:commit-2-3
+'
+
test_done
--
2.48.0
