feat!: Preserves UsageException instead of replacing it with ExitException #23
Conversation
BREAKING CHANGES: Upon argument parse errors the UsageException was replaced by an ExitException. This behavior is now changed to rethrow the orginal UsageException instead.
📝 WalkthroughWalkthroughThis pull request primarily introduces formatting improvements and minor refactoring across several modules and tests. The changes include reformatting constructor initializations, adjusting indentation, and updating test descriptions. A key functional change is in the command runner’s error handling: the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant BCR as BetterCommandRunner
participant C as Command
U->>BCR: runCommand(args)
BCR->>C: execute command
C-->>BCR: throws UsageException
BCR->>U: rethrow UsageException
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lib/src/better_command_runner/better_command_runner.dart(5 hunks)lib/src/logger/helpers/ansi_style.dart(1 hunks)lib/src/logger/loggers/std_out_logger.dart(2 hunks)test/better_command_runner/exit_exceptions_test.dart(1 hunks)test/prompts/multiple_select_test.dart(12 hunks)test/prompts/select_test.dart(10 hunks)test/std_out_logger_test.dart(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- test/prompts/multiple_select_test.dart
- lib/src/logger/helpers/ansi_style.dart
🔇 Additional comments (23)
test/better_command_runner/exit_exceptions_test.dart (3)
35-44: Tests properly enforce thrown UsageException for unknown command.This aligns with the new behavior of rethrowing
UsageException.
47-56: Tests properly enforce thrown UsageException for invalid command.This ensures consistent behavior across different invalid arguments scenarios.
59-68: Tests correctly handle UsageException for missing mandatory options.Validating missing arguments is now consistent with the updated usage exception logic.
test/prompts/select_test.dart (11)
12-15: Multi-line test description refactor.No functional changes; the readability improvement is approved.
30-33: Multi-line test description refactor.No functional changes.
48-51: Multi-line test description refactor.No functional changes.
62-65: Multi-line test description refactor.Approved as-is.
76-79: Multi-line test description refactor.Approved change.
98-101: Multi-line test description refactor.Approved with no objections.
108-117: Verify consistency between ExitException usage and the removal of ExitException in other parts.The rest of the codebase rethrows
UsageException, yet pressing "q" triggers anExitException. Confirm whether this is intentional for user cancellation or if it should align with the new logic.
120-123: Multi-line test description refactor.Looks good.
130-133: Multi-line test description refactor.No functional changes.
155-157: Multi-line test description refactor.No objections here.
177-179: Multi-line test description refactor.Approved.
test/std_out_logger_test.dart (2)
1-49: Sufficient coverage testing default logger behavior.Verifying output to stdout for all log levels is thorough.
51-96: Validates redirection to stderr above warning level.Tests confirm that warning and error logs are routed correctly to stderr.
lib/src/better_command_runner/better_command_runner.dart (4)
16-19: Formatting of typedef is acceptable.No functional change introduced; readability is fine.
64-69: Constructor reformat maintains existing logic.No further concerns.
119-119: Rethrowing UsageException preserves original details.Switching from throwing a custom exit exception to rethrowing
UsageExceptionimproves debugging accuracy.
175-175: Consistent rethrow of UsageException in runCommand.This aligns with the updated error-handling strategy throughout the runner.
lib/src/logger/loggers/std_out_logger.dart (3)
17-20: Clear documentation for the new parameterThe documentation clearly explains the purpose of the new
logToStderrLevelThresholdparameter and its default behavior.
211-217: Clean implementation of the new stdout/stderr routing logicThe implementation properly checks if the threshold is set before routing messages to stderr, aligning with the PR objective to make logging to stderr opt-in instead of the default behavior.
204-207: Minor formatting adjustment for improved readabilityNo functional changes in this segment, just improved indentation for better code readability.
|
This PR introduces multiple unrelated changes. To preserve a valid commit history in I think this is especially important since there are two separate breaking changes, |
christerswahn
force-pushed
the
646-errors
branch
from
90388d4 to
8c47920
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
16-21: CI Workflow (Dart Format Job): Updated Dart Setup Action & YAML IndentationThe update to use
dart-lang/setup-dart@v1.7.1with SDK version3.7in the dart_format job looks appropriate. However, YAML linting flagged an indentation issue on line 19 (expected 18 spaces but found 16). Please adjust the indentation to conform to YAML standards.- sdk: 3.7 + sdk: 3.7🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 19-19: wrong indentation: expected 18 but found 16
(indentation)
27-29: CI Workflow (Dart Analyze Job): Updated Dart Setup Action & Indentation WarningThe dart_analyze job now uses
dart-lang/setup-dart@v1.7.1which is consistent with our updated setup. Note that the YAML lint warning on line 29 indicates that the indentation is off (expected 18 spaces). Please fix the indentation.- sdk: 3.7 + sdk: 3.7🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 29-29: wrong indentation: expected 18 but found 16
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/workflows/ci.yml(2 hunks)lib/src/analytics/analytics.dart(2 hunks)lib/src/better_command_runner/better_command.dart(1 hunks)lib/src/logger/helpers/progress.dart(1 hunks)lib/src/logger/loggers/std_out_logger.dart(2 hunks)lib/src/package_version/package_version.dart(3 hunks)lib/src/package_version/pub_api_client.dart(1 hunks)lib/src/prompts/confirm.dart(1 hunks)lib/src/prompts/select.dart(3 hunks)test/local_storage_manager/local_storage_manager_test.dart(11 hunks)test/package_version_test.dart(3 hunks)test/prompts/confirm_test.dart(13 hunks)test/prompts/input_test.dart(6 hunks)test/prompts/multiple_select_test.dart(12 hunks)test/prompts/select_test.dart(10 hunks)test/test_utils/io_helper.dart(1 hunks)test/test_utils/mock_stdin.dart(1 hunks)
✅ Files skipped from review due to trivial changes (13)
- test/prompts/confirm_test.dart
- lib/src/prompts/confirm.dart
- lib/src/package_version/pub_api_client.dart
- test/package_version_test.dart
- test/prompts/input_test.dart
- test/test_utils/io_helper.dart
- lib/src/logger/helpers/progress.dart
- lib/src/analytics/analytics.dart
- lib/src/better_command_runner/better_command.dart
- test/test_utils/mock_stdin.dart
- lib/src/prompts/select.dart
- lib/src/package_version/package_version.dart
- test/local_storage_manager/local_storage_manager_test.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/src/logger/loggers/std_out_logger.dart
- test/prompts/multiple_select_test.dart
- test/prompts/select_test.dart
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci.yml
[warning] 19-19: wrong indentation: expected 18 but found 16
(indentation)
[warning] 29-29: wrong indentation: expected 18 but found 16
(indentation)
[warning] 39-39: wrong indentation: expected 18 but found 16
(indentation)
christerswahn
force-pushed
the
646-errors
branch
from
8c47920 to
8a9c2e7
Compare
christerswahn
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
36-40:⚠️ Potential issueInconsistent Setup Action Version & Indentation in Dart Test Job
The Dart Test job still usesdart-lang/setup-dart@v1.6.4(line 37) whereas the other jobs have been updated tov1.7.1. This inconsistency could lead to unexpected CI behavior. Additionally, thesdkparameter on line 39 is indented with 16 spaces instead of 18. Aligning the version and indentation with the other jobs will ensure uniform behavior across your CI pipeline.- - uses: dart-lang/setup-dart@v1.6.4 + - uses: dart-lang/setup-dart@v1.7.1- sdk: 3.7 + sdk: 3.7🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 39-39: wrong indentation: expected 18 but found 16
(indentation)
[error] 40-40: no new line character at the end of file
(new-line-at-end-of-file)
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
16-21: Indentation Issue in Dart Format Job
Thesdkparameter under thewith:block (line 19) is indented with 16 spaces instead of the expected 18 spaces according to YAMLlint. Correcting the indentation will help avoid potential YAML parsing issues.- sdk: 3.7 + sdk: 3.7🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 19-19: wrong indentation: expected 18 but found 16
(indentation)
27-29: Indentation Issue in Dart Analyze Job
Thesdkparameter (line 29) in the Dart Analyze job is indented with 16 spaces while YAMLlint expects 18 spaces. Please adjust this indentation for consistency and to prevent lint warnings.- sdk: 3.7 + sdk: 3.7🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 29-29: wrong indentation: expected 18 but found 16
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/workflows/ci.yml(2 hunks)lib/src/analytics/analytics.dart(2 hunks)lib/src/better_command_runner/better_command.dart(1 hunks)lib/src/logger/helpers/progress.dart(1 hunks)lib/src/logger/loggers/std_out_logger.dart(2 hunks)lib/src/package_version/package_version.dart(3 hunks)lib/src/package_version/pub_api_client.dart(1 hunks)lib/src/prompts/confirm.dart(1 hunks)lib/src/prompts/select.dart(3 hunks)test/local_storage_manager/local_storage_manager_test.dart(11 hunks)test/package_version_test.dart(3 hunks)test/prompts/confirm_test.dart(13 hunks)test/prompts/input_test.dart(6 hunks)test/prompts/multiple_select_test.dart(12 hunks)test/prompts/select_test.dart(10 hunks)test/test_utils/io_helper.dart(1 hunks)test/test_utils/mock_stdin.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- lib/src/package_version/pub_api_client.dart
- test/test_utils/mock_stdin.dart
- test/prompts/confirm_test.dart
- test/package_version_test.dart
- lib/src/logger/helpers/progress.dart
- test/prompts/input_test.dart
- lib/src/prompts/confirm.dart
- lib/src/prompts/select.dart
- lib/src/package_version/package_version.dart
- lib/src/better_command_runner/better_command.dart
- test/prompts/select_test.dart
- test/local_storage_manager/local_storage_manager_test.dart
- test/test_utils/io_helper.dart
- test/prompts/multiple_select_test.dart
- lib/src/logger/loggers/std_out_logger.dart
- lib/src/analytics/analytics.dart
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci.yml
[warning] 19-19: wrong indentation: expected 18 but found 16
(indentation)
[warning] 29-29: wrong indentation: expected 18 but found 16
(indentation)
[warning] 39-39: wrong indentation: expected 18 but found 16
(indentation)
christerswahn
force-pushed
the
646-errors
branch
from
8a9c2e7 to
d822c90
Compare
christerswahn
force-pushed
the
646-errors
branch
from
d822c90 to
bc8ca52
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/ci.yml (4)
19-19: Fix YAML indentation for Dart SDK version specification (dart_format job).Static analysis reports that line 19 is indented with 16 spaces while 18 spaces are expected. Please adjust the indentation to ensure proper YAML formatting.
- sdk: 3.6 + sdk: 3.6🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 19-19: wrong indentation: expected 18 but found 16
(indentation)
29-29: Fix YAML indentation for Dart SDK version specification (dart_analyze job).Line 29 is mis-indented (16 spaces instead of 18). Increasing the indentation will resolve the YAML lint warning.
- sdk: 3.6 + sdk: 3.6🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 29-29: wrong indentation: expected 18 but found 16
(indentation)
39-39: Fix YAML indentation for Dart SDK version specification (dart_test job).The static analysis indicates that line 39 has insufficient indentation. Please update it to 18 spaces.
- sdk: 3.6 + sdk: 3.6🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 39-39: wrong indentation: expected 18 but found 16
(indentation)
40-40: Ensure the file ends with a newline.The linter notes that there's no newline at the end of the file. Please add a newline to conform with best practices.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 40-40: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/workflows/ci.yml(1 hunks)lib/src/analytics/analytics.dart(2 hunks)lib/src/better_command_runner/better_command.dart(1 hunks)lib/src/logger/helpers/progress.dart(1 hunks)lib/src/logger/loggers/std_out_logger.dart(2 hunks)lib/src/package_version/package_version.dart(3 hunks)lib/src/package_version/pub_api_client.dart(1 hunks)lib/src/prompts/confirm.dart(1 hunks)lib/src/prompts/select.dart(3 hunks)test/local_storage_manager/local_storage_manager_test.dart(11 hunks)test/package_version_test.dart(3 hunks)test/prompts/confirm_test.dart(13 hunks)test/prompts/input_test.dart(6 hunks)test/prompts/multiple_select_test.dart(12 hunks)test/prompts/select_test.dart(10 hunks)test/test_utils/io_helper.dart(1 hunks)test/test_utils/mock_stdin.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- lib/src/logger/loggers/std_out_logger.dart
- test/local_storage_manager/local_storage_manager_test.dart
- lib/src/package_version/pub_api_client.dart
- lib/src/better_command_runner/better_command.dart
- test/prompts/input_test.dart
- test/prompts/confirm_test.dart
- lib/src/prompts/confirm.dart
- lib/src/prompts/select.dart
- lib/src/logger/helpers/progress.dart
- lib/src/analytics/analytics.dart
- test/package_version_test.dart
- test/prompts/multiple_select_test.dart
- lib/src/package_version/package_version.dart
- test/test_utils/io_helper.dart
- test/test_utils/mock_stdin.dart
- test/prompts/select_test.dart
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci.yml
[warning] 19-19: wrong indentation: expected 18 but found 16
(indentation)
[warning] 29-29: wrong indentation: expected 18 but found 16
(indentation)
[warning] 39-39: wrong indentation: expected 18 but found 16
(indentation)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
17-21: Confirm the intended Dart SDK version across CI jobs.All jobs now specify
sdk: 3.6withdart-lang/setup-dart@v1.7.1. Please verify that Dart SDK 3.6 is the expected version, as previous guidance mentioned version 3.7. Consistency in the target Dart SDK version is important for uniform CI behavior.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 19-19: wrong indentation: expected 18 but found 16
(indentation)
BREAKING CHANGES:
Upon argument parse errors the UsageException was replaced by an ExitException.
This behavior is now changed to rethrow the original UsageException instead.
Summary by CodeRabbit
Refactor
Chores
Bug Fixes