style lets output#348
Merged
Merged
Conversation
Reviewer's GuideRefactors lets CLI help and error output to use the external Fang renderer with a new theming system, parsing docopt metadata into command annotations to drive richer, subgroup-aware help screens while cleaning up legacy help logic and wiring Fang into the main CLI entrypoint. Sequence diagram for CLI execution with Fang help and error handlingsequenceDiagram
actor User
participant Main as cli.Main
participant RootCmd as cobra.Command_root
participant Fang as fang.Execute
participant HelpR as cmd.HelpRenderer
participant ErrH as cmd.ErrorHandler
User->>Main: run lets command
Main->>RootCmd: newRootCmd(version, buildDate)
Main->>Fang: Execute(ctx, RootCmd, WithColorSchemeFunc(theme.DefaultColorScheme), WithErrorHandler(cmd.ErrorHandler), WithHelpRenderer(cmd.HelpRenderer))
alt command runs normally
Fang->>RootCmd: ExecuteContext(ctx)
RootCmd-->>Fang: nil error
Fang-->>Main: nil
else user requests help (e.g. --help)
Fang->>RootCmd: ExecuteContext(ctx)
RootCmd->>HelpR: Help(ctx.Writer, ctx.Styles, ctx.Width)
HelpR-->>RootCmd: rendered help output
RootCmd-->>Fang: nil error
Fang-->>Main: nil
else command or flag error
Fang->>RootCmd: ExecuteContext(ctx)
RootCmd-->>Fang: error
Fang->>ErrH: ErrorHandler(writer, styles, error)
ErrH-->>Fang: styled error output
Fang-->>Main: error
end
Main-->>User: exit code
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="internal/cmd/subcommand_test.go" line_range="90" />
<code_context>
+ }
+}
+
+func TestSubcommandHelpArg(t *testing.T) {
+ command := &configpkg.Command{
+ Name: "release",
</code_context>
<issue_to_address>
**suggestion (testing):** Consider also testing `-h` in addition to `--help` for the subcommand help path
Since `hasHelpArg` treats both `--help` and `-h` as help triggers, please also cover the `-h` form (either in this test or a small additional one) so both aliases remain protected against regressions.
Suggested implementation:
```golang
Example:
lets release 1.0.0 -m "Release 1.0.0"`,
}
func TestHasHelpArgShortFlag(t *testing.T) {
t.Helper()
if !hasHelpArg([]string{"release", "-h"}) {
t.Fatalf("expected -h to be treated as help arg")
}
}
```
This change assumes `hasHelpArg` is in the same package as `subcommand_test.go` and has the signature `func hasHelpArg(args []string) bool`. If the signature differs (e.g., different parameter type or additional parameters), update the call in `TestHasHelpArgShortFlag` accordingly.
If you prefer to keep all coverage inside `TestSubcommandHelpArg`, you could instead convert that test to table-driven and assert behavior for both `--help` and `-h` arguments using whatever invocation pattern it currently uses.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| } | ||
|
|
||
| func TestSubcommandHelpArg(t *testing.T) { |
There was a problem hiding this comment.
suggestion (testing): Consider also testing -h in addition to --help for the subcommand help path
Since hasHelpArg treats both --help and -h as help triggers, please also cover the -h form (either in this test or a small additional one) so both aliases remain protected against regressions.
Suggested implementation:
Example:
lets release 1.0.0 -m "Release 1.0.0"`,
}
func TestHasHelpArgShortFlag(t *testing.T) {
t.Helper()
if !hasHelpArg([]string{"release", "-h"}) {
t.Fatalf("expected -h to be treated as help arg")
}
}This change assumes hasHelpArg is in the same package as subcommand_test.go and has the signature func hasHelpArg(args []string) bool. If the signature differs (e.g., different parameter type or additional parameters), update the call in TestHasHelpArgShortFlag accordingly.
If you prefer to keep all coverage inside TestSubcommandHelpArg, you could instead convert that test to table-driven and assert behavior for both --help and -h arguments using whatever invocation pattern it currently uses.
kindermax
force-pushed
the
style-lets-output
branch
from
427d586 to
b52cc2b
Compare
- Replace hand-rolled cmpOr with stdlib cmp.Or - Hoist paddedLeft2 style to package var; avoids alloc per help row - Fix renderCommandGroup O(n²) double-scan: partition into map once - Remove no-op JoinHorizontal in renderDocoptFlagPart - Fix misleading capacity hint in renderHelpDescription - Style subgroup names with title color/transform, no top padding
- Drop usageTitle; reuse sectionTitle for "usage" label - Extract subgroupTitleStyle constructor; remove inline style overrides - Inline styleHelpUsageLine into its only caller and delete it - Remove redundant MarginBottom(0) from compactTitleStyle - renderCobraFlag: return directly instead of JoinHorizontal on one string - Inline errorHeader (used once) - Use var-decl nil slices instead of make([]string, 0) where no cap is known - renderHelpDescription: fast-path single-line descriptions - Replace sort.Slice/sort.Strings with slices.SortFunc/slices.Sort
The fang renderer changed the output format significantly from cobra defaults: - Help output starts with a blank line (renderLongShort adds leading newline) - Section titles are uppercase (Title style uses strings.ToUpper) - Descriptions capitalize first word (FlagDescription uses titleFirstWord) - Error output uses styled ERROR header with Margin(1), adding a leading blank line - Error messages capitalize first word via titleFirstWord on ErrorText - No trailing "Use lets help [command]..." footer - No type annotations on flags Update all affected tests to use assert_output --partial instead of exact fixture matching or assert_line --index 0, which were both fragile against the new styled output. Also add fang.WithVersion(rootCmd.Version) to pass the binary version into fang so version output shows "0.0.0-dev" instead of "unknown (built from source)". Publish fang v0.1.0 and remove go.mod replace directive so Docker-based bats tests can resolve the dependency.
Replace bats format assertions with snapshot tests in internal/cmd/ using github.com/charmbracelet/x/exp/golden. Ten golden files cover root help, subcommand help, grouped commands, and error output (unknown command, dependency chain). Bats tests are simplified to behavioural checks only (exit codes, suggestion presence). Format regressions are caught by the golden files instead. Adds --update-golden flag to `lets test-unit` to regenerate snapshots when rendering changes intentionally.
Replace programmatic command construction with config.Load + InitSubCommands so golden tests exercise the full YAML → config → render pipeline. Fixtures are copied from the corresponding bats test directories into testdata/fixtures/ and loaded by path. Error renderer tests stay synthetic since they inject DependencyError directly rather than running shell commands.
Rename errorOutput.println to writeln to avoid forbidigo match on the builtin println pattern. Replace HasPrefix+TrimPrefix pair in buildCommandUse with an unconditional TrimPrefix (staticcheck S1017).
kindermax
force-pushed
the
style-lets-output
branch
from
05502c0 to
06e6037
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
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 by Sourcery
Integrate a new themed help and error rendering system powered by the Fang library and docopt metadata, replacing the existing custom help output while enriching command documentation and visuals.
Enhancements:
Build:
Tests: