-
Notifications
You must be signed in to change notification settings - Fork 28
Standardize CLI error/warning messages to use console formatting helpers #4057
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
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Replace emoji-prefixed messages (⚠️ , ✓, 🔨) with console formatting - Use FormatWarningMessage for warnings (10 instances) - Use FormatSuccessMessage for success messages (3 instances) - Use FormatProgressMessage for progress messages (3 instances) - Use FormatListItem for list items (1 instance) - Add console import to packages.go - Total: 17 new console helper usages (668 -> 685) Co-authored-by: pelikhan <4175913+pelikhan@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.
Pull Request Overview
This PR standardizes CLI error and warning messages across 4 files by replacing raw emoji-prefixed output with console package formatting helpers. The changes improve consistency in user-facing messages by using FormatWarningMessage(), FormatSuccessMessage(), FormatProgressMessage(), and FormatListItem() functions.
Key Changes:
- Replaced 17 instances of raw emoji-prefixed messages (
⚠️ , ✓, 🔨) with console formatting helpers - Added console package import to
packages.go - Updated workflow lock file with security validation changes
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/compile_command.go | Standardized 10 messages (warnings, success, progress) to use console formatting helpers |
| pkg/cli/enable.go | Converted 5 warning messages to use FormatWarningMessage() |
| pkg/cli/packages.go | Added console import and converted 1 warning to use FormatWarningMessage() |
| pkg/cli/update_command.go | Updated list formatting to use FormatListItem() with one incorrect usage that duplicates error symbols |
| pkg/workflow/shell_backslash_integration_test.go | Code formatting changes (indentation only, no functional impact) |
| .github/workflows/cloclo.lock.yml | Removed "workflow_run" from safe events list (security change, unrelated to PR objective) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| core.info(`Event ${eventName} requires validation (write role not allowed)`); | ||
| } | ||
| const safeEvents = ["workflow_run", "schedule"]; | ||
| const safeEvents = ["schedule"]; |
Copilot
AI
Nov 15, 2025
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.
This change removes "workflow_run" from the list of safe events that bypass permission validation. While this appears to be a security-related change, it's not mentioned in the PR description which focuses on console formatting standardization.
If this change is intentional and related to security improvements, it should be documented in the PR description. If it's unintentional (e.g., from recompiling workflows with updated code), it should be verified that this doesn't break any expected workflow behavior.
| const safeEvents = ["schedule"]; | |
| const safeEvents = ["schedule", "workflow_run"]; |
| fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("Failed to update %d workflow(s):", len(failedUpdates)))) | ||
| for _, failure := range failedUpdates { | ||
| fmt.Fprintf(os.Stderr, " ✗ %s: %s\n", failure.Name, failure.Error) | ||
| fmt.Fprintf(os.Stderr, " %s %s: %s\n", console.FormatErrorMessage("✗"), failure.Name, failure.Error) |
Copilot
AI
Nov 15, 2025
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.
This will result in duplicate error symbols ("✗ ✗") being displayed. The FormatErrorMessage() function already adds a "✗ " prefix, so wrapping "✗" with it creates a double symbol.
Consider using a similar pattern to the success messages above (line 305) which uses FormatListItem():
fmt.Fprintf(os.Stderr, " ✗ %s: %s\n", failure.Name, failure.Error)Or if you want to use console formatting for consistency, create a different approach that doesn't duplicate the symbol.
| fmt.Fprintf(os.Stderr, " %s %s: %s\n", console.FormatErrorMessage("✗"), failure.Name, failure.Error) | |
| fmt.Fprintln(os.Stderr, " "+console.FormatErrorMessage(fmt.Sprintf("%s: %s", failure.Name, failure.Error))) |
Eliminates inconsistent error message styling in CLI commands by replacing raw emoji-prefixed messages with console package helpers.
Changes
Replaced in 4 files (17 instances total):
⚠️warnings →console.FormatWarningMessage()✓success →console.FormatSuccessMessage()🔨progress →console.FormatProgressMessage()Before:
After:
Files
pkg/cli/compile_command.go- 10 messages (warnings, success, progress)pkg/cli/enable.go- 5 warningspkg/cli/packages.go- 1 warning + console importpkg/cli/update_command.go- 1 list itemAll critical error/warning patterns now standardized. Informational messages (43 instances) intentionally remain plain text.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.