Skip to content

Conversation

almas-x
Copy link
Contributor

@almas-x almas-x commented Dec 17, 2024

📑 Description

Optimizes the output of Artisan commands by introducing multi-color formatting.

Screenshots

image image

Summary by CodeRabbit

  • New Features

    • Enhanced command handling with custom behavior for unrecognized commands and usage errors.
    • Improved help and version printing for CLI applications with custom formatting.
    • Suggestions for similar command names when unrecognized commands are entered.
    • Enhanced output capture for logging during command execution.
  • Bug Fixes

    • Updated error handling logic to ensure successful exits even in error scenarios.
  • Tests

    • Introduced unit tests for command-line interface commands, validating help and error message formatting.
  • Chores

    • Updated application metadata for clarity and consistency.

✅ Checks

  • Added test cases for my code

@almas-x almas-x requested a review from a team as a code owner December 17, 2024 02:21
Copy link
Contributor

coderabbitai bot commented Dec 17, 2024

Walkthrough

The 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

File Change Summary
console/application.go - Updated NewApplication to include CommandNotFound and OnUsageError handlers
- Modified Register method to set OnUsageError for commands
- Changed Run method error handling to exit with code 0
console/cli_helper.go - Added new file with custom CLI help and error handling functions
- Implemented commandNotFound to suggest alternative commands
- Created custom help and version printing templates
- Added utility functions for output formatting
console/cli_helper_test.go - Added test suite for CLI helper functions
- Implemented test commands and scenarios for help and error handling
console/service_provider.go - Updated application name to "artisan"
- Modified usage description
support/color/color.go - Updated Print and Println methods to return pointer to support.Printer
- Enhanced CaptureOutput function to redirect various loggers

Sequence Diagram

sequenceDiagram
    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
Loading

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 @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Dec 17, 2024

Codecov Report

Attention: Patch coverage is 75.75758% with 56 lines in your changes missing coverage. Please review.

Project coverage is 69.97%. Comparing base (003a287) to head (bc1722e).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
console/cli_helper.go 76.01% 41 Missing and 12 partials ⚠️
console/service_provider.go 0.00% 2 Missing ⚠️
console/application.go 66.66% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

@hwbrzzl
Copy link
Contributor

hwbrzzl commented Dec 17, 2024

FYI, please add the Review Ready tag when the PR is ready.

@almas-x
Copy link
Contributor Author

almas-x commented Dec 17, 2024

Review Ready

@@ -1,204 +1,207 @@
package console

import (
"os"
"strings"
"os"
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

@hwbrzzl hwbrzzl left a 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?

Comment on lines 4 to 10
"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"
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

@hwbrzzl
Copy link
Contributor

hwbrzzl commented Dec 17, 2024

And please update branch

@almas-x
Copy link
Contributor Author

almas-x commented Dec 17, 2024

And please update branch

Should I update branch with merge commit or rebase?

@almas-x
Copy link
Contributor Author

almas-x commented Dec 17, 2024

Pretty good, so beautiful 👍 Just wondering, the logic is a bit complicated. Did you write it manually or reference something?

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.

@hwbrzzl
Copy link
Contributor

hwbrzzl commented Dec 17, 2024

Pretty good, so beautiful 👍 Just wondering, the logic is a bit complicated. Did you write it manually or reference something?

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.

hwbrzzl
hwbrzzl previously approved these changes Dec 17, 2024
Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hwbrzzl hwbrzzl enabled auto-merge (squash) December 17, 2024 07:05
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 003a287 and 1a30712.

📒 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 suggestions
  • onUsageError: 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 and onUsageError are properly implemented in console/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

@hwbrzzl hwbrzzl disabled auto-merge December 17, 2024 07:12
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 in printHelpCustom function

Errors 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

📥 Commits

Reviewing files that changed from the base of the PR and between 1a30712 and bc1722e.

📒 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.

@hwbrzzl hwbrzzl enabled auto-merge (squash) December 17, 2024 07:20
@hwbrzzl hwbrzzl merged commit 8afbbef into goravel:master Dec 17, 2024
11 checks passed
@almas-x almas-x deleted the optimize-artisan-output branch December 17, 2024 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants