fix: sub-commands styling #47
Merged
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR improves sub-command styling in help/man outputs, displays full command paths in examples, and refactors testing to use pointer‐based root commands.
- Replace generic placeholder names in golden files and add new golden outputs for nested subcommands
- Update
styleUsageandstyleExampleto use full paths andUseLine(), and refactorisSubCommand - Simplify version logic in
ExecuteviabuildVersionand convert many tests to usetoMkroothelpers
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/TestSetup/with_subcommands/*.golden | Updated expected help outputs for subcommands and examples |
| testdata/TestSetup/complete/man.golden | Changed manpage title and usage to include man subcommand |
| help.go | Switched to UseLine() for usage, enhanced subcommand styling, updated isSubCommand signature |
| fang.go | Extracted version-building logic into buildVersion and wire-up in Execute |
| fang_test.go | Refactored tests to use toMkroot wrappers and Go 1.24’s T.Context() |
| example/main.go | Added examples for nested subcommands |
Comments suppressed due to low confidence (2)
fang.go:147
- The new
buildVersionfunction has branching logic for default and VCS-based versions; consider adding unit tests to cover both code paths.
func buildVersion(opts settings) string {
help.go:288
cleanArgscurrently accumulates all tokens including flags and quoted strings, which can break command traversal; consider appending only actual subcommand names to the path.
cleanArgs = append(cleanArgs, arg)
meowgorithm
approved these changes
Jun 26, 2025
|
Hi @caarlos0 As Go uses the N-1 support matrix typically it would be great to stay on 1.23 until 1.25 is GA and have other libs to not be forced to jump on latest. |
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 properly handles examples to sub commands, as well as show the full path in places it probably should.
on top of that, it improves testing, which wasn't working properly due to root cmd not being a pointer.
Looks like this:
closes #45
closes #44