fix(cli): implement nodeup list and add JSON envelope#80
Merged
Conversation
`nodeup list` was a stub printing "not yet implemented (Phase 1)". Replace with a real implementation mirroring `nodeup check`'s per-manager loop: ResolveAll / per-manager ListInstalled / JSON-or- table rendering. Versions are sorted ascending (semver); the active version (first manager that answers Current()) is surfaced in both the JSON envelope and a trailing line of the table. The new --manager flag narrows the listing to a single manager (case- insensitive) for parity with upgrade/check. Per-manager errors are captured in the JSON envelope instead of aborting the whole listing. Closes #45. Co-Authored-By: puku-ai-2.8 <noreply@puku.sh>
|
✔️ 61400dd - Conventional commits check succeeded. |
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
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
nodeup listwas a stub printing "not yet implemented (Phase 1)". It now does the obvious thing — resolve every detected manager viaDetectAll, call each one'sListInstalled(), and render the union either as a JSON envelope (--json) or a one-line-per-manager table. Versions are sorted ascending (semver — pre-release tags order correctly). The currently-active version, when at least one manager answersCurrent(), is surfaced alongside the listing in both renderers.A new
--managerflag narrows the listing to a single manager (case-insensitive), matching the same flag onnodeup upgradeandnodeup check. Per-manager errors fromListInstalledare captured in the JSON envelope'serrorfield rather than aborting the whole listing, mirroring the soft-fail policy already oncheck.Closes #45.
Linked issues
Type of change
fix— bug fix (PATCH bump) (filling in for the long-broken stub)Checklist
fix(scope): subject)make cilocally and it passes (fmt, vet, test pass locally;make lintfails on the v2→v1 schema mismatch tracked in build(lint): .golangci.yml v1 schema incompatible with golangci-lint v2 (brew installs v2, make lint fails to even parse config) #62 — pre-existing onmain, independent of this fix; CI runs golangci-lint v1.64.8 per.github/workflows/ci.ymland parses the config fine)internal/cli/list_test.go— JSON envelope happy-path + omits-on-nil current/error, table renderer empty + mixed-states,--managerresolution helper incl. case-insensitive,formatListVersions,lower,registryNames)docs/, inline godoc)Scope notes
nodeup list --json(top-level envelope withinstalled: [{manager, versions, error?}]and an optionalcurrent) is intentionally similar to — but not identical to —nodeup check --json.checkreports what nodejs.org offers;listreports what's installed locally. The per-entry field names (manager,versions,error) match so ajqpipeline collecting both commands doesn't have to special-case.Versionsis rendered as strings (semver) rather than numbers because pre-release tags (20.0.0-rc.1, ...) are not valid JSON numbers and would otherwise be silently dropped on the wire.Currentis first manager that answersCurrent()— there is typically one manager-ownednodeon PATH per machine, so this is a best-effort signal rather than a strict contract. Managers that don't implementCurrent(e.g., nvm-windows — returnsErrNVMWindowsNotImplemented) are skipped silently.Currentthrough the table renderer unconditionally: it prints only when probeable. The empty-state line "No Node.js version manager detected." mirrorscheck.go's wording exactly.findManagerByNameis case-insensitive (matches by lowercased names) so--manager FNMworks the same as--manager fnm. The error message lists every detected manager in registration order —registryNameskeeps that ordering consistent with the rest of the CLI.Out of scope (separate issues)
internal/cli/packages.gostill usescontext.Background()at line ~141 forRestore— fix(cli): context.Background() used instead of cmd.Context() in packages.go (contextcheck violation) #49.nodeup packages restoreaborts on first failed install — bug(packages): restore aborts on first failed install; migration report never written #46.Screenshots / output
User-visible behaviour change:
nodeup listprintednodeup list — not yet implemented (Phase 1)and exited 0.nodeup listenumerates installed versions across every detected manager:--manager fnm:--json:{ "installed": [ {"manager": "fnm", "versions": ["20.18.0", "22.11.0"]}, {"manager": "volta", "versions": ["18.20.4"]} ], "current": "22.11.0" }Verification path:
nodeup list,nodeup list --json,nodeup list --manager fnmon a machine with at least one manager installed;go test -race ./internal/cli/...for the unit-test surface.