Conversation
📝 WalkthroughWalkthroughThis PR bumps versions to v1.8.10, consolidates many per-command argument validators into a generic Validator/ValidateArgs system, shifts some flag-parse timing, and changes card list/set commands to store API errors (and render them) instead of quitting. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@cmd/berry/berry.go`:
- Line 40: The validator in the call to utils.ValidateArgs is using the wrong
command name string; update the utils.Validator literal in cmd/berry/berry.go
(the call to utils.ValidateArgs) so the CmdName field is "berry" instead of
"types" — i.e., change utils.Validator{MaxArgs: 3, CmdName: "types",
RequireName: false, HasFlags: false} to use CmdName: "berry" so validation/error
messages reference the correct command.
In `@cmd/card/card.go`:
- Line 36: The call to utils.ValidateArgs is passing the wrong CmdName ("types")
causing misleading error messages; update the Validator literal used in cmd/card
(the utils.ValidateArgs call in card.go that sets Validator{MaxArgs: 3, CmdName:
"types", ...}) to use CmdName: "card", and likewise update the Validator literal
in cmd/berry (the utils.ValidateArgs call in berry.go that sets CmdName:
"types") to use CmdName: "berry" so checkNoOtherOptions and related error text
reference the correct command names.
In `@cmd/card/setslist.go`:
- Around line 93-96: On the error path inside the message handler where you set
m.Error and m.Loading = false, ensure m.List is either initialized to a valid
list.Model or that downstream handlers guard against a zero-value list before
calling methods on it; specifically update the error branch in the handler that
sets m.Error so it initializes m.List (the list.Model used elsewhere) or add
nil/zero checks before calling m.List.SetWidth and m.List.Update(msg)
(referencing the m.List, SetWidth and Update usage in the same file) to avoid
invoking methods on an uninitialized list.Model.
In `@cmd/utils/validateargs_test.go`:
- Line 194: Tests use incorrect Validator.CmdName values; update the Validator
instances in the listed tests so CmdName matches the real command name.
Specifically, change the CmdName in TestValidateNaturesArgs to "natures", in
TestValidateBerryArgs to "berry", in TestValidateCardArgs to "card", and in
TestValidateSpeedArgs to "speed" where Validator{... CmdName: ...} is
constructed in validateargs_test.go; ensure these Validator configs mirror the
production command code.
In `@cmd/utils/validateargs.go`:
- Around line 42-44: The error returned in the v.RequireName check uses plain
fmt.Errorf; update the branch in the validation logic (the block that checks
v.RequireName and len(args) == 2) to build the message using
styling.ErrorColor.Render("✖ Error!") combined with the rest of the text and
wrap the full message with styling.ErrorBorder.Render(...) before returning the
error (i.e., return fmt.Errorf(styling.ErrorBorder.Render(...))). Keep CmdName
(v.CmdName) in the message so the final error text matches other errors in the
file.
🧹 Nitpick comments (1)
styling/styling.go (1)
56-59: Consider using a consistent red across error styles.
ApiErrorStyleuses#FF0000while the adjacentErrorColoruses#F2055C. If this distinction is intentional (e.g., differentiating API errors from validation errors), a comment would help. Otherwise, consider reusing#F2055Cfor visual consistency.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/card/setslist.go (2)
77-89:enterkey handler callsm.List.SelectedItem()without an error guard.When
m.Erroris set,m.Listis a zero-valuelist.Model. Pressingenterin the error state callsm.List.SelectedItem()on it. While the.(item)type assertion should safely fail (ok == false), resulting in a clean quit, this is inconsistent with the explicit guards you added elsewhere and relies on an implicit contract oflist.Model's zero-value behavior.Consider adding a guard for consistency and safety:
Suggested fix
case "enter": + if m.Error != nil { + return m, nil + } i, ok := m.List.SelectedItem().(item)
137-143: Consider whether raw error details should be shown to end users.
m.Error.Error()may contain internal details like URLs, connection info, or stack traces depending on what the HTTP client or JSON parser returns. For a CLI tool this is likely fine (and even helpful for debugging), but if you want a cleaner UX, you could show a generic message and log the details separately.
1 participant
Summary by CodeRabbit
Bug Fixes
Style
Documentation
Refactor
Chores