Skip to content

Commit 5126145

Browse files
gitstergitster
committed
Merge branch 'jc/fix-ref-sorting-parse'
Things like "git -c branch.sort=bogus branch new HEAD", i.e. the operation modes of the "git branch" command that do not need the sort key information, no longer errors out by seeing a bogus sort key. * jc/fix-ref-sorting-parse: for-each-ref: delay parsing of --sort=<atom> options
2 parents 44ac8fd + 98e7ab6 commit 5126145

File tree

8 files changed

+95
-52
lines changed

8 files changed

+95
-52
lines changed

‎builtin/branch.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,11 @@ define_list_config_array(color_branch_slots);
7777
static int git_branch_config(const char *var, const char *value, void *cb)
7878
{
7979
const char *slot_name;
80-
struct ref_sorting **sorting_tail = (struct ref_sorting **)cb;
8180

8281
if (!strcmp(var, "branch.sort")) {
8382
if (!value)
8483
return config_error_nonbool(var);
85-
parse_ref_sorting(sorting_tail, value);
84+
string_list_append(cb, value);
8685
return 0;
8786
}
8887

@@ -625,7 +624,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
625624
enum branch_track track;
626625
struct ref_filter filter;
627626
int icase = 0;
628-
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
627+
static struct ref_sorting *sorting;
628+
struct string_list sorting_options = STRING_LIST_INIT_DUP;
629629
struct ref_format format = REF_FORMAT_INIT;
630630

631631
struct option options[] = {
@@ -666,7 +666,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
666666
OPT_MERGED(&filter, N_("print only branches that are merged")),
667667
OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
668668
OPT_COLUMN(0, "column", &colopts, N_("list branches in columns")),
669-
OPT_REF_SORT(sorting_tail),
669+
OPT_REF_SORT(&sorting_options),
670670
OPT_CALLBACK(0, "points-at", &filter.points_at, N_("object"),
671671
N_("print only branches of the object"), parse_opt_object_name),
672672
OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
@@ -683,7 +683,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
683683
if (argc == 2 && !strcmp(argv[1], "-h"))
684684
usage_with_options(builtin_branch_usage, options);
685685

686-
git_config(git_branch_config, sorting_tail);
686+
git_config(git_branch_config, &sorting_options);
687687

688688
track = git_branch_track;
689689

@@ -749,8 +749,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
749749
* local branches 'refs/heads/...' and finally remote-tracking
750750
* branches 'refs/remotes/...'.
751751
*/
752-
if (!sorting)
753-
sorting = ref_default_sorting();
752+
sorting = ref_sorting_options(&sorting_options);
754753
ref_sorting_set_sort_flags_all(sorting, REF_SORTING_ICASE, icase);
755754
ref_sorting_set_sort_flags_all(
756755
sorting, REF_SORTING_DETACHED_HEAD_FIRST, 1);

‎builtin/for-each-ref.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ static char const * const for_each_ref_usage[] = {
1717
int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
1818
{
1919
int i;
20-
struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
20+
struct ref_sorting *sorting;
21+
struct string_list sorting_options = STRING_LIST_INIT_DUP;
2122
int maxcount = 0, icase = 0;
2223
struct ref_array array;
2324
struct ref_filter filter;
@@ -39,7 +40,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
3940
OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
4041
OPT_STRING( 0 , "format", &format.format, N_("format"), N_("format to use for the output")),
4142
OPT__COLOR(&format.use_color, N_("respect format colors")),
42-
OPT_REF_SORT(sorting_tail),
43+
OPT_REF_SORT(&sorting_options),
4344
OPT_CALLBACK(0, "points-at", &filter.points_at,
4445
N_("object"), N_("print only refs which points at the given object"),
4546
parse_opt_object_name),
@@ -70,8 +71,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
7071
if (verify_ref_format(&format))
7172
usage_with_options(for_each_ref_usage, opts);
7273

73-
if (!sorting)
74-
sorting = ref_default_sorting();
74+
sorting = ref_sorting_options(&sorting_options);
7575
ref_sorting_set_sort_flags_all(sorting, REF_SORTING_ICASE, icase);
7676
filter.ignore_case = icase;
7777

‎builtin/ls-remote.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
5454
struct transport *transport;
5555
const struct ref *ref;
5656
struct ref_array ref_array;
57-
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
57+
struct string_list sorting_options = STRING_LIST_INIT_DUP;
5858

5959
struct option options[] = {
6060
OPT__QUIET(&quiet, N_("do not print remote URL")),
@@ -68,7 +68,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
6868
OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
6969
OPT_BOOL(0, "get-url", &get_url,
7070
N_("take url.<base>.insteadOf into account")),
71-
OPT_REF_SORT(sorting_tail),
71+
OPT_REF_SORT(&sorting_options),
7272
OPT_SET_INT_F(0, "exit-code", &status,
7373
N_("exit with exit code 2 if no matching refs are found"),
7474
2, PARSE_OPT_NOCOMPLETE),
@@ -86,8 +86,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
8686

8787
packet_trace_identity("ls-remote");
8888

89-
UNLEAK(sorting);
90-
9189
if (argc > 1) {
9290
int i;
9391
CALLOC_ARRAY(pattern, argc);
@@ -139,8 +137,13 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
139137
item->symref = xstrdup_or_null(ref->symref);
140138
}
141139

142-
if (sorting)
140+
if (sorting_options.nr) {
141+
struct ref_sorting *sorting;
142+
143+
sorting = ref_sorting_options(&sorting_options);
143144
ref_array_sort(sorting, &ref_array);
145+
ref_sorting_release(sorting);
146+
}
144147

145148
for (i = 0; i < ref_array.nr; i++) {
146149
const struct ref_array_item *ref = ref_array.items[i];

‎builtin/tag.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,6 @@ static const char tag_template_nocleanup[] =
178178
static int git_tag_config(const char *var, const char *value, void *cb)
179179
{
180180
int status;
181-
struct ref_sorting **sorting_tail = (struct ref_sorting **)cb;
182181

183182
if (!strcmp(var, "tag.gpgsign")) {
184183
config_sign_tag = git_config_bool(var, value);
@@ -188,7 +187,7 @@ static int git_tag_config(const char *var, const char *value, void *cb)
188187
if (!strcmp(var, "tag.sort")) {
189188
if (!value)
190189
return config_error_nonbool(var);
191-
parse_ref_sorting(sorting_tail, value);
190+
string_list_append(cb, value);
192191
return 0;
193192
}
194193

@@ -436,7 +435,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
436435
struct ref_transaction *transaction;
437436
struct strbuf err = STRBUF_INIT;
438437
struct ref_filter filter;
439-
static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
438+
struct ref_sorting *sorting;
439+
struct string_list sorting_options = STRING_LIST_INIT_DUP;
440440
struct ref_format format = REF_FORMAT_INIT;
441441
int icase = 0;
442442
int edit_flag = 0;
@@ -470,7 +470,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
470470
OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")),
471471
OPT_MERGED(&filter, N_("print only tags that are merged")),
472472
OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
473-
OPT_REF_SORT(sorting_tail),
473+
OPT_REF_SORT(&sorting_options),
474474
{
475475
OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
476476
N_("print only tags of the object"), PARSE_OPT_LASTARG_DEFAULT,
@@ -486,7 +486,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
486486

487487
setup_ref_filter_porcelain_msg();
488488

489-
git_config(git_tag_config, sorting_tail);
489+
git_config(git_tag_config, &sorting_options);
490490

491491
memset(&opt, 0, sizeof(opt));
492492
memset(&filter, 0, sizeof(filter));
@@ -525,8 +525,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
525525
die(_("--column and -n are incompatible"));
526526
colopts = 0;
527527
}
528-
if (!sorting)
529-
sorting = ref_default_sorting();
528+
sorting = ref_sorting_options(&sorting_options);
530529
ref_sorting_set_sort_flags_all(sorting, REF_SORTING_ICASE, icase);
531530
filter.ignore_case = icase;
532531
if (cmdmode == 'l') {

‎ref-filter.c

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2470,6 +2470,12 @@ static int memcasecmp(const void *vs1, const void *vs2, size_t n)
24702470
return 0;
24712471
}
24722472

2473+
struct ref_sorting {
2474+
struct ref_sorting *next;
2475+
int atom; /* index into used_atom array (internal) */
2476+
enum ref_sorting_order sort_flags;
2477+
};
2478+
24732479
static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
24742480
{
24752481
struct atom_value *va, *vb;
@@ -2663,7 +2669,7 @@ static int parse_sorting_atom(const char *atom)
26632669
}
26642670

26652671
/* If no sorting option is given, use refname to sort as default */
2666-
struct ref_sorting *ref_default_sorting(void)
2672+
static struct ref_sorting *ref_default_sorting(void)
26672673
{
26682674
static const char cstr_name[] = "refname";
26692675

@@ -2674,7 +2680,7 @@ struct ref_sorting *ref_default_sorting(void)
26742680
return sorting;
26752681
}
26762682

2677-
void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg)
2683+
static void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg)
26782684
{
26792685
struct ref_sorting *s;
26802686

@@ -2692,17 +2698,25 @@ void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg)
26922698
s->atom = parse_sorting_atom(arg);
26932699
}
26942700

2695-
int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
2701+
struct ref_sorting *ref_sorting_options(struct string_list *options)
26962702
{
2703+
struct string_list_item *item;
2704+
struct ref_sorting *sorting = NULL, **tail = &sorting;
2705+
2706+
if (!options->nr) {
2707+
sorting = ref_default_sorting();
2708+
} else {
2709+
for_each_string_list_item(item, options)
2710+
parse_ref_sorting(tail, item->string);
2711+
}
2712+
26972713
/*
2698-
* NEEDSWORK: We should probably clear the list in this case, but we've
2699-
* already munged the global used_atoms list, which would need to be
2700-
* undone.
2714+
* From here on, the ref_sorting list should be used to talk
2715+
* about the sort order used for the output. The caller
2716+
* should not touch the string form anymore.
27012717
*/
2702-
BUG_ON_OPT_NEG(unset);
2703-
2704-
parse_ref_sorting(opt->value, arg);
2705-
return 0;
2718+
string_list_clear(options, 0);
2719+
return sorting;
27062720
}
27072721

27082722
void ref_sorting_release(struct ref_sorting *sorting)

‎ref-filter.h

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,13 @@
2323
#define FILTER_REFS_KIND_MASK (FILTER_REFS_ALL | FILTER_REFS_DETACHED_HEAD)
2424

2525
struct atom_value;
26+
struct ref_sorting;
2627

27-
struct ref_sorting {
28-
struct ref_sorting *next;
29-
int atom; /* index into used_atom array (internal) */
30-
enum {
31-
REF_SORTING_REVERSE = 1<<0,
32-
REF_SORTING_ICASE = 1<<1,
33-
REF_SORTING_VERSION = 1<<2,
34-
REF_SORTING_DETACHED_HEAD_FIRST = 1<<3,
35-
} sort_flags;
28+
enum ref_sorting_order {
29+
REF_SORTING_REVERSE = 1<<0,
30+
REF_SORTING_ICASE = 1<<1,
31+
REF_SORTING_VERSION = 1<<2,
32+
REF_SORTING_DETACHED_HEAD_FIRST = 1<<3,
3633
};
3734

3835
struct ref_array_item {
@@ -97,9 +94,8 @@ struct ref_format {
9794
#define OPT_NO_MERGED(f, h) _OPT_MERGED_NO_MERGED("no-merged", f, h)
9895

9996
#define OPT_REF_SORT(var) \
100-
OPT_CALLBACK_F(0, "sort", (var), \
101-
N_("key"), N_("field name to sort on"), \
102-
PARSE_OPT_NONEG, parse_opt_ref_sorting)
97+
OPT_STRING_LIST(0, "sort", (var), \
98+
N_("key"), N_("field name to sort on"))
10399

104100
/*
105101
* API for filtering a set of refs. Based on the type of refs the user
@@ -121,14 +117,10 @@ int format_ref_array_item(struct ref_array_item *info,
121117
struct ref_format *format,
122118
struct strbuf *final_buf,
123119
struct strbuf *error_buf);
124-
/* Parse a single sort specifier and add it to the list */
125-
void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *atom);
126-
/* Callback function for parsing the sort option */
127-
int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset);
128-
/* Default sort option based on refname */
129-
struct ref_sorting *ref_default_sorting(void);
130120
/* Release a "struct ref_sorting" */
131121
void ref_sorting_release(struct ref_sorting *);
122+
/* Convert list of sort options into ref_sorting */
123+
struct ref_sorting *ref_sorting_options(struct string_list *);
132124
/* Function to parse --merged and --no-merged options */
133125
int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);
134126
/* Get the current HEAD's description */

‎t/t3200-branch.sh

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,17 @@ test_expect_success 'invalid sort parameter in configuration' '
14401440
(
14411441
cd sort &&
14421442
git config branch.sort "v:notvalid" &&
1443-
test_must_fail git branch
1443+
1444+
# this works in the "listing" mode, so bad sort key
1445+
# is a dying offence.
1446+
test_must_fail git branch &&
1447+
1448+
# these do not need to use sorting, and should all
1449+
# succeed
1450+
git branch newone main &&
1451+
git branch -c newone newerone &&
1452+
git branch -m newone newestone &&
1453+
git branch -d newerone newestone
14441454
)
14451455
'
14461456

‎t/t6300-for-each-ref.sh

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,11 @@ test_expect_success 'Verify descending sort' '
419419
test_cmp expected actual
420420
'
421421

422+
test_expect_success 'Give help even with invalid sort atoms' '
423+
test_expect_code 129 git for-each-ref --sort=bogus -h >actual 2>&1 &&
424+
grep "^usage: git for-each-ref" actual
425+
'
426+
422427
cat >expected <<\EOF
423428
refs/tags/testtag
424429
refs/tags/testtag-2
@@ -1019,6 +1024,27 @@ test_expect_success 'equivalent sorts fall back on refname' '
10191024
test_cmp expected actual
10201025
'
10211026

1027+
test_expect_success '--no-sort cancels the previous sort keys' '
1028+
cat >expected <<-\EOF &&
1029+
100000 <[email protected]> refs/tags/multi-ref1-100000-user1
1030+
100000 <[email protected]> refs/tags/multi-ref1-100000-user2
1031+
100000 <[email protected]> refs/tags/multi-ref2-100000-user1
1032+
100000 <[email protected]> refs/tags/multi-ref2-100000-user2
1033+
200000 <[email protected]> refs/tags/multi-ref1-200000-user1
1034+
200000 <[email protected]> refs/tags/multi-ref1-200000-user2
1035+
200000 <[email protected]> refs/tags/multi-ref2-200000-user1
1036+
200000 <[email protected]> refs/tags/multi-ref2-200000-user2
1037+
EOF
1038+
git for-each-ref \
1039+
--format="%(taggerdate:unix) %(taggeremail) %(refname)" \
1040+
--sort=-refname \
1041+
--sort=taggeremail \
1042+
--no-sort \
1043+
--sort=taggerdate \
1044+
"refs/tags/multi-*" >actual &&
1045+
test_cmp expected actual
1046+
'
1047+
10221048
test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '
10231049
test_when_finished "git checkout main" &&
10241050
git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&

0 commit comments

Comments
 (0)