Refactor Repo Mirror List Sorting and Filtering · Entire

After reviewing this PR I've got the following feedback:

Sorting Inconsistency

• Inconsistent sortSpec handling between the two list paths.

The --show-available branch passes the raw sortSpec to sortRows (repo_mirror.go:446), while the main mirror branch passes the trimmed normalizedSort. Since sortRows computes desc := strings.HasPrefix(spec, "-") on the raw spec, a leading whitespace spec diverges: --show-available --sort " -repo" errors as an unknown column, while --sort " -repo" on the main list sorts descending. Unlikely input, but trivial to align by trimming once at the top of sortRows (or before both call sites).

Tyebreaking in Sorting

• Non-repo column sorts reintroduce the nondeterministic tie ordering this PR set out to fix.

Only the default (no --sort) and --sort repo/-repo get the (owner/repo, clusterHost) tiebreak via sortMirrorsDefault. Any other column (--sort private, --sort "clone url") falls through to sortRows — a single-key stable sort with no secondary key. The code's own comment calls that order 'arbitrary', so, for example, entire repo mirror list --sort private over an all-private list can emit different orderings across runs.

Recommendations

• Suggest threading the owner/repo secondary key into sortRows, or documenting that only the default / --sort repo ordering is fully deterministic.
• Related: no command-level test covers this sortRows branch for existing mirrors; only the sortMirrorsDefault branch (--sort repo/-repo) is exercised end-to-end.

Code Location Overview

1. sortRows (repo_mirror.go, lines 56-86)

func sortRows[T any](items []T, headers []string, row func(T) []string, spec string) error {
    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 idx < 0 {
            return fmt.Errorf("unknown sort column %q; valid columns: %s", name, strings.ToLower(strings.Join(headers, ", ")))
        }
    }
    sort.SliceStable(items, func(a, b int) bool {
        ka, kb := strings.ToLower(row(items[a])[idx]), strings.ToLower(row(items[b])[idx])
        if desc {
            return ka > kb
        }
        return ka < kb
    })
    return nil
}

2. sortMirrorsDefault (repo_mirror.go, lines 40-54)

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)
    })
}

Conclusion

After checking the sort code, there is a belief that the complexity in sorting the array of elements may be unnecessary given the known columns. The recommendation is to simplify the methods to avoid unnecessary complexity.