repo mirror list: reject multiple --sort, filter on owner/repo

4edc7a0 ·

gtrrz-victor · 1w ago · 2 files · +20 added/-8 removed

Two review points:

• --sort is single-column, but a scalar StringVar silently kept the last of repeated flags. Switch to StringArrayVar and reject len>1 in PreRunE with a clear "only one --sort is supported" error, resolving the single value into sortSpec.
• --repo filtered on the bare repo name while the REPO column and sort key use owner/repo, so a value copied from the table (e.g. acme/web) matched nothing. Filter on the owner/repo form on both the mirror and --show-available paths so it matches the row it came from.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Sessions

01KX0DJVNBD74NBKMBXTW201FF View transcript

?\ Refactor Repo Mirror List Sorting and Filtering Claude Code·Opus 4.8 [1m]·7 steps

Changes

2

• cmd/entire/cli

• Mrepo_mirror.go +10/-8

• Mrepo_mirror_test.go +10

146 unmodified lines

return nil
// filterByRepo keeps items whose repo name contains substr (case-insensitive).
// The control plane already filters by owner/provider/cluster server-side but
// not by repo name, so `repo mirror list --repo` narrows that last dimension
// client-side. repoOf extracts the repo field from each item (mirrors and
// available mirrors both carry one). An empty substr returns items unchanged.
// filterByRepo keeps items whose repo identifier contains substr (case-
// insensitive). The control plane already filters by owner/provider/cluster
// server-side but not by repo name, so `repo mirror list --repo` narrows that
// last dimension client-side. repoOf returns the item's displayed identifier —
// the callers pass the owner/repo form shown in the REPO column, so a value
// copied from the table (e.g. acme/web) matches the row it came from. An empty
// substr returns items unchanged.

func filterByRepo[T any](items []T, repoOf func(T) string, substr string) []T {
    substr = strings.TrimSpace(substr)
    if substr == "" {
        return items
    }
    // implementation details...
}

Mcmd/entire/cli/repo_mirror.go +10/-8

532 unmodified lines

require.NotContains(t, stdout, "other/api")
})

t.Run("--repo matches the owner/repo form shown in the REPO column", func(t *testing.T) {
    // A value copied straight from the displayed REPO column must match the
    // row it came from; filtering on the bare repo name would drop it.
    serveMirrorList(t, mirrors, nil)
    stdout, _ := runMirrorList(t, "--repo", "acme/web")
    require.Contains(t, stdout, "acme/web")
    require.NotContains(t, stdout, "acme/cli")
    require.NotContains(t, stdout, "other/api")
})

t.Run("default output is owner/repo sorted", func(t *testing.T) {
    serveMirrorList(t, mirrors, nil)
    stdout, _ := runMirrorList(t)

Mcmd/entire/cli/repo_mirror_test.go +10