diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml new file mode 100644 index 0000000..94efe48 --- /dev/null +++ b/.github/workflows/checks.yaml @@ -0,0 +1,65 @@ +name: Checks + +# Reusable: called by ci.yaml on pushes and pull requests, and by +# release.yaml before goreleaser publishes anything. Defined once here so the +# checks that gate a release are exactly the ones a PR had to pass. +on: + workflow_call: + +permissions: + contents: read + +jobs: + lint: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + # Read from go.mod so CI can never drift from what the module + # requires (it says 1.25.5, so a pinned 1.24 silently made every + # job download a newer toolchain first). + go-version-file: go.mod + - name: golangci-lint + uses: golangci/golangci-lint-action@v8 + + test: + name: test (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + include: + # -race needs a C toolchain, which the Windows runner doesn't have + # set up by default. + - os: ubuntu-latest + raceflag: "-race" + - os: windows-latest + raceflag: "" + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: go vet + run: go vet ./... + # Contract tests are behind the `contract` build tag and hit the live + # hub, so they stay out of CI. + - name: go test + run: go test ${{ matrix.raceflag }} ./... + + modules: + name: modules + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Check mod files + run: | + go mod tidy + git diff --exit-code go.mod go.sum + - name: Verify module hashes + run: go mod verify diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..dd282a8 --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,22 @@ +name: CI + +# A PR from a branch in this repo only fires the pull_request event (its push +# doesn't match main), so this doesn't double up. Tag pushes don't match +# either - those go to release.yaml. +on: + pull_request: + push: + branches: + - main + +permissions: + contents: read + +# A new push to a PR makes the in-flight run pointless. +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + checks: + uses: ./.github/workflows/checks.yaml diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index cfc04d6..b3f96f9 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -9,20 +9,14 @@ permissions: contents: write jobs: - lint: - name: lint - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 - with: - go-version: 1.24 - - name: golangci-lint - uses: golangci/golangci-lint-action@v8 + # Same lint/test/module checks a pull request has to pass, so a tag can't + # publish something CI would have rejected. + checks: + uses: ./.github/workflows/checks.yaml goreleaser: runs-on: ubuntu-latest - needs: lint + needs: checks steps: - name: Checkout uses: actions/checkout@v4 @@ -32,15 +26,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: 1.24 - - - name: Check mod files - run: | - go mod tidy - git diff --exit-code go.mod go.sum - - - name: Verify module hashes - run: go mod verify + go-version-file: go.mod - name: Run GoReleaser uses: goreleaser/goreleaser-action@v6 @@ -49,4 +35,4 @@ jobs: version: "~> v2" args: release --clean env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/README.md b/README.md index e0a98d2..07323a1 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,28 @@ Some [basic usage](#basic-usage) covered here, for more info, run `stellar --hel > > You can force copy mode anywhere (e.g. for testing) by setting `STELLAR_APPLY_MODE=copy`. +### Shell completion + +stellar ships tab completion for commands, flags, and theme identifiers +(`author/slug@version`). Candidates come from your local theme cache, so +completion is always instant and works offline. If you want hub themes +suggested too, set `STELLAR_COMPLETION_ONLINE=1` (adds up to ~2s of network +lookup per completion; degrades silently to local-only when offline). + +```bash +# bash +stellar completion bash > ~/.local/share/bash-completion/completions/stellar + +# zsh +stellar completion zsh > "${fpath[1]}/_stellar" + +# fish +stellar completion fish > ~/.config/fish/completions/stellar.fish + +# powershell (add to $PROFILE) +stellar completion powershell | Out-String | Invoke-Expression +``` + ## Why use **Before:** Getting good starship configs so far was mostly random, from someones github dotfiles, searching for something entirely else... @@ -256,7 +278,7 @@ When adding new CLI features, please add corresponding E2E tests in `cmd/e2e_tes - [ ] **Preview: fix bash formatting** - [ ] **`stellar preview` on Windows**: `cmd/preview.go` only spawns terminals on macOS/Linux and returns "unsupported platform" on Windows. Needs a Windows Terminal / PowerShell branch that opens a shell with `STARSHIP_CONFIG` set. - [ ] **Windows packaging**: consider scoop/winget packaging (leftover `stellar.exe.old` from self-update is already cleaned up on the next run). -- [ ] **CI test job**: the release workflow runs no `go test` today; add one (ideally with a `windows-latest` runner) to guard the copy path natively. +- [x] **CI test job**: `go vet` and `go test` (with `-race` on Linux) run on every pull request and again before goreleaser, on both `ubuntu-latest` and `windows-latest`, so a tag can't publish a failing build and the copy path is guarded natively. - [ ] **`stellar publish` command**: Upload local themes directly to stellar-hub - Challenge: Need to implement CLI authentication (OAuth flow with browser redirect or API keys) - Would read from `~/.config/stellar///.toml` diff --git a/cmd/completion_args.go b/cmd/completion_args.go new file mode 100644 index 0000000..0774406 --- /dev/null +++ b/cmd/completion_args.go @@ -0,0 +1,85 @@ +package cmd + +import ( + "os" + "strings" + + "github.com/a3chron/stellar/internal/completion" + "github.com/spf13/cobra" +) + +// init wires up shell tab-completion for the commands that take theme +// identifiers ("author/slug@version"). It lives in its own file rather than +// touching apply.go/preview.go/info.go/remove.go directly so those files' +// existing structure stays untouched. +func init() { + applyCmd.ValidArgsFunction = themeIdentifierArgs + previewCmd.ValidArgsFunction = themeIdentifierArgs + infoCmd.ValidArgsFunction = themeIdentifierArgs + removeCmd.ValidArgsFunction = removeIdentifierArgs + + // Commands that take no arguments still need this: with no + // ValidArgsFunction cobra returns ShellCompDirectiveDefault and the shell + // falls back to offering filenames, so `stellar list ` would list the + // user's working directory. + for _, c := range []*cobra.Command{ + listCmd, cleanCmd, currentCmd, rollbackCmd, updateCmd, versionCmd, + } { + c.ValidArgsFunction = cobra.NoFileCompletions + } +} + +// themeCompletionMode returns the candidate sources for apply/preview/info +// completion. Local-only by default: even a 2s hub round trip makes TAB feel +// broken, and most users complete themes they already have cached. Setting +// STELLAR_COMPLETION_ONLINE=1 opts in to hub suggestions. +// +// Read per invocation (not in init) because every shell completion request +// is its own process and tests toggle the variable at runtime. +func themeCompletionMode() completion.Mode { + if os.Getenv(completion.EnvOnline) == "1" || os.Getenv(completion.EnvOnline) == "true" { + return completion.LocalAndRemote + } + return completion.LocalOnly +} + +// themeIdentifierArgs is the ValidArgsFunction for commands that accept +// exactly one identifier (apply, preview, info): once that argument is +// already typed, there's nothing left to complete. +func themeIdentifierArgs(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + if len(args) > 0 { + return nil, cobra.ShellCompDirectiveNoFileComp + } + return completion.ThemeIdentifier(toComplete, themeCompletionMode()) +} + +// removeIdentifierArgs completes "stellar remove" arguments from the local +// cache only - matching remove's own semantics, it never touches the +// network - and, since remove accepts several identifiers, filters out any +// candidate that's already been typed on the command line so repeated +// completion doesn't keep re-suggesting the same theme. +func removeIdentifierArgs(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + candidates, directive := completion.ThemeIdentifier(toComplete, completion.LocalOnly) + if len(candidates) == 0 || len(args) == 0 { + return candidates, directive + } + + already := make(map[string]bool, len(args)) + for _, a := range args { + already[a] = true + } + + filtered := make([]string, 0, len(candidates)) + for _, c := range candidates { + value := c + if idx := strings.IndexByte(c, '\t'); idx != -1 { + value = c[:idx] + } + if already[value] { + continue + } + filtered = append(filtered, c) + } + + return filtered, directive +} diff --git a/cmd/completion_test.go b/cmd/completion_test.go new file mode 100644 index 0000000..f82278d --- /dev/null +++ b/cmd/completion_test.go @@ -0,0 +1,521 @@ +// Package cmd contains E2E tests for stellar CLI commands, including shell +// tab-completion (see internal/completion for the implementation these +// tests exercise end-to-end via the hidden cobra "__complete" command). +package cmd + +import ( + "bytes" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/a3chron/stellar/internal/completion" + "github.com/a3chron/stellar/internal/testutil" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// enableOnlineCompletion opts the test in to hub-backed completion for +// apply/preview/info - the default is local-only for speed. +func enableOnlineCompletion(t *testing.T) { + t.Helper() + t.Setenv(completion.EnvOnline, "1") +} + +// runComplete invokes the hidden "__complete" command with args and returns +// the output split into lines: zero or more candidate lines, followed by a +// trailing ":" line (see cobra's completions.go - the directive +// integer is always the last line, following a single colon). +func runComplete(t *testing.T, args ...string) []string { + t.Helper() + + cmd := NewRootCmd() + var out, errOut bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&errOut) + cmd.SetArgs(append([]string{cobra.ShellCompRequestCmd}, args...)) + + err := cmd.Execute() + require.NoError(t, err, "stderr: %s", errOut.String()) + + trimmed := strings.TrimRight(out.String(), "\n") + if trimmed == "" { + return nil + } + return strings.Split(trimmed, "\n") +} + +// directiveLine returns the last line of a runComplete result (the +// ":" line). +func directiveLine(lines []string) string { + if len(lines) == 0 { + return "" + } + return lines[len(lines)-1] +} + +// candidateLines returns every line except the trailing directive line. +func candidateLines(lines []string) []string { + if len(lines) == 0 { + return nil + } + return lines[:len(lines)-1] +} + +func TestCompletion_EmptyInput_LocalAuthorsOnly(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("local", "mytheme", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "") + + assert.Equal(t, ":6", directiveLine(lines)) + assert.Equal(t, []string{"local/\tlocal", "testuser/\tlocal"}, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_LocalAuthorPrefix_SuppressesRemote(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("local", "mytheme", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "lo") + + assert.Equal(t, []string{"local/\tlocal"}, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_UnknownAuthorPrefix_FallsBackToHub(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("local", "mytheme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "other") + + assert.Equal(t, []string{"otheruser/\thub"}, candidateLines(lines)) + assert.GreaterOrEqual(t, mockAPI.Requests("/api/themes"), 1) +} + +func TestCompletion_AuthorSlash_LocalThenRemoteDeduped(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + // "sample-theme" exists both locally and on the hub (should be deduped, + // kept as the local entry); "local-only" only exists locally; + // "custom-theme" only exists on the hub. + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + env.CreateThemeFile("testuser", "local-only", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "testuser/") + + assert.Equal(t, ":36", directiveLine(lines)) + assert.Equal(t, []string{ + "testuser/local-only\tlocal", + "testuser/sample-theme\tlocal", + "testuser/custom-theme\thub", + }, candidateLines(lines)) + assert.GreaterOrEqual(t, mockAPI.Requests("/api/themes"), 1) +} + +func TestCompletion_RemoteOnlyAuthor_Slash(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + lines := runComplete(t, "apply", "otheruser/") + + assert.Equal(t, []string{"otheruser/ocean-theme\thub"}, candidateLines(lines)) +} + +func TestCompletion_VersionStage_LocalThenRemoteThenLatest(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + // Only 1.0 is cached locally; the hub (via CreateDefaultMockAPI) has + // 1.2, 1.1 and 1.0 for testuser/sample-theme. + env.CreateThemeFile("testuser", "sample-theme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "testuser/sample-theme@") + + assert.Equal(t, ":36", directiveLine(lines)) + assert.Equal(t, []string{ + "testuser/sample-theme@1.0\tlocal", + "testuser/sample-theme@1.2\thub", + "testuser/sample-theme@1.1\thub", + "testuser/sample-theme@latest", + }, candidateLines(lines)) +} + +func TestCompletion_Offline_DegradesToLocalOnly(t *testing.T) { + testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + // Nothing is listening on this port: connection should fail fast rather + // than hang for the full 2s completion-client timeout. + t.Setenv("STELLAR_API_URL", "http://127.0.0.1:1") + + lines := runComplete(t, "apply", "anything") + + assert.Empty(t, candidateLines(lines)) + assert.Equal(t, ":6", directiveLine(lines)) +} + +func TestCompletion_Remove_LocalOnly_NeverHitsAPI(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + + lines := runComplete(t, "remove", "testuser/") + + assert.Equal(t, []string{"testuser/sample-theme\tlocal"}, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_Remove_ExcludesAlreadyTypedArgs(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + env.CreateThemeFile("testuser", "sample-theme", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("testuser", "other-theme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "remove", "testuser/sample-theme", "testuser/") + + assert.Equal(t, []string{"testuser/other-theme\tlocal"}, candidateLines(lines)) +} + +func TestCompletion_MissingStellarDir_NoErrorEmptyOutput(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + require.NoError(t, os.RemoveAll(env.StellarDir)) + + lines := runComplete(t, "apply", "") + + assert.Empty(t, candidateLines(lines)) +} + +func TestCompletion_BackupTheme_SkipsRemoteLookup(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("someauthor", "backup", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "someauthor/backup@") + + assert.Contains(t, candidateLines(lines), "someauthor/backup@1.0\tlocal") + assert.Equal(t, 0, mockAPI.Requests("/api/someauthor/backup")) +} + +func TestCompletion_EmptyAuthorSegment_NoCandidates(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + // "/x" would otherwise list author dirs as slugs of an empty author and + // fire an unfiltered hub query - both must be suppressed. + env.CreateThemeFile("xylo", "mytheme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "/x") + + assert.Empty(t, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_EmptySlugSegment_NoCandidates(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + + // "alice/@latest" would not parse, so "alice/@" must complete to nothing. + lines := runComplete(t, "apply", "alice/@") + + assert.Empty(t, candidateLines(lines)) +} + +func TestCompletion_VersionStage_VPrefix(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("testuser", "sample-theme", "1.0", testutil.SampleTOML()) + + // The parser accepts "@v1.0", so "@v" should offer v-prefixed versions + // (and never a nonsensical "vlatest"). + lines := runComplete(t, "apply", "testuser/sample-theme@v") + + assert.Equal(t, []string{ + "testuser/sample-theme@v1.0\tlocal", + "testuser/sample-theme@v1.2\thub", + "testuser/sample-theme@v1.1\thub", + }, candidateLines(lines)) +} + +func TestCompletion_HubCanonicalAuthorCasing(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + mockAPI.AddTheme(testutil.MockTheme{ + ID: "cased-id", + Author: "CasedUser", + Slug: "neon-theme", + Name: "Neon Theme", + Versions: []testutil.MockVersion{ + {Version: "1.0", ConfigContent: testutil.SampleTOML(), CreatedAt: "2024-03-01T00:00:00Z"}, + }, + }) + env.SetupMockAPI(mockAPI) + + // The hub's /api/{author}/{slug} routes match author names exactly, and + // every shell filters candidates against the typed word (bash's compgen + // and zsh's compadd case-sensitively). So the hub's canonical casing + // completes... + lines := runComplete(t, "apply", "CasedUser/") + assert.Equal(t, []string{"CasedUser/neon-theme\thub"}, candidateLines(lines)) + + // ...while a differently-cased prefix suggests nothing, rather than + // emitting a candidate that bash and zsh would silently discard. + lines = runComplete(t, "apply", "caseduser/") + assert.Empty(t, candidateLines(lines)) +} + +func TestCompletion_Default_UnknownAuthor_LocalOnlyNoNetwork(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + // Without STELLAR_COMPLETION_ONLINE, an unknown author prefix must NOT + // fall back to the hub - completion stays instant and offline. + lines := runComplete(t, "apply", "other") + + assert.Empty(t, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_Default_SlugStage_LocalOnlyNoNetwork(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "testuser/") + + assert.Equal(t, []string{"testuser/sample-theme\tlocal"}, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_Default_VersionStage_LocalPlusLatestNoNetwork(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("testuser", "sample-theme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "testuser/sample-theme@") + + assert.Equal(t, []string{ + "testuser/sample-theme@1.0\tlocal", + "testuser/sample-theme@latest", + }, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_HostileHubValues_Filtered(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + // Author and slug outside the identifier character class (ANSI escape, + // colon, space) must never reach the user's terminal as candidates. + mockAPI.AddTheme(testutil.MockTheme{ + ID: "evil-id", + Author: "evil\x1b[31muser", + Slug: "bad:theme name", + Name: "Evil", + Versions: []testutil.MockVersion{ + {Version: "1.0", ConfigContent: testutil.SampleTOML(), CreatedAt: "2024-03-01T00:00:00Z"}, + }, + }) + env.SetupMockAPI(mockAPI) + + lines := runComplete(t, "apply", "evil") + + assert.Empty(t, candidateLines(lines)) + // Guard against a vacuous pass: the hub must actually have been queried, + // proving the empty result came from filtering, not from a dead network path. + assert.GreaterOrEqual(t, mockAPI.Requests("/api/themes"), 1) +} + +func TestCompletion_MalformedLocalCacheEntries_Filtered(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + env.CreateThemeFile("good", "mytheme", "1.0", testutil.SampleTOML()) + // The cache is a plain directory a synced dotfiles checkout or an + // extracted tarball can write to, so it gets the same treatment as an + // untrusted hub response: names outside the identifier character class + // must never be suggested - an ANSI escape especially, since candidates + // are printed straight into the user's terminal. + hostile := []string{".git", "my author"} + if runtime.GOOS != "windows" { + // Windows rejects control characters in filenames outright, so the + // escape-sequence case can only be set up (and can only arise) on a + // Unix filesystem. + hostile = append(hostile, "evil\x1b[31muser") + } + for _, name := range hostile { + require.NoError(t, os.MkdirAll(filepath.Join(env.StellarDir, name), 0o755)) + } + + lines := runComplete(t, "apply", "") + + assert.Equal(t, []string{"good/\tlocal"}, candidateLines(lines)) +} + +func TestCompletion_MalformedLocalSlugsAndVersions_Filtered(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + require.NoError(t, os.MkdirAll(filepath.Join(env.StellarDir, "alice", "bad slug"), 0o755)) + // A theme directory can hold any *.toml name; only versions apply can + // actually parse ("1.2", "latest") may be suggested. + themeDir := filepath.Join(env.StellarDir, "alice", "rainbow") + for _, name := range []string{"1.0.1.toml", "notes.toml", "latest.toml"} { + require.NoError(t, os.WriteFile(filepath.Join(themeDir, name), []byte(testutil.SampleTOML()), 0o644)) + } + + lines := runComplete(t, "apply", "alice/") + assert.Equal(t, []string{"alice/rainbow\tlocal"}, candidateLines(lines)) + + lines = runComplete(t, "apply", "alice/rainbow@") + assert.Equal(t, []string{"alice/rainbow@1.0\tlocal", "alice/rainbow@latest\tlocal"}, candidateLines(lines)) +} + +func TestCompletion_NoArgCommands_NoFileCompletion(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + + // Without a ValidArgsFunction these would return ShellCompDirectiveDefault + // (":0") and the shell would offer the user's filenames instead. + // ("version" is wired up the same way but registered on the package-level + // rootCmd rather than in NewRootCmd, so it isn't reachable from here.) + for _, name := range []string{"list", "clean", "current", "rollback", "update"} { + lines := runComplete(t, name, "") + assert.Equal(t, ":4", directiveLine(lines), "command %q should suppress file completion", name) + assert.Empty(t, candidateLines(lines), "command %q should offer no candidates", name) + } +} + +func TestCompletion_HangingHub_BoundedByTimeout(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + // The 2s cap in api.NewCompletionClient is the property the whole feature + // rests on: TAB must never hang on a slow hub. A refused connection (see + // TestCompletion_Offline_DegradesToLocalOnly) fails instantly and so + // doesn't exercise it - this server accepts and then never answers. + released := make(chan struct{}) + server := httptest.NewServer( + http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + <-released + }), + ) + // Cleanups run LIFO, so this releases the blocked handler *before* + // server.Close() starts waiting for it - registering them the other way + // round deadlocks the test. + t.Cleanup(server.Close) + t.Cleanup(func() { close(released) }) + t.Setenv("STELLAR_API_URL", server.URL) + + env.CreateThemeFile("local", "mytheme", "1.0", testutil.SampleTOML()) + + start := time.Now() + lines := runComplete(t, "apply", "unknown-author") + elapsed := time.Since(start) + + assert.Less(t, elapsed, 10*time.Second, "completion must not wait on a hanging hub") + // Local candidates survive the failed lookup, and nothing leaks onto + // stdout - stray output there corrupts what the shell parses. + assert.Empty(t, candidateLines(lines)) + assert.Equal(t, ":6", directiveLine(lines)) +} + +func TestCompletion_AllIdentifierCommands_CompleteThemes(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + + // apply, preview and info all take one identifier; remove takes several. + // Each needs its own ValidArgsFunction, and only apply/remove were + // covered before. + for _, name := range []string{"apply", "preview", "info", "remove"} { + lines := runComplete(t, name, "ali") + assert.Equal(t, []string{"alice/\tlocal"}, candidateLines(lines), + "command %q should complete theme identifiers", name) + } +} diff --git a/cmd/e2e_test.go b/cmd/e2e_test.go index 5dbee28..f15288b 100644 --- a/cmd/e2e_test.go +++ b/cmd/e2e_test.go @@ -402,7 +402,9 @@ func TestE2E_Apply(t *testing.T) { configContent := env.ReadFile(filepath.Join(env.StellarDir, "config.json")) assert.Contains(t, configContent, "local/mytheme@1.0") - assert.Contains(t, configContent, themePath) + // The config is JSON, so a Windows path is stored with its separators + // escaped - compare against the encoded form, not the raw path. + assert.Contains(t, configContent, strings.ReplaceAll(themePath, `\`, `\\`)) assert.Contains(t, configContent, "applied_hash", "config should record the hash of the applied theme") }) @@ -1074,7 +1076,9 @@ func TestE2E_Rollback(t *testing.T) { currentPath := env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) require.NoError(t, os.Symlink(currentPath, env.StarshipPath)) - previousPath := env.StellarDir + "/testuser/sample-theme/1.2.toml" + // filepath.Join, not "/": this is compared against the symlink target + // stellar itself writes, which uses the platform separator. + previousPath := filepath.Join(env.StellarDir, "testuser", "sample-theme", "1.2.toml") config := `{ "current_theme": "alice/rainbow@1.0", diff --git a/cmd/root.go b/cmd/root.go index 4d821db..6728b18 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -19,6 +19,14 @@ func NewRootCmd() *cobra.Command { Short: "Starship theme manager", Long: `Stellar - Discover, preview, and apply Starship themes from the community`, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + // Shell completion runs this on every keystroke, so it must not + // touch the filesystem - and must not fail: an error here prints + // no directive line at all, which every shell reads as "offer + // filenames", silently breaking completion (e.g. on a read-only + // HOME) rather than degrading to no candidates. + if isCompletionRequest(cmd) { + return nil + } // Best-effort cleanup of files left behind by a previous self-update cleanupUpdateLeftovers() // Initialize stellar directory structure before any command runs @@ -42,6 +50,19 @@ func NewRootCmd() *cobra.Command { return cmd } +// isCompletionRequest reports whether cmd is (or sits under) one of cobra's +// hidden completion-request commands, i.e. whether this process was spawned by +// the user pressing TAB rather than running stellar themselves. +func isCompletionRequest(cmd *cobra.Command) bool { + for c := cmd; c != nil; c = c.Parent() { + if c.Name() == cobra.ShellCompRequestCmd || + c.Name() == cobra.ShellCompNoDescRequestCmd { + return true + } + } + return false +} + func Execute() error { return rootCmd.Execute() } diff --git a/cmd/update_test.go b/cmd/update_test.go index 72839e2..6f16be2 100644 --- a/cmd/update_test.go +++ b/cmd/update_test.go @@ -172,7 +172,19 @@ func TestCleanupUpdateLeftovers_RemovesRealArtifacts(t *testing.T) { oldBinary := execPath + ".old" tmpUpdate := filepath.Join(filepath.Dir(execPath), ".stellar-update-test123") - require.NoError(t, os.WriteFile(oldBinary, []byte("old binary"), 0644)) + // Setting up next to the *running* binary is inherently at the mercy of + // the platform. On Windows this fails with a sharing violation ("the + // process cannot access the file because it is being used by another + // process"): every command the E2E tests execute runs + // cleanupUpdateLeftovers via PersistentPreRunE, and a delete Windows still + // has pending blocks re-creating the same name. That's an artefact of + // testing against the live executable, not a defect in the cleanup being + // tested, so treat it as "can't run here" rather than a failure. The + // removal logic itself is covered against a temp dir elsewhere in this + // file. + if err := os.WriteFile(oldBinary, []byte("old binary"), 0644); err != nil { + t.Skipf("cannot stage a leftover next to the running test binary: %v", err) + } require.NoError(t, os.WriteFile(tmpUpdate, []byte("partial download"), 0644)) ageFile(t, tmpUpdate) t.Cleanup(func() { diff --git a/internal/api/client.go b/internal/api/client.go index 2048a90..b69f724 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "net/url" "time" "github.com/a3chron/stellar/internal/paths" @@ -17,23 +18,35 @@ type Client struct { httpClient *http.Client } -func NewClient() *Client { +// newClient builds a Client pointed at baseURL with the given request +// timeout. It's the single place that assembles the http.Client so +// NewClient, NewClientWithURL and NewCompletionClient can't drift apart on +// anything but the two knobs that actually differ between them. +func newClient(baseURL string, timeout time.Duration) *Client { return &Client{ - baseURL: paths.APIURL(BaseURL), + baseURL: baseURL, httpClient: &http.Client{ - Timeout: 30 * time.Second, + Timeout: timeout, }, } } +func NewClient() *Client { + return newClient(paths.APIURL(BaseURL), 30*time.Second) +} + // NewClientWithURL creates a client with a specific base URL (for testing) func NewClientWithURL(baseURL string) *Client { - return &Client{ - baseURL: baseURL, - httpClient: &http.Client{ - Timeout: 30 * time.Second, - }, - } + return newClient(baseURL, 30*time.Second) +} + +// NewCompletionClient creates a client tuned for shell completion requests. +// Shell completion runs synchronously on every keystroke in the user's +// shell, so it must never block waiting on a slow or unreachable +// stellar-hub: callers are expected to treat any error from this client as +// "degrade to local-only completions" rather than surfacing it. +func NewCompletionClient() *Client { + return newClient(paths.APIURL(BaseURL), 2*time.Second) } // Author info nested in theme response @@ -67,6 +80,16 @@ type VersionInfo struct { CreatedAt string `json:"createdAt"` } +// ThemeSummary is the lightweight theme shape returned by GET /api/themes, +// used by shell completion (internal/completion) to look up hub authors and +// theme slugs without downloading a full ThemeInfo per candidate. +type ThemeSummary struct { + Author AuthorInfo `json:"author"` + Name string `json:"name"` + Slug string `json:"slug"` + LatestVersion string `json:"latestVersion"` +} + func (c *Client) FetchThemeConfig(author, name, version string) (string, error) { url := fmt.Sprintf("%s/api/%s/%s/%s", c.baseURL, author, name, version) @@ -116,6 +139,34 @@ func (c *Client) GetThemeInfo(author, name string) (*ThemeInfo, error) { return &info, nil } +// SearchThemesByAuthorName queries GET /api/themes for themes whose author +// name matches authorName as a prefix (server-side match), used by shell +// completion to suggest hub authors/themes that aren't in the local cache. +func (c *Client) SearchThemesByAuthorName(authorName string) ([]ThemeSummary, error) { + reqURL := fmt.Sprintf("%s/api/themes?authorName=%s&limit=100&sort=name", c.baseURL, url.QueryEscape(authorName)) + + resp, err := c.httpClient.Get(reqURL) + if err != nil { + return nil, fmt.Errorf("failed to search themes: %w", err) + } + defer func() { + _ = resp.Body.Close() + }() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("server returned %d", resp.StatusCode) + } + + var result struct { + Themes []ThemeSummary `json:"themes"` + } + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil, err + } + + return result.Themes, nil +} + func (c *Client) IncrementDownloadCount(author, name string) error { url := fmt.Sprintf("%s/api/%s/%s", c.baseURL, author, name) diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 2849dd9..9bd0b23 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -3,7 +3,9 @@ package api import ( "net/http" "net/http/httptest" + "net/url" "testing" + "time" "github.com/a3chron/stellar/internal/testutil" "github.com/stretchr/testify/assert" @@ -184,6 +186,52 @@ func TestClient_StatusCodes(t *testing.T) { } } +func TestNewCompletionClient(t *testing.T) { + client := NewCompletionClient() + assert.Equal(t, BaseURL, client.baseURL) + assert.Equal(t, 2*time.Second, client.httpClient.Timeout) +} + +func TestClient_SearchThemesByAuthorName(t *testing.T) { + var gotQuery url.Values + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.Query() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"themes":[ + {"author":{"id":"a1","name":"alice","image":null},"name":"Rainbow","slug":"rainbow","latestVersion":"1.2"}, + {"author":{"id":"a1","name":"alice","image":null},"name":"Sunset","slug":"sunset","latestVersion":"2.0"} + ]}`)) + })) + defer server.Close() + + client := NewClientWithURL(server.URL) + + summaries, err := client.SearchThemesByAuthorName("ali") + require.NoError(t, err) + require.Len(t, summaries, 2) + + assert.Equal(t, "ali", gotQuery.Get("authorName")) + assert.Equal(t, "100", gotQuery.Get("limit")) + assert.Equal(t, "name", gotQuery.Get("sort")) + + assert.Equal(t, "alice", summaries[0].Author.Name) + assert.Equal(t, "rainbow", summaries[0].Slug) + assert.Equal(t, "1.2", summaries[0].LatestVersion) + assert.Equal(t, "sunset", summaries[1].Slug) +} + +func TestClient_SearchThemesByAuthorName_NonOK(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + client := NewClientWithURL(server.URL) + + _, err := client.SearchThemesByAuthorName("ali") + assert.Error(t, err) +} + func TestClientWithEnvOverride(t *testing.T) { env := testutil.SetupTestEnv(t) mockAPI := testutil.CreateDefaultMockAPI() diff --git a/internal/cache/manager.go b/internal/cache/manager.go index c44c1fc..c671963 100644 --- a/internal/cache/manager.go +++ b/internal/cache/manager.go @@ -5,6 +5,7 @@ import ( "log" "os" "path/filepath" + "sort" "strings" "github.com/a3chron/stellar/internal/paths" @@ -164,6 +165,101 @@ func CleanCache(excludeCurrentPath string) error { return nil } +// ListAuthors returns the names of author directories under the stellar +// cache (e.g. ["alice", "bob"]), sorted, skipping non-directories and +// config.json. Returns (nil, nil) if the stellar home directory doesn't +// exist yet: shell completion (internal/completion) calls this on every +// keystroke and cobra doesn't guarantee the directory has been created by +// the time "__complete" runs, so a missing cache must read as "no authors" +// rather than an error. +func ListAuthors() ([]string, error) { + cacheDir, err := paths.StellarHome() + if err != nil { + return nil, err + } + + entries, err := os.ReadDir(cacheDir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + + var authors []string + for _, e := range entries { + if !e.IsDir() || e.Name() == "config.json" { + continue + } + authors = append(authors, e.Name()) + } + + sort.Strings(authors) + return authors, nil +} + +// ListAuthorThemes returns the theme slug directories cached under author +// (e.g. ["rainbow", "sunset"]), sorted. Returns (nil, nil) if the author +// directory doesn't exist (see ListAuthors for why that's not an error). +func ListAuthorThemes(author string) ([]string, error) { + cacheDir, err := paths.StellarHome() + if err != nil { + return nil, err + } + + entries, err := os.ReadDir(filepath.Join(cacheDir, author)) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + + var slugs []string + for _, e := range entries { + if !e.IsDir() { + continue + } + slugs = append(slugs, e.Name()) + } + + sort.Strings(slugs) + return slugs, nil +} + +// ListThemeVersions returns the cached version strings for author/name, +// newest first (theme.CompareSemver, descending), e.g. ["1.2", "1.1", "1.0"]. +// Returns (nil, nil) if the theme directory doesn't exist (see ListAuthors +// for why that's not an error). +func ListThemeVersions(author, name string) ([]string, error) { + cacheDir, err := paths.StellarHome() + if err != nil { + return nil, err + } + + entries, err := os.ReadDir(filepath.Join(cacheDir, author, name)) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + + var versions []string + for _, e := range entries { + if e.IsDir() || filepath.Ext(e.Name()) != ".toml" { + continue + } + versions = append(versions, strings.TrimSuffix(e.Name(), ".toml")) + } + + sort.Slice(versions, func(i, j int) bool { + return theme.CompareSemver(versions[i], versions[j]) > 0 + }) + + return versions, nil +} + func TmpCachePath(t *theme.Theme) string { return paths.TmpThemePath(t.Author, t.Name, t.Version) } diff --git a/internal/cache/manager_test.go b/internal/cache/manager_test.go index e56ac7f..0e8b839 100644 --- a/internal/cache/manager_test.go +++ b/internal/cache/manager_test.go @@ -1,6 +1,7 @@ package cache import ( + "os" "path/filepath" "testing" @@ -73,6 +74,65 @@ func TestListCachedThemes_IgnoresConfigJSON(t *testing.T) { assert.Equal(t, "alice/rainbow@1.0", themes[0]) } +func TestListAuthors(t *testing.T) { + env := testutil.SetupTestEnv(t) + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("bob", "sunset", "2.0", testutil.SampleTOML()) + env.CreateConfig(`{"current_theme": ""}`) + + authors, err := ListAuthors() + require.NoError(t, err) + + assert.Equal(t, []string{"alice", "bob"}, authors) +} + +func TestListAuthors_MissingDir(t *testing.T) { + env := testutil.SetupTestEnv(t) + require.NoError(t, os.RemoveAll(env.StellarDir)) + + authors, err := ListAuthors() + require.NoError(t, err) + assert.Nil(t, authors) +} + +func TestListAuthorThemes(t *testing.T) { + env := testutil.SetupTestEnv(t) + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("alice", "sunset", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("bob", "other", "1.0", testutil.SampleTOML()) + + themes, err := ListAuthorThemes("alice") + require.NoError(t, err) + assert.Equal(t, []string{"rainbow", "sunset"}, themes) +} + +func TestListAuthorThemes_MissingAuthor(t *testing.T) { + testutil.SetupTestEnv(t) + + themes, err := ListAuthorThemes("nobody") + require.NoError(t, err) + assert.Nil(t, themes) +} + +func TestListThemeVersions(t *testing.T) { + env := testutil.SetupTestEnv(t) + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("alice", "rainbow", "1.5", testutil.SampleTOML()) + env.CreateThemeFile("alice", "rainbow", "1.10", testutil.SampleTOML()) + + versions, err := ListThemeVersions("alice", "rainbow") + require.NoError(t, err) + assert.Equal(t, []string{"1.10", "1.5", "1.0"}, versions) +} + +func TestListThemeVersions_MissingTheme(t *testing.T) { + testutil.SetupTestEnv(t) + + versions, err := ListThemeVersions("alice", "nonexistent") + require.NoError(t, err) + assert.Nil(t, versions) +} + func TestTmpCacheFunctions(t *testing.T) { env := testutil.SetupTestEnv(t) diff --git a/internal/completion/completion.go b/internal/completion/completion.go new file mode 100644 index 0000000..c1e4bed --- /dev/null +++ b/internal/completion/completion.go @@ -0,0 +1,333 @@ +// Package completion implements shell tab-completion for stellar theme +// identifiers ("author/slug@version"), shared by the commands that accept +// one (apply, preview, info) or several (remove) of them. +package completion + +import ( + "fmt" + "strings" + + "github.com/a3chron/stellar/internal/api" + "github.com/a3chron/stellar/internal/cache" + "github.com/a3chron/stellar/internal/theme" + "github.com/spf13/cobra" +) + +// Mode controls whether ThemeIdentifier is allowed to reach out to the +// stellar-hub API in addition to the local cache. +type Mode int + +const ( + // LocalOnly restricts completion to the local cache (~/.config/stellar). + // This is the default for every command: remote lookups (even with a 2s + // cap) make TAB feel broken, and `stellar remove` only ever operates on + // cached themes anyway. + LocalOnly Mode = iota + // LocalAndRemote additionally queries the stellar-hub API when the local + // cache alone doesn't have enough to complete usefully. Opt-in via the + // EnvOnline environment variable. + LocalAndRemote +) + +// EnvOnline opts apply/preview/info completion in to hub suggestions +// ("1" or "true"). Off by default: completion must never feel slow. +const EnvOnline = "STELLAR_COMPLETION_ONLINE" + +const ( + descLocal = "local" + descHub = "hub" +) + +// ThemeIdentifier completes an "author/slug@version" identifier in three +// stages, splitting on the first "/" and then the first "@": +// +// - Stage A (no "/" yet): author names, emitted as "author/". +// - Stage B ("author/" typed, no "@" yet): slugs for that author, emitted +// as "author/slug". +// - Stage C ("@" typed): versions for that author/slug, emitted as +// "author/slug@version". +// +// It never blocks on the network for long: any remote lookup goes through +// api.NewCompletionClient (2s timeout), and any error from it degrades +// silently to whatever local results were already gathered - logged only via +// cobra.CompDebugln, since stray text on stdout would corrupt what the +// user's shell parses as completion candidates. +// +// It also tolerates a local cache that doesn't exist yet: every cache +// listing helper it calls returns (nil, nil) rather than an error for a +// missing directory, so completion always degrades to "no candidates" +// instead of failing. +func ThemeIdentifier(toComplete string, mode Mode) ([]string, cobra.ShellCompDirective) { + slashIdx := strings.IndexByte(toComplete, '/') + if slashIdx == -1 { + return completeAuthor(toComplete, mode) + } + + author := toComplete[:slashIdx] + rest := toComplete[slashIdx+1:] + + // An empty author ("/x") can never form a valid identifier, and letting + // it through would list the cache root's author dirs as slugs of "". + if !theme.IsValidSegment(author) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + atIdx := strings.IndexByte(rest, '@') + if atIdx == -1 { + return completeSlug(author, rest, mode) + } + + slug := rest[:atIdx] + versionPrefix := rest[atIdx+1:] + + // Same for an empty slug ("author/@"): nothing valid can complete it. + if !theme.IsValidSegment(slug) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + return completeVersion(author, slug, versionPrefix, mode) +} + +// completeAuthor implements Stage A: local author directories first; if (and +// only if) toComplete is non-empty and none of them match, fall back to +// querying the hub for authors by prefix. +func completeAuthor(toComplete string, mode Mode) ([]string, cobra.ShellCompDirective) { + directive := cobra.ShellCompDirectiveNoFileComp | cobra.ShellCompDirectiveNoSpace + + if !validSegment(toComplete) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + authors, _ := cache.ListAuthors() + + var candidates []string + for _, a := range authors { + // The cache is a plain directory: it can hold anything a synced + // dotfiles checkout or an extracted tarball put there, so its entries + // get the same grammar check as the hub's (see below). + if !theme.IsValidSegment(a) { + continue + } + if strings.HasPrefix(a, toComplete) { + candidates = append(candidates, withDesc(a+"/", descLocal)) + } + } + + // Empty input never triggers a network call: it's a legitimate case + // (user just typed the bare command) and local-only is a good enough + // answer. Same if a local author already matched, or the caller is + // LocalOnly. + if toComplete == "" || len(candidates) > 0 || mode == LocalOnly { + return candidates, directive + } + + client := api.NewCompletionClient() + summaries, err := client.SearchThemesByAuthorName(toComplete) + if err != nil { + cobra.CompDebugln(err.Error(), false) + return candidates, directive + } + + seen := make(map[string]bool) + for _, s := range summaries { + // The hub response is untrusted input: never emit a name that + // couldn't have come from ParseIdentifier's character class, or a + // hostile hub could inject control sequences into the user's shell. + if !theme.IsValidSegment(s.Author.Name) { + continue + } + // The hub matches authorName case-insensitively, but every shell + // filters candidates against the typed word itself - bash's compgen + // and zsh's compadd case-sensitively. A candidate that doesn't + // prefix-match what the user typed is silently dropped there, so + // emitting one would make TAB work in fish and PowerShell only. + // Filtering here keeps behaviour identical across shells (and doesn't + // rely on the server having honoured authorName at all). + if !strings.HasPrefix(s.Author.Name, toComplete) { + continue + } + if seen[s.Author.Name] { + continue + } + seen[s.Author.Name] = true + candidates = append(candidates, withDesc(s.Author.Name+"/", descHub)) + } + + return candidates, directive +} + +// completeSlug implements Stage B: local slugs for author first, then (in +// LocalAndRemote mode) that author's hub themes appended, deduplicated on +// slug. Every candidate keeps the author exactly as the user typed it: the +// hub's /api/{author}/{slug} routes resolve authors with an exact match, so a +// hub theme is only suggested when the typed casing is the hub's casing - +// otherwise the completion would either 404 on apply or (see completeAuthor) +// be dropped by the shell's own prefix filter anyway. +func completeSlug(author, slugPrefix string, mode Mode) ([]string, cobra.ShellCompDirective) { + directive := cobra.ShellCompDirectiveNoFileComp | cobra.ShellCompDirectiveKeepOrder + + if !validSegment(slugPrefix) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + localSlugs, _ := cache.ListAuthorThemes(author) + + seen := make(map[string]bool) + var candidates []string + for _, slug := range localSlugs { + // Cache entries get the same grammar check as hub ones - a stray + // directory must not become a suggestion (see completeAuthor). + if !theme.IsValidSegment(slug) { + continue + } + if !strings.HasPrefix(slug, slugPrefix) { + continue + } + seen[slug] = true + candidates = append(candidates, withDesc(author+"/"+slug, descLocal)) + } + + if mode == LocalOnly { + return candidates, directive + } + + client := api.NewCompletionClient() + summaries, err := client.SearchThemesByAuthorName(author) + if err != nil { + cobra.CompDebugln(err.Error(), false) + return candidates, directive + } + + for _, s := range summaries { + // Exact, not EqualFold: the hub resolves authors exactly, and the + // user's shell filters candidates against the typed word. + if s.Author.Name != author { + continue + } + // Untrusted hub response: only emit values matching the identifier + // character class (see completeAuthor). + if !theme.IsValidSegment(s.Slug) { + continue + } + if seen[s.Slug] || !strings.HasPrefix(s.Slug, slugPrefix) { + continue + } + seen[s.Slug] = true + candidates = append(candidates, withDesc(s.Author.Name+"/"+s.Slug, descHub)) + } + + return candidates, directive +} + +// completeVersion implements Stage C: local versions newest-first, then (in +// LocalAndRemote mode) remote versions from GetThemeInfo not already +// present, then the "latest" keyword last. Remote is skipped entirely for +// the reserved backup theme (theme.BackupThemeName): it's never published on +// the hub, so looking it up there would only cost a doomed round trip. +func completeVersion(author, slug, versionPrefix string, mode Mode) ([]string, cobra.ShellCompDirective) { + directive := cobra.ShellCompDirectiveNoFileComp | cobra.ShellCompDirectiveKeepOrder + + if !validVersionSegment(versionPrefix) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + // The parser accepts an optional "v" before the version ("@v1.0"), but + // versions are stored without it. Strip it for matching and re-prepend + // it on emission so the shell's own prefix filter keeps the candidates. + // No prefix of "latest" starts with "v", so a leading "v" is unambiguous. + emitV := strings.HasPrefix(versionPrefix, "v") + if emitV { + versionPrefix = versionPrefix[1:] + } + emit := func(version string) string { + if emitV { + version = "v" + version + } + return identifierAt(author, slug, version) + } + // suggestable gates every candidate, local or remote, on the grammar + // apply will later hold it to: "@1.0.1" or "@notes" would only complete + // to an "invalid theme identifier" error. "@vlatest" parses but is silly. + suggestable := func(version string) bool { + if !theme.IsValidVersion(version) { + return false + } + if emitV && version == "latest" { + return false + } + return strings.HasPrefix(version, versionPrefix) + } + + localVersions, _ := cache.ListThemeVersions(author, slug) // already newest-first + + seen := make(map[string]bool) + var candidates []string + for _, v := range localVersions { + // A theme directory can hold any *.toml name ("1.0.1.toml", + // "notes.toml", an editor backup), and ListThemeVersions reports the + // filename verbatim - hence the same gate the hub values get. + if !suggestable(v) { + continue + } + seen[v] = true + candidates = append(candidates, withDesc(emit(v), descLocal)) + } + + if mode == LocalAndRemote && slug != theme.BackupThemeName { + client := api.NewCompletionClient() + info, err := client.GetThemeInfo(author, slug) + if err != nil { + cobra.CompDebugln(err.Error(), false) + } else { + for _, v := range info.Versions { + // Untrusted hub response (see completeAuthor). + if !suggestable(v.Version) { + continue + } + if seen[v.Version] { + continue + } + seen[v.Version] = true + candidates = append(candidates, withDesc(emit(v.Version), descHub)) + } + } + } + + if !seen["latest"] && suggestable("latest") { + candidates = append(candidates, identifierAt(author, slug, "latest")) + } + + return candidates, directive +} + +func identifierAt(author, slug, version string) string { + return fmt.Sprintf("%s/%s@%s", author, slug, version) +} + +func withDesc(value, desc string) string { + return value + "\t" + desc +} + +// validSegment reports whether s is a viable *partial* author or slug - i.e. +// what the user has typed so far, so the empty string qualifies. Candidates +// about to be emitted are held to the stricter theme.IsValidSegment instead. +func validSegment(s string) bool { + for _, r := range s { + if !theme.IsValidIdentifierRune(r) { + return false + } + } + return true +} + +// validVersionSegment is like validSegment but additionally allows '.', since +// a partially typed version looks like "1", "1." or "lat". Emitted versions +// are held to theme.IsValidVersion instead. +func validVersionSegment(s string) bool { + for _, r := range s { + if r != '.' && !theme.IsValidIdentifierRune(r) { + return false + } + } + return true +} diff --git a/internal/config/config.go b/internal/config/config.go index 76134a7..2007685 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,8 +9,8 @@ import ( ) type Config struct { - CurrentTheme string `json:"current_theme"` // "alice/rainbow@1.2" - CurrentPath string `json:"current_path"` // Full path to .toml + CurrentTheme string `json:"current_theme"` // "alice/rainbow@1.2" + CurrentPath string `json:"current_path"` // Full path to .toml PreviousTheme string `json:"previous_theme,omitempty"` PreviousPath string `json:"previous_path,omitempty"` DownloadedThemes []string `json:"downloaded_themes,omitempty"` // ["alice/rainbow", "bob/sunset"] diff --git a/internal/testutil/mockapi.go b/internal/testutil/mockapi.go index d00c79f..d79a132 100644 --- a/internal/testutil/mockapi.go +++ b/internal/testutil/mockapi.go @@ -3,6 +3,8 @@ package testutil import ( "encoding/json" "net/http" + "sort" + "strconv" "strings" "sync" ) @@ -24,11 +26,11 @@ type MockTheme struct { // MockVersion represents a theme version type MockVersion struct { - Version string + Version string ConfigContent string // The actual TOML content - VersionNotes string - Dependencies []string - CreatedAt string + VersionNotes string + Dependencies []string + CreatedAt string } // MockAPIHandler implements http.Handler for testing @@ -36,6 +38,7 @@ type MockAPIHandler struct { mu sync.Mutex themes map[string]*MockTheme // key: "author/slug" DownloadCounts map[string]int // Track download increments for verification + RequestCounts map[string]int // Track requests received, keyed by r.URL.Path } // NewMockAPIHandler creates a new mock API handler @@ -43,6 +46,7 @@ func NewMockAPIHandler() *MockAPIHandler { return &MockAPIHandler{ themes: make(map[string]*MockTheme), DownloadCounts: make(map[string]int), + RequestCounts: make(map[string]int), } } @@ -58,10 +62,19 @@ func (h *MockAPIHandler) AddTheme(theme MockTheme) { // ServeHTTP implements http.Handler func (h *MockAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + h.mu.Lock() + h.RequestCounts[r.URL.Path]++ + h.mu.Unlock() + // Parse path: /api/{author}/{slug} or /api/{author}/{slug}/{version} path := strings.TrimPrefix(r.URL.Path, "/api/") parts := strings.Split(path, "/") + if len(parts) == 1 && parts[0] == "themes" { + h.handleSearchThemes(w, r) + return + } + if len(parts) < 2 { http.Error(w, "Not found", http.StatusNotFound) return @@ -71,8 +84,15 @@ func (h *MockAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { slug := parts[1] key := author + "/" + slug + // Snapshot the theme under the mutex: handleIncrementDownload writes + // theme.Downloads concurrently, so handlers must not read the shared + // struct after unlocking (would race under go test -race). h.mu.Lock() - theme, exists := h.themes[key] + stored, exists := h.themes[key] + var theme MockTheme + if exists { + theme = *stored + } h.mu.Unlock() if !exists { @@ -83,14 +103,14 @@ func (h *MockAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Handle version endpoint: GET /api/{author}/{slug}/{version} if len(parts) == 3 { version := parts[2] - h.handleVersionRequest(w, r, theme, version) + h.handleVersionRequest(w, r, &theme, version) return } // Handle theme endpoint: GET/POST /api/{author}/{slug} switch r.Method { case http.MethodGet: - h.handleGetTheme(w, theme) + h.handleGetTheme(w, &theme) case http.MethodPost: h.handleIncrementDownload(w, key) default: @@ -111,7 +131,7 @@ func (h *MockAPIHandler) handleGetTheme(w http.ResponseWriter, theme *MockTheme) "createdAt": theme.CreatedAt, "updatedAt": theme.UpdatedAt, "author": map[string]interface{}{ - "id": theme.Author, + "id": authorID(theme.Author), "name": theme.Author, "image": nil, "bio": nil, @@ -161,6 +181,108 @@ func (h *MockAPIHandler) handleVersionRequest(w http.ResponseWriter, r *http.Req http.Error(w, "Version not found", http.StatusNotFound) } +// clampQueryInt reads an integer query parameter, falling back to def for +// anything unparseable and clamping the rest into [min, max] - mirroring the +// hub's own handling of limit/offset. +func clampQueryInt(r *http.Request, key string, def, minVal, maxVal int) int { + raw := r.URL.Query().Get(key) + if raw == "" { + return def + } + parsed, err := strconv.Atoi(raw) + if err != nil { + return def + } + return max(minVal, min(parsed, maxVal)) +} + +// authorID derives an opaque author id from an author name. On the real hub +// these are unrelated - the id is a better-auth row id, while every +// /api/{author}/{slug} route resolves the author by exact name - so the mock +// must not let them be used interchangeably, or code that reads the wrong one +// would pass every test here and 404 in production. +func authorID(author string) string { + return "user-" + strings.ToLower(author) +} + +// handleSearchThemes implements GET /api/themes?authorName=, used by +// shell completion (internal/completion) to look up hub authors/themes +// without downloading a full ThemeInfo per candidate. It filters registered +// themes by a case-insensitive prefix match on author name, and orders the +// result by theme name (matching the hub's sort=name, which is what the +// client requests), tie-broken by author so map iteration order can't leak +// into a test. +func (h *MockAPIHandler) handleSearchThemes(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + return + } + + authorPrefix := strings.ToLower(r.URL.Query().Get("authorName")) + + // Copy matching themes under the mutex (see ServeHTTP for why). + h.mu.Lock() + var matches []MockTheme + for _, theme := range h.themes { + if authorPrefix != "" && !strings.HasPrefix(strings.ToLower(theme.Author), authorPrefix) { + continue + } + matches = append(matches, *theme) + } + h.mu.Unlock() + + sort.Slice(matches, func(i, j int) bool { + if matches[i].Name != matches[j].Name { + return matches[i].Name < matches[j].Name + } + return matches[i].Author < matches[j].Author + }) + + themesJSON := make([]map[string]interface{}, len(matches)) + for i, t := range matches { + // The hub omits latestVersion entirely for a theme with no versions + // (JSON.stringify drops an undefined value). + theme := map[string]interface{}{ + "id": t.ID, + "author": map[string]interface{}{ + "id": authorID(t.Author), + "name": t.Author, + "image": nil, + }, + "name": t.Name, + "slug": t.Slug, + "description": t.Description, + "screenshotUrl": "https://example.com/" + t.Slug + ".png", + "downloads": t.Downloads, + "colorScheme": t.ColorScheme, + "createdAt": t.CreatedAt, + "updatedAt": t.UpdatedAt, + } + if len(t.Versions) > 0 { + theme["latestVersion"] = t.Versions[0].Version + } + themesJSON[i] = theme + } + + // Honour limit/offset the way the hub does (default 20, capped at 100), + // so a test can't get a page size the real API could never return. + limit := clampQueryInt(r, "limit", 20, 1, 100) + offset := clampQueryInt(r, "offset", 0, 0, len(themesJSON)) + themesJSON = themesJSON[offset:min(offset+limit, len(themesJSON))] + + response := map[string]interface{}{ + "themes": themesJSON, + "pagination": map[string]interface{}{ + "total": len(themesJSON), + "limit": limit, + "offset": offset, + }, + } + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(response) +} + // handleIncrementDownload increments the download count func (h *MockAPIHandler) handleIncrementDownload(w http.ResponseWriter, key string) { h.mu.Lock() @@ -181,6 +303,26 @@ func (h *MockAPIHandler) GetDownloadCount(author, slug string) int { return h.DownloadCounts[author+"/"+slug] } +// Requests returns how many requests the handler has received for path +// (matched against r.URL.Path exactly, e.g. "/api/testuser/sample-theme"). +func (h *MockAPIHandler) Requests(path string) int { + h.mu.Lock() + defer h.mu.Unlock() + return h.RequestCounts[path] +} + +// TotalRequests returns the total number of requests received across all paths. +func (h *MockAPIHandler) TotalRequests() int { + h.mu.Lock() + defer h.mu.Unlock() + + total := 0 + for _, count := range h.RequestCounts { + total += count + } + return total +} + // CreateDefaultMockAPI creates a mock API with sample themes for testing func CreateDefaultMockAPI() *MockAPIHandler { handler := NewMockAPIHandler() @@ -243,5 +385,35 @@ func CreateDefaultMockAPI() *MockAPIHandler { }, }) + // Add a theme from a second author, used by shell completion tests to + // exercise "unknown author prefix -> hub-only suggestions" behavior. + handler.AddTheme(MockTheme{ + ID: "test-id-3", + Author: "otheruser", + Slug: "ocean-theme", + Name: "Ocean Theme", + Description: "A calming ocean theme", + Downloads: 5, + Group: "nature", + CreatedAt: "2024-02-01T00:00:00Z", + UpdatedAt: "2024-02-05T00:00:00Z", + Versions: []MockVersion{ + { + Version: "2.1", + ConfigContent: SampleTOML(), + VersionNotes: "Latest version", + Dependencies: []string{}, + CreatedAt: "2024-02-05T00:00:00Z", + }, + { + Version: "2.0", + ConfigContent: SampleTOML(), + VersionNotes: "Initial release", + Dependencies: []string{}, + CreatedAt: "2024-02-01T00:00:00Z", + }, + }, + }) + return handler } diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index 3da23b6..0152824 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/a3chron/stellar/internal/paths" @@ -16,11 +17,11 @@ import ( // TestEnv holds the test environment configuration type TestEnv struct { - t *testing.T - RootDir string // Root temp directory - StellarDir string // ~/.config/stellar equivalent + t *testing.T + RootDir string // Root temp directory + StellarDir string // ~/.config/stellar equivalent StarshipPath string // ~/.config/starship.toml equivalent - TmpDir string // /tmp/stellar equivalent + TmpDir string // /tmp/stellar equivalent // Original env values for restoration origEnv map[string]string @@ -117,9 +118,17 @@ func (e *TestEnv) CreateThemeFile(author, name, version, content string) string return themePath } -// CreateConfig creates a config.json file in the test stellar directory +// CreateConfig creates a config.json file in the test stellar directory. +// +// Callers build the JSON by concatenating filesystem paths into it, which is +// invalid JSON on Windows: a path like C:\Users\... contains \U, and encoding/ +// json rejects it as a bad escape ("invalid character 'U' in string escape +// code"). Backslashes are therefore escaped here rather than at all ~30 call +// sites. No test wants a real JSON escape sequence in this content, so this +// is safe - but if one ever does, it needs to pass \\ itself. func (e *TestEnv) CreateConfig(content string) string { configPath := filepath.Join(e.StellarDir, "config.json") + content = strings.ReplaceAll(content, `\`, `\\`) if err := os.WriteFile(configPath, []byte(content), 0644); err != nil { e.t.Fatalf("failed to write config file: %v", err) } diff --git a/internal/theme/parser.go b/internal/theme/parser.go index a00e6a0..f719084 100644 --- a/internal/theme/parser.go +++ b/internal/theme/parser.go @@ -36,6 +36,39 @@ func IsValidIdentifierRune(r rune) bool { r == '_' || r == '-' } +// versionRe matches a complete version as ParseIdentifier accepts it, minus +// the optional "v" prefix (which the parser strips). Kept next to the +// identifier regex below so the two can't drift. +var versionRe = regexp.MustCompile(`^([0-9]+\.[0-9]+|latest)$`) + +// IsValidSegment reports whether s is a complete, usable author or theme +// segment: non-empty and made up entirely of IsValidIdentifierRune runes. +// +// Callers that emit segments they didn't parse themselves (e.g. shell +// completion listing cache directories, or reading an API response) must gate +// on this, so a name that ParseIdentifier would reject is never handed back to +// the user as a suggestion - and so a hostile name can't smuggle control +// characters into a terminal. +func IsValidSegment(s string) bool { + if s == "" { + return false + } + for _, r := range s { + if !IsValidIdentifierRune(r) { + return false + } + } + return true +} + +// IsValidVersion reports whether s is a complete version ParseIdentifier +// accepts ("1.2" or "latest"), without the optional "v" prefix. Same contract +// as IsValidSegment: gate emitted versions on it, since a cache directory can +// hold arbitrary *.toml filenames. +func IsValidVersion(s string) bool { + return versionRe.MatchString(s) +} + // ParseIdentifier parses "alice/rainbow@1.2", "alice/rainbow@latest", or "alice/rainbow" func ParseIdentifier(identifier string) (*Theme, error) { // Normalize: remove leading/trailing whitespace @@ -105,16 +138,16 @@ func FindLatestLocalVersion(themeDir string) (string, error) { // Sort by semver descending, "latest" goes last as fallback sort.Slice(versions, func(i, j int) bool { - return compareSemver(versions[i], versions[j]) > 0 + return CompareSemver(versions[i], versions[j]) > 0 }) return versions[0], nil } -// compareSemver compares two version strings. +// CompareSemver compares two version strings. // Returns >0 if a > b, <0 if a < b, 0 if equal. // Non-numeric versions (like "latest") are sorted to the end. -func compareSemver(a, b string) int { +func CompareSemver(a, b string) int { aMajor, aMinor, aOk := parseSemver(a) bMajor, bMinor, bOk := parseSemver(b) diff --git a/internal/theme/parser_test.go b/internal/theme/parser_test.go index 7b22ff4..e440336 100644 --- a/internal/theme/parser_test.go +++ b/internal/theme/parser_test.go @@ -2,6 +2,7 @@ package theme import ( "os" + "path/filepath" "testing" "github.com/a3chron/stellar/internal/testutil" @@ -11,85 +12,85 @@ import ( func TestParseIdentifier(t *testing.T) { tests := []struct { - name string - identifier string - wantAuthor string - wantName string - wantVersion string + name string + identifier string + wantAuthor string + wantName string + wantVersion string wantExplicit bool - wantErr bool + wantErr bool }{ { - name: "simple identifier", - identifier: "alice/rainbow", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "latest", + name: "simple identifier", + identifier: "alice/rainbow", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { - name: "with version", - identifier: "alice/rainbow@1.2", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "1.2", + name: "with version", + identifier: "alice/rainbow@1.2", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "1.2", wantExplicit: true, - wantErr: false, + wantErr: false, }, { - name: "with v prefix version", - identifier: "alice/rainbow@v1.2", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "1.2", + name: "with v prefix version", + identifier: "alice/rainbow@v1.2", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "1.2", wantExplicit: true, - wantErr: false, + wantErr: false, }, { - name: "with latest version", - identifier: "alice/rainbow@latest", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "latest", + name: "with latest version", + identifier: "alice/rainbow@latest", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "latest", wantExplicit: true, - wantErr: false, + wantErr: false, }, { - name: "with underscores", - identifier: "some_user/my_theme", - wantAuthor: "some_user", - wantName: "my_theme", - wantVersion: "latest", + name: "with underscores", + identifier: "some_user/my_theme", + wantAuthor: "some_user", + wantName: "my_theme", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { - name: "with hyphens", - identifier: "some-user/my-theme", - wantAuthor: "some-user", - wantName: "my-theme", - wantVersion: "latest", + name: "with hyphens", + identifier: "some-user/my-theme", + wantAuthor: "some-user", + wantName: "my-theme", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { - name: "with numbers", - identifier: "user123/theme456", - wantAuthor: "user123", - wantName: "theme456", - wantVersion: "latest", + name: "with numbers", + identifier: "user123/theme456", + wantAuthor: "user123", + wantName: "theme456", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { - name: "with whitespace", - identifier: " alice/rainbow ", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "latest", + name: "with whitespace", + identifier: " alice/rainbow ", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { name: "invalid - no slash", @@ -249,7 +250,7 @@ func TestCompareSemver(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := compareSemver(tt.a, tt.b) + result := CompareSemver(tt.a, tt.b) if tt.expect > 0 { assert.Greater(t, result, 0, "expected %s > %s", tt.a, tt.b) } else if tt.expect < 0 { @@ -337,8 +338,9 @@ func TestTheme_CachePath(t *testing.T) { path, err := theme.CachePath() require.NoError(t, err) - // Should be relative to the test stellar home - expected := env.StellarDir + "/alice/rainbow/1.2.toml" + // Should be relative to the test stellar home. filepath.Join, not "/", so + // the expectation uses the platform's separator like CachePath does. + expected := filepath.Join(env.StellarDir, "alice", "rainbow", "1.2.toml") assert.Equal(t, expected, path) } @@ -354,7 +356,7 @@ func TestTheme_CacheDir(t *testing.T) { dir, err := theme.CacheDir() require.NoError(t, err) - expected := env.StellarDir + "/alice/rainbow" + expected := filepath.Join(env.StellarDir, "alice", "rainbow") assert.Equal(t, expected, dir) } diff --git a/run-tests.sh b/run-tests.sh index a5a3888..da2bfcb 100755 --- a/run-tests.sh +++ b/run-tests.sh @@ -82,11 +82,11 @@ run_go_tests() { case "$action" in pass) echo -e " ${GREEN}PASS${NC} $test" - ((passed++)) + passed=$((passed + 1)) ;; fail) echo -e " ${RED}FAIL${NC} $test" - ((failed++)) + failed=$((failed + 1)) failed_tests+=("$pkg $test") ;; esac