-
Notifications
You must be signed in to change notification settings - Fork 94
feat: improve artisan command output readability #766
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe pull request introduces enhancements to the command-line interface (CLI) functionality across multiple files in the console package. The changes focus on improving error handling, command registration, and output customization. Key modifications include adding custom handlers for unrecognized commands and usage errors, implementing more informative help and error messages, and updating the application initialization process. The changes aim to provide a more robust and user-friendly CLI experience with improved error guidance and output formatting. Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant CommandHandler
participant ErrorHandler
User->>CLI: Enter command
alt Command exists
CLI->>CommandHandler: Execute command
CommandHandler-->>User: Return result
else Command not found
CLI->>ErrorHandler: Trigger commandNotFound
ErrorHandler->>User: Suggest alternatives
else Usage error
CLI->>ErrorHandler: Trigger onUsageError
ErrorHandler->>User: Provide detailed error guidance
end
This sequence diagram illustrates the enhanced command handling flow, showing how different scenarios (successful command, unknown command, and usage errors) are now managed with more sophisticated error handling and user guidance. Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #766 +/- ##
==========================================
+ Coverage 69.88% 69.97% +0.08%
==========================================
Files 212 213 +1
Lines 17890 18118 +228
==========================================
+ Hits 12503 12678 +175
- Misses 4708 4749 +41
- Partials 679 691 +12 ☔ View full report in Codecov by Sentry. |
FYI, please add the |
Review Ready |
console/application.go
Outdated
@@ -1,204 +1,207 @@ | |||
package console | |||
|
|||
import ( | |||
"os" | |||
"strings" | |||
"os" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lint should not be changed, what IDE are you using, please? Is there a wrong configuration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am using Goland, and I have configured it to use 4 spaces for tabs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty good, so beautiful 👍 Just wondering, the logic is a bit complicated. Did you write it manually or reference something?
console/cli_helper_test.go
Outdated
"bytes" | ||
"github.com/goravel/framework/contracts/console" | ||
"github.com/goravel/framework/contracts/console/command" | ||
"github.com/goravel/framework/support/color" | ||
"github.com/stretchr/testify/assert" | ||
"io" | ||
"testing" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sort them:
System packages
Third packages
Internal packages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay
And please update branch |
Should I update branch with |
I manually wrote the help template for urfave/cli based on Laravel artisan format and implemented the findAlternatives method in Go, which replicates the command suggestion logic from Laravel. |
Amazing, Great work! I updated the branch, and we need to use the rebase command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
console/service_provider.go (1)
16-21
: Consider adding error handling for application initialization.While the changes improve the CLI interface, the initialization could benefit from explicit error handling. Consider wrapping the
NewApplication
call in a try-catch mechanism or adding error checks to ensure graceful failure if initialization fails.Example approach:
app.Singleton(Binding, func(app foundation.Application) (any, error) { name := "artisan" usage := "Goravel Framework" usageText := "artisan [global options] command [options] [arguments...]" - return NewApplication(name, usage, usageText, app.Version(), true), nil + application, err := NewApplication(name, usage, usageText, app.Version(), true) + if err != nil { + return nil, fmt.Errorf("failed to initialize console application: %w", err) + } + return application, nil })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
console/application.go
(3 hunks)console/cli_helper.go
(1 hunks)console/cli_helper_test.go
(1 hunks)console/service_provider.go
(1 hunks)support/color/color.go
(1 hunks)
🔇 Additional comments (5)
console/cli_helper_test.go (2)
3-13
: Sort imports according to Go conventions
For improved readability and maintainability, imports should be grouped and ordered as follows:
// Standard library imports
"bytes"
"io"
"testing"
// Third-party imports
"github.com/stretchr/testify/assert"
// Internal imports
"github.com/goravel/framework/contracts/console"
"github.com/goravel/framework/contracts/console/command"
"github.com/goravel/framework/support/color"
As previously mentioned, please sort the imports into standard library packages, third-party packages, and internal packages, each separated by a blank line.
107-108
: 🛠️ Refactor suggestion
Validate error handling in test cases
In the test cases where invalid commands or options are provided (e.g., "command not found", "option not found"), the assertion assert.NoError(t, cliApp.Call(tt.call))
may not be appropriate because these cases are expected to produce errors or specific outputs. Instead, consider verifying that the errors are handled correctly and that the expected error messages are displayed.
Modify the test assertions to check for expected errors or output:
func(io.Writer) {
- assert.NoError(t, cliApp.Call(tt.call))
+ err := cliApp.Call(tt.call)
+ assert.Nil(t, err)
}
Ensure that the tests validate both the absence of unexpected errors and the presence of correct error messages in the output.
Likely invalid or redundant comment.
console/application.go (2)
43-45
: LGTM! Enhanced command registration with better organization and error handling
The additions improve command organization through categories and provide consistent error handling at the command level.
24-25
: Verify the implementation of CLI error handlers
The addition of commandNotFound
and onUsageError
handlers aligns with improving CLI output readability. However, we need to ensure these handlers are properly implemented.
✅ Verification successful
Let me try a different search strategy to locate these handlers, as they might be defined as variables or using different naming patterns.
CLI error handlers are properly implemented
The handlers are well-implemented in console/cli_helper.go
:
commandNotFound
: Provides user-friendly error messages with command suggestionsonUsageError
: Handles flag-related errors with clear error messages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of CLI error handlers
# Expected: Find the handler implementations in the codebase
# Search for handler implementations
echo "Searching for commandNotFound implementation:"
ast-grep --pattern 'func commandNotFound($_) $_'
echo "Searching for onUsageError implementation:"
ast-grep --pattern 'func onUsageError($_) $_'
Length of output: 294
Script:
#!/bin/bash
# Search for handler definitions with broader patterns
echo "Searching for commandNotFound variable definition:"
ast-grep --pattern 'commandNotFound = $_'
echo "Searching for onUsageError variable definition:"
ast-grep --pattern 'onUsageError = $_'
# Also search using ripgrep for more context
echo "Searching with ripgrep for both handlers:"
rg "commandNotFound|onUsageError" -A 5 -B 5
Length of output: 2783
console/service_provider.go (1)
17-18
: LGTM! Parameter changes improve CLI clarity.
The renaming of parameters makes the CLI more intuitive, with "artisan" as the command name and "Goravel Framework" as the description.
Let's verify the integration with the new error handlers mentioned in the summary:
✅ Verification successful
Error handlers are properly integrated and implemented
The verification confirms:
- The
NewApplication
function is correctly used with 5 parameters in the service provider - Error handlers
commandNotFound
andonUsageError
are properly implemented inconsole/cli_helper.go
with:- Command suggestion functionality for undefined commands
- Specific error handling for flag-related issues
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration of new error handlers in the application initialization
# Check if the new error handlers are properly imported and used
ast-grep --pattern 'NewApplication($_, $_, $_, $_, $_)' | grep -A 5 -B 5 'NewApplication'
# Verify the existence and implementation of error handlers
rg -A 5 'func (.*commandNotFound|onUsageError)'
Length of output: 1286
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
console/cli_helper.go (1)
199-241
: Improve error handling inprintHelpCustom
functionErrors during template parsing and execution are currently logged only when
CLI_TEMPLATE_ERROR_DEBUG
is set. Consider logging these errors unconditionally or providing user feedback to assist in debugging.Modify the error handling to ensure critical errors are not missed.
for name, value := range templates { if _, err := t.New(name).Parse(value); err != nil { - if os.Getenv("CLI_TEMPLATE_ERROR_DEBUG") != "" { _, _ = fmt.Fprintf(cli.ErrWriter, "CLI TEMPLATE ERROR: %#v\n", err) - } } } // Similar changes for other error checks...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
console/cli_helper.go
(1 hunks)
🔇 Additional comments (1)
console/cli_helper.go (1)
175-180
: [Duplicate Comment] Empty string handling in capitalize
function
Good job handling empty strings to prevent index out of range errors in the capitalize
function, as previously suggested.
📑 Description
Optimizes the output of Artisan commands by introducing multi-color formatting.
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✅ Checks