refactor!: Clarified BetterCommandRunner output/logger setup #29
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the logging mechanism used by the Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Command as BetterCommand
participant Logger as MessageOutput
User->>Command: Invoke printUsage
Command->>Logger: logUsage(usageInfo)
sequenceDiagram
participant Runner as BetterCommandRunner
participant Logger as MessageOutput
participant Error as UsageException
Runner->>Runner: Detect error condition
Runner->>Logger: logUsageException(Error)
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
SandPod
left a comment
SandPod
left a comment
There was a problem hiding this comment.
Suggestion to simplify the interface.
I'm not sure we need the abstraction in this case since it is simply configuration passed in.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/src/better_command_runner/better_command_runner.dart (1)
9-34: Consider adding more specific documentation about migrationFor users who are updating from the previous API, it would be helpful to include explicit documentation about how this new class replaces the previous logging approach.
/// A proxy for user-provided functions for passing specific log messages. /// /// It is valid to not provide a function in order to not pass that output. +/// +/// This class replaces the previous approach of passing separate `logError` and +/// `logInfo` callback functions, consolidating them into a single interface. final class MessageOutput {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/src/better_command_runner/better_command.dart(1 hunks)lib/src/better_command_runner/better_command_runner.dart(5 hunks)test/better_command_runner/logging_test.dart(1 hunks)test/better_command_test.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- test/better_command_runner/logging_test.dart
- lib/src/better_command_runner/better_command.dart
🔇 Additional comments (10)
test/better_command_test.dart (2)
7-7: Constructor parameter updated to match the refactored APIThe constructor parameter has been correctly updated from
logInfotopassOutputto align with the changes in theBetterCommandclass hierarchy.
28-32: Successfully adapted the test to use the new MessageOutput classThe test now correctly instantiates a
MessageOutputobject with alogUsagefunction instead of directly passing a logging function. This change properly aligns with the refactored API while maintaining the same test logic.lib/src/better_command_runner/better_command_runner.dart (8)
9-34: Consider renaming to MessageOutputs (plural) for clarityThe class represents multiple output mechanisms (currently usage exceptions and usage messages), so a plural name might better reflect its purpose.
-final class MessageOutput { +final class MessageOutputs {The class looks well-designed with:
- Immutability (marked as
final)- Clear documentation for each method
- Proper null-safety with optional function callbacks
- Encapsulation of the logging functionality
61-61: Field renamed to reflect new consolidated approachThe change from separate
_logErrorand_logInfofields to a single_messageOutputfield successfully consolidates the logging concerns.
72-73: Documentation updated to reflect API changesThe constructor documentation has been correctly updated to describe the new
messageOutputparameter.
80-81: Constructor parameter updated for clarityThe constructor now accepts a single
MessageOutputparameter instead of separate logging functions, which simplifies the API and makes the intention clearer.
85-85: Constructor field assignment updated correctlyThe field assignment has been properly updated to match the new parameter.
137-137: Exception handling updated to use new logging approachThe exception logging has been correctly updated to use the new
_messageOutput?.logUsageException(e)method.
145-145: Usage printing updated to use new logging approachThe usage printing has been correctly updated to use the new
_messageOutput?.logUsage(usage)method.
193-193: Exception handling consistently updated throughout the classThe usage exception handling is consistently updated in all relevant locations.
In order to make it more clear what output behavior is being configured,
BetterCommandRunner's constructor parameters were changed from:to:
MessageOutput is a new type containing specific output members:
Summary by CodeRabbit
New Features
Refactor