-
Notifications
You must be signed in to change notification settings - Fork 2
Display backend error messages in red #18
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
Greptile OverviewGreptile SummaryChanged the Key Issues:
Recommendation: Create a dedicated Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| src/cortex-core/src/style.rs | Changed system_message() style from muted gray to error red - affects all system messages including both errors and informational messages |
Sequence Diagram
sequenceDiagram
participant User
participant TUI as Cortex TUI
participant EventLoop
participant AppState
participant StyleRS as style.rs
participant ChatWidget as Chat Widget
User->>TUI: Executes command/triggers error
TUI->>EventLoop: Process command/error
alt Error condition
EventLoop->>EventLoop: add_system_message("Error: ...")
else Informational message
EventLoop->>EventLoop: add_system_message("Info message")
end
EventLoop->>AppState: add_message(Message::system(content))
AppState->>ChatWidget: Render message
ChatWidget->>StyleRS: Get style via CortexStyle::system_message()
alt Before this PR
StyleRS-->>ChatWidget: Returns TEXT_MUTED (gray #486581)
else After this PR
StyleRS-->>ChatWidget: Returns ERROR (red #FF6B6B)
end
ChatWidget->>User: Display message in styled color
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.
1 file reviewed, 1 comment
src/cortex-core/src/style.rs
Outdated
| /// System message style: error red italic text for backend error messages | ||
| #[inline] | ||
| pub fn system_message() -> Style { | ||
| Style::default() | ||
| .fg(TEXT_MUTED) | ||
| .fg(ERROR) | ||
| .add_modifier(Modifier::ITALIC) | ||
| } |
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.
system_message() is used for both errors AND informational messages (e.g., "Resumed session: {id}", command output). Making all system messages red misleads users into thinking informational messages are errors.
Consider creating a separate error_message() style as the PR description suggests, or distinguish message types at the call site.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cortex-core/src/style.rs
Line: 365:371
Comment:
`system_message()` is used for both errors AND informational messages (e.g., "Resumed session: {id}", command output). Making all system messages red misleads users into thinking informational messages are errors.
Consider creating a separate `error_message()` style as the PR description suggests, or distinguish message types at the call site.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.de17bae to
339ce15
Compare
Address Greptile review feedback on PR #18: - Reverted system_message() to use TEXT_MUTED (gray) for informational messages - Added new error_message() style with ERROR (red) + italic for backend errors - Rendering logic in rendering.rs already detects errors and uses colors.error This ensures regular system messages (like 'Resumed session') stay muted gray while actual error messages are displayed in red.
Changes
system_message()style incortex-core/src/style.rsTEXT_MUTED(gray #486581) toERROR(red #FF6B6B)Visual Impact
All system messages (including backend errors) will now appear in red with italic styling.