feat: implicit default catalog and shell completion - #4
Merged
Conversation
…time; test real completion behavior
OnlyValidArgs (added in 0791f21) restricts args[0] to shell.Supported() exact-case match, so the prior strings.ToLower was both unreachable for mixed-case input and dead code. Removed along with the strings import.
The EffectiveCatalog wrapper conflated a catalog (a domain entity) with its resolution provenance (a query-time fact). Replace it with a plain []CatalogConfig return from EffectiveCatalogs and a new IsImplicit(cfg, name) query so callers ask the question they actually have rather than plumbing a flag through the catalog value. - internal/config: EffectiveCatalogs now returns []CatalogConfig; add IsImplicit helper; remove EffectiveCatalog wrapper type. - internal/app: drop the .CatalogConfig unwrap when building sources. - cmd/catalog/list: derive the SOURCE column via config.IsImplicit. - cmd/catalog/remove: replace the scan-for-Implicit loop with a single IsImplicit call.
Only 'remove' previously returned an actionable error when invoked against the implicit default catalog. 'reorder' and 'info' both fell through to the generic 'catalog %q not found' path, which gives the user no hint that the catalog exists at runtime but isn't stored in config.yaml. - Add an unexported implicitDefaultError(verb, name) helper in cmd.go and route remove/reorder/info through it (DRY). - Extract reorderCatalogs() so the implicit-default check can run before config.ReorderCatalogs sees the request. - info.go now checks IsImplicit before returning 'not found'. - Tests: reorder_test.go and info_test.go assert the error mentions NEM_NO_DEFAULT_CATALOG and the relevant verb.
Move EnvNoDefaultCatalog reads out of EffectiveCatalogs/IsImplicit and into a one-shot snapshot at Config.Load() time. Both functions are now pure functions of Config, eliminating os.Getenv calls from their bodies and removing the empty-vs-unset ambiguity. - Add unexported Config.noDefaultCatalog field captured at Load(). - Document the contract on EnvNoDefaultCatalog: any non-empty value disables; empty string is treated as unset. - Add isNoDefaultCatalogSet helper alongside the constant. - Add internal test helper ConfigWithNoDefault for tests that need to construct a Config with the snapshot pre-populated without staging a config.yaml. - Drive internal/app/app_test.go opt-out test through config.Load() so the snapshot is captured from the env-set test setup.
…derCatalogs
Move the implicit-default guard from the cmd layer into the config-layer
mutators so both checks happen atomically under the file lock, eliminate
the double Load() in cmd/catalog/{remove,reorder}, and unify the
"reject implicit default" contract behind a typed error.
- Add config.ImplicitDefaultError (Name-carrying) plus ErrImplicitDefault
sentinel for errors.Is callers that don't need the name.
- RemoveCatalog and ReorderCatalogs check IsImplicit inside the Mutate
callback and return ImplicitDefaultError{Name: n}.
- cmd/catalog/{remove,reorder} drop their explicit config.Load() + check
and translate via errors.As(err, &config.ImplicitDefaultError{}).
- info.go keeps its in-place check (read-only path, no mutator).
Catalog-name tab-completion previously read cfg.Catalogs directly, so the implicit default catalog never appeared in completions even when it was usable everywhere else. Switch to config.EffectiveCatalogs so completion stays in sync with what the rest of the CLI sees, and fold the two near-duplicate iteration helpers behind a shared body. - Add unexported completeFromCatalogs body. - CompleteCatalogNames and CompleteCatalogNamesFromConfig become thin wrappers around it. - Add tests covering implicit-default inclusion for both wrappers.
Previously 'nem catalog info default' against the implicit default returned the same actionable error as 'remove'/'reorder'. Since 'info' is purely read-only and the implicit catalog's remote and ref are well-defined, refusing was unhelpful. Now shows the same fields as for explicit catalogs plus a 'Source: implicit' line pointing at the opt-out env var.
trankimtung
changed the title
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.
Summary
vi-dev/nem-catalog@main) synthesized in-memory whenconfig.yamlhas none; opt out withNEM_NO_DEFAULT_CATALOG=1.nem catalog listshowsSOURCEcolumn;infoprints implicit details with opt-out hint;remove/reorderreturn actionable error viaconfig.ImplicitDefaultError.ValidArgs+cobra.OnlyValidArgsonactivate/deactivate; canonical list moved toshell.Supported()with anti-drift test against embeddedscripts/*.sh.Test plan
go test ./... -racegolangci-lint rungo vet ./...NEM_HOME=/tmp/x nem catalog listshows one row,SOURCE=implicitNEM_HOME=/tmp/x nem catalog info defaultprints remote/ref + implicit hintNEM_HOME=/tmp/x nem catalog remove defaulterrors withNEM_NO_DEFAULT_CATALOGhintNEM_HOME=/tmp/x NEM_NO_DEFAULT_CATALOG=1 nem catalog listempty (header only)nem __complete activate ""offersbash,zsh,fishnem activate junkrejected at args validation~/.nem/config.yamlwritten by any of the above