Improve UX and add JSON output#5
Merged
Merged
Conversation
The user experience of having a separate subcommands was suboptimal. Specifically when a user does not know the names of all available program in a collection. It is not easier to first run `stackwhere list /path/to/collection.o` to get the list of programs, and then append the program name to the command to get the stack listing of a specific program. We still keep this behind the `list` subcommand to allow for additional features under different subcommands in the future. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
There was a problem hiding this comment.
Pull request overview
This PR streamlines the CLI UX by consolidating the prior collection/program subcommands into a single list command, and introduces a global --json output mode to make command results script-friendly.
Changes:
- Replace
collectionandprogramsubcommands with a unifiedlist {collection} [program]command. - Add a persistent
--json/-jflag and JSON output support forlistandversion. - Update tests/docs to reflect the new command surface.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates usage examples/docs to the new list command. |
| cmd/stackwhere/main.go | Adds persistent --json flag and registers only list + version. |
| cmd/stackwhere/version.go | Adds JSON output mode for version and structures version info. |
| cmd/stackwhere/list.go | Implements merged list behavior (collection summary vs program detail) and JSON output. |
| cmd/stackwhere/list_test.go | Updates slot-usage test for exported fields; adds stack-usage test. |
| cmd/stackwhere/collection.go | Removes deprecated collection subcommand implementation. |
| cmd/stackwhere/collection_test.go | Removes deprecated collection test (moved/replaced elsewhere). |
Comments suppressed due to low confidence (5)
cmd/stackwhere/list.go:62
--call-stackcurrently has no effect on JSON output:getStackSlotUsagealways populatesCallstack, and the JSON encoder emits it whenever non-empty even if the flag is false. Consider omitting/clearingCallstack(or computing it conditionally) unless--call-stackis enabled to keep JSON and text outputs consistent.
cmd/stackwhere/list.go:71- Non-JSON output is written via
fmt.Printf, which bypasses Cobra's configured output writer. Usecmd.OutOrStdout()(e.g.,fmt.Fprintf(cmd.OutOrStdout(), ...)orcmd.Printf) so output can be redirected/captured consistently (including in tests).
cmd/stackwhere/list.go:249 - Sorting comparator returns only the stack-usage difference, but the comment says it also sorts by name. When two programs have equal
stack_usage,slices.SortFuncmay produce nondeterministic ordering; add a secondary name comparison (and avoid int64 subtraction -> int cast) to ensure a total order.
cmd/stackwhere/list.go:275 - Non-JSON output is printed with
fmt.Printf, which ignores Cobra's output writer. Prefer writing tocmd.OutOrStdout()so users/tests can redirect output consistently.
cmd/stackwhere/list.go:62 - New JSON-output paths aren’t covered by tests. Adding a small test that runs the command with
--jsonand asserts the output is valid JSON (and respects flags like--call-stack) would help prevent regressions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Added global `--json`/`-j` flag and made every command output JSON when the flag is set. The JSON output is pretty-printed for human inspection. This allows stackwhere to be used in scripts and pipelines. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
15e9a00 to
1d976b6
Compare
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.
This PR merges the separate sub commands for collection and program listing into a singular
listcommand for the sake of user experience.This PR also adds JSON outputs to all commands to allow for usage in scripts or for data extraction.