feat: shell completion - #4
Merged
Merged
Conversation
Whitespace only, no behavioural change. These two files were left unformatted by earlier commits and `gofmt -l` flagged them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes `author/slug@version` in three stages (author, slug, version) for apply, preview, info and remove, sourced from the local theme cache so TAB is instant and works offline. Set STELLAR_COMPLETION_ONLINE=1 to additionally query the hub, via a 2s-timeout client that degrades silently to local-only results when the hub is slow or unreachable. Supporting changes: - internal/cache: ListAuthors, ListAuthorThemes and ListThemeVersions, all returning (nil, nil) for a missing directory so completion never fails on a cache that doesn't exist yet. - internal/api: NewCompletionClient (2s instead of 30s) and SearchThemesByAuthorName over GET /api/themes?authorName=. - internal/theme: IsValidSegment and IsValidVersion, derived from the same character class and grammar as ParseIdentifier, so a name that apply would reject is never suggested. Every candidate is gated on them, local ones included: the cache is a plain directory that a synced dotfiles checkout can put anything in, and candidates are printed straight into the user's terminal. - Hub candidates must prefix-match the typed word case-sensitively. bash's compgen and zsh's compadd filter that way themselves, so emitting the hub's canonical casing worked in fish and PowerShell only. - No-argument commands get cobra.NoFileCompletions; without a ValidArgsFunction cobra returns ShellCompDirectiveDefault and the shell offers the user's filenames after e.g. `stellar list`. - The root PersistentPreRunE is skipped for completion requests. It ran a directory cleanup and MkdirAll on every keystroke, and an error there suppresses the directive line entirely - which every shell reads as "offer filenames", silently breaking completion on a read-only HOME. - internal/testutil: mock GET /api/themes, per-path request counters, a distinct opaque author id (the hub's id and name are unrelated, and routes resolve authors by name), limit/offset handling, and a snapshot-under-mutex fix for a data race in the theme handlers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`((passed++))` evaluates to the pre-increment value, so the arithmetic command exits 1 the first time it runs with passed=0 - and under `set -e` that killed the script right after printing the first PASS. The suite itself was fine; only the runner bailed out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The release workflow built and published binaries without ever running the test suite, so a tag was the first thing to find out whether the code worked. goreleaser now needs a test job as well as lint. Linux runs with -race and is blocking. Windows runs too, since it's the platform whose copy-vs-symlink path most needs native coverage, but is marked continue-on-error: the suite has never been run on a Windows runner, and a release is the wrong moment to discover that. Drop that line once it's been green once. Also switched all three jobs from a pinned go-version: 1.24 to go-version-file: go.mod. The module requires 1.25.5, so the pin made every job download a newer toolchain before doing any work, and it can now no longer drift from the module. Contract tests stay out of CI - they're behind the `contract` build tag and hit the live hub. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tests only ran on a tag push, so anything broken on main stayed invisible until release time - which is exactly when you don't want to find out. lint, test and the go.mod checks move into a reusable checks.yaml that both ci.yaml (pull requests, and pushes to main) and release.yaml call. Defining them once means the checks gating a release are by construction the same ones the PR had to pass, rather than two lists that drift. The module checks used to live inside the goreleaser job, where they only ever ran during a release; they now run on every PR as well. A PR from a branch in this repo fires only the pull_request event, since its push doesn't match main, so this doesn't double up. In-flight runs are cancelled when a PR gets a new push. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first native Windows CI run failed in ~25 places. All of it is the tests assuming a Unix filesystem - product code is unaffected, and nothing here changes behaviour for Windows users. - testutil.CreateConfig: call sites build config.json by concatenating a path into a JSON string, and "C:\Users\..." makes that invalid JSON - \U is not a valid escape, so every affected test failed with "invalid character 'U' in string escape code". This was the bulk of the failures (rollback, current, remove, clean). Backslashes are now escaped in the helper rather than at all ~30 call sites; on Unix it's a no-op. Config writing in internal/config already used json.MarshalIndent and was never affected. - e2e apply: asserted the config file contains the raw theme path, but JSON stores it with escaped separators. Compares the encoded form now. - theme parser: two expectations built paths with "/" instead of filepath.Join, so they could only ever match on Unix. - completion: the hostile-cache-entry test staged a directory with an ANSI escape in its name. Windows rejects control characters in filenames outright, so that case is now Unix-only - it's also the only platform where the threat exists. - update: staging a leftover next to the *running* test binary hits a Windows sharing violation, since every executed command runs cleanupUpdateLeftovers and a pending delete blocks re-creating the name. It skips with the reason instead of failing; the removal logic is covered against a temp dir by TestRemoveUpdateLeftovers. Verified on Linux only - the Windows leg is still continue-on-error, so the next run reports without blocking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Last Windows holdout. previousPath was assembled with "/" and then compared against the symlink target stellar writes, which uses the platform separator - so the assertion could only ever match on Unix. Swept the rest of the suite for the same shape: the other occurrences are paths handed to file APIs (Windows accepts "/" there) or fed into config JSON for reading back, and every other assert.Equal against a symlink target already uses a CreateThemeFile path, which is filepath.Join-built. This was the only comparison that broke. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It's been green on a native runner, which is what the advisory continue-on-error was waiting for. Both legs now gate pull requests and releases, so the copy-vs-symlink path can't regress unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.