repo mirror list: replace generic sortRows with concrete sorters · Entire
repo mirror list: replace generic sortRows with concrete sorters
5b8bb2d→main ·
gtrrz-victor · 1w ago · 2 files · +217 added/-96 removed
Sorting only ever covers two fixed 3-column sets, so the generic sortRows[T]/sortMirrorsDefault split wasn't earning its complexity and had two ordering bugs:
- desc was read from the raw spec but the column name from the trimmed
spec, and the two call sites passed raw vs trimmed, so
--sort " -repo"behaved differently between the mirror and --show-available lists. - only default/
--sort repogot the (owner/repo, clusterHost) tiebreak; every other column fell back to a single-key sort, leaving duplicate-key rows in arbitrary server order across runs.
Replace with a shared parseSortColumn (trims before reading direction) plus concrete sortMirrors/sortAvailable that always apply the owner/repo(+cluster) tiebreak, so every column is deterministic and whitespace is handled the same on both paths.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Sessions
01KX0AYB67E5W5XWBJB70GV80View transcript
Changes
2
cmd/entire/cli
Mrepo_mirror.go +94/-58
Mrepo_mirror_test.go +123/-38
1
2
3
4
5
6
7
1 unmodified line
9
10
11
11
12
13
14
15
12 unmodified lines
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
33
34
35
36
37
43
44
45
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
46
47
48
49
50
51
52
53
54
65
66
67
68
69
70
71
72
55
56
57
58
59
60
61
74
75
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
78
79
87
88
89
90
91
92
93
94
95
81
96
97
83
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
6 unmodified lines
141
142
143
96
97
98
144
145
146
341 unmodified lines
488
489
490
446
491
492
493
494
37 unmodified lines
532
533
534
490
491
492
493
494
495
496
497
498
499
535
536
537
538
package cli
import (
"cmp"
"context"
"errors"
"fmt"
1 unmodified line
"net"
"net/url"
"regexp"
"sort"
"slices"
"strings"
"time"
12 unmodified lines
// `git clone` accepts), since the list API doesn't return it.
var mirrorColumns = []string{"REPO", "CLONE URL", "PRIVATE"}
// mirrorPrivate renders the PRIVATE column ("yes"/"no"), shared by the table
// row and the --sort private key so both agree on the cell value.
func mirrorPrivate(m coreapi.Mirror) string {
if m.IsPrivate.Or(false) {
return "yes"
}
return "no"
}
func mirrorRow(m coreapi.Mirror) []string {
repo := m.Owner + "/" + m.Repo
cloneURL := mirrorCloneURL(m.ClusterHost, m.Owner, m.Repo)
private := "no"
if m.IsPrivate.Or(false) {
private = "yes"
}
return []string{repo, cloneURL, private}
}
return []string{repo, cloneURL, mirrorPrivate(m)}
}
// sortMirrorsDefault orders mirrors by the owner/repo name ascending (matching
// the REPO column and an explicit `--sort repo`), breaking ties by cluster host
// ascending. It's the default (no --sort) ordering: a repo mirrored on several
// clusters would otherwise sit in arbitrary server order. Case-insensitive,
// matching sortRows.
func sortMirrorsDefault(mirrors []coreapi.Mirror) {
sort.SliceStable(mirrors, func(i, j int) bool {
ri := strings.ToLower(mirrors[i].Owner + "/" + mirrors[i].Repo)
rj := strings.ToLower(mirrors[j].Owner + "/" + mirrors[j].Repo)
if ri != rj {
return ri < rj
}
return strings.ToLower(mirrors[i].ClusterHost) < strings.ToLower(mirrors[j].ClusterHost)
})
}
// sortRows orders items in place by one column's rendered value, ascending
// case-insensitive string order. spec is a header name; a leading '-' sorts
// descending. An empty spec sorts by the first column. An unknown column name
// is an error naming the valid columns. Stable, so rows equal on the sort
// column keep their input order. row must return one cell per header, which the
// mirror/available row funcs do.
func sortRows[T any](items []T, headers []string, row func(T) []string, spec string) error {
desc := strings.HasPrefix(spec, "-")
// parseSortColumn resolves a --sort spec to a lowercased column name and
// direction against columns. It trims first, then reads the '-' prefix, so
// leading/trailing whitespace is handled identically on every path (the
// direction and the column name never disagree). An empty spec selects the
// first column. An unknown name errors naming the valid columns.
func parseSortColumn(spec string, columns []string) (col string, desc bool, err error) {
spec = strings.TrimSpace(spec)
desc = strings.HasPrefix(spec, "-")
name := strings.ToLower(strings.TrimSpace(strings.TrimPrefix(spec, "-")))
idx := 0
if name != "" {
idx = -1
for i, h := range headers {
if strings.EqualFold(h, name) {
idx = i
break
}
if name == "" {
return strings.ToLower(columns[0]), desc, nil
}
for _, h := range columns {
if strings.EqualFold(h, name) {
return name, desc, nil
}
if idx < 0 {
return fmt.Errorf("unknown sort column %q; valid columns: %s", name, strings.ToLower(strings.Join(headers, ", ")))
}
return "", false, fmt.Errorf("unknown sort column %q; valid columns: %s", name, strings.ToLower(strings.Join(columns, ", ")))
}
// sortMirrors orders mirrors in place by the --sort spec: by the named column's
// value ascending (case-insensitive), always breaking ties by owner/repo then
// cluster host so a repo mirrored across clusters (or rows equal on any other
// column) has a stable, deterministic order rather than arbitrary server order.
// A '-' prefix reverses the whole ordering. `repo`/default sorts by the
// tiebreak alone.
func sortMirrors(mirrors []coreapi.Mirror, spec string) error {
col, desc, err := parseSortColumn(spec, mirrorColumns)
if err != nil {
return err
}
key := func(m coreapi.Mirror) string {
switch col {
case "clone url":
return strings.ToLower(mirrorCloneURL(m.ClusterHost, m.Owner, m.Repo))
case "private":
return mirrorPrivate(m)
default: // repo -> tiebreak alone
return ""
}
}
sort.SliceStable(items, func(a, b int) bool {
ka, kb := strings.ToLower(row(items[a])[idx]), strings.ToLower(row(items[b])[idx])
slices.SortStableFunc(mirrors, func(a, b coreapi.Mirror) int {
c := cmp.Compare(key(a), key(b))
if c == 0 {
c = cmp.Compare(strings.ToLower(a.Owner + "/" + a.Repo), strings.ToLower(b.Owner + "/" + b.Repo))
}
if c == 0 {
c = cmp.Compare(strings.ToLower(a.ClusterHost), strings.ToLower(b.ClusterHost))
}
if desc {
return ka > kb
return -c
}
return ka < kb
return c
})
return nil
}
// sortAvailable orders available mirrors in place by the --sort spec, matching
// sortMirrors: by the named column ascending (case-insensitive) with an
// owner/repo tiebreak for a deterministic order on equal keys. AvailableMirror
// has no cluster host (the onboardable set is cluster-agnostic), so owner/repo
// is the only secondary key. A '-' prefix reverses the whole ordering.
func sortAvailable(avail []coreapi.AvailableMirror, spec string) error {
col, desc, err := parseSortColumn(spec, availableMirrorColumns)
if err != nil {
return err
}
key := func(m coreapi.AvailableMirror) string {
switch col {
case "access":
return strings.ToLower(string(m.Access))
case "status":
return strings.ToLower(string(m.Status))
default: // repo -> tiebreak alone
return ""
}
}
slices.SortStableFunc(avail, func(a, b coreapi.AvailableMirror) int {
c := cmp.Compare(key(a), key(b))
if c == 0 {
c = cmp.Compare(strings.ToLower(a.Owner + "/" + a.Repo), strings.ToLower(b.Owner + "/" + b.Repo))
}
if desc {
return -c
}
return c
})
return nil
}
func filterByRepo[T any](items []T, repoOf func(T) string, substr string) []T {
substr = strings.TrimSpace(substr)
if substr == "" {
if items == nil {
return []T{}
}
return items
}
substr = strings.ToLower(substr)
341 unmodified lines
return nil, err
}
avail := filterByRepo(out.Available, func(m coreapi.AvailableMirror) string { return m.Repo }, repo)
if err := sortRows(avail, availableMirrorColumns, availableMirrorRow, sortSpec); err != nil {
if err := sortAvailable(avail, sortSpec); err != nil {
return nil, err
}
return avail, nil
}
37 unmodified lines
return nil, err
}
mirrors = filterByRepo(mirrors, func(m coreapi.Mirror) string { return m.Repo }, repo)
normalizedSort := strings.TrimSpace(sortSpec)
repoSort := strings.EqualFold(strings.TrimSpace(strings.TrimPrefix(normalizedSort, "-")), "repo")
if normalizedSort == "" || repoSort {
sortMirrorsDefault(mirrors)
if strings.HasPrefix(normalizedSort, "-") {
for i, j := 0, len(mirrors)-1; i < j; i, j = i+1, j-1 {
mirrors[i], mirrors[j] = mirrors[j], mirrors[i]
}
}
} else if err := sortRows(mirrors, mirrorColumns, mirrorRow, normalizedSort); err != nil {
if err := sortMirrors(mirrors, sortSpec); err != nil {
return nil, err
}
return mirrors, nil
}
```