Fix chat headers and make them configurable#42
Conversation
There was a problem hiding this comment.
Pull Request Overview
Fixes missing user message headers in the chat interface and adds configuration options for customizing chat headers. The PR resolves an issue where user messages weren't displaying the "## 👤 You" header and introduces the ability to customize both user and assistant headers through configuration.
- Fixed user message header display by ensuring user messages are properly added with headers
- Added configurable chat headers through the config system with sensible defaults
- Updated message rendering logic to use the configurable headers consistently
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lua/eca/sidebar.lua | Updated message handling to display user headers and use configurable headers from config |
| lua/eca/config.lua | Added chat configuration section with default header settings |
| docs/configuration.md | Added documentation for the new chat header configuration options |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| -- Only check for empty text | ||
| if not text or text == "" then | ||
| Logger.trace("Ignoring empty text response") | ||
| Logger.debug("Ignoring empty text response") |
There was a problem hiding this comment.
[nitpick] This change from Logger.trace to Logger.debug increases log verbosity for what appears to be a common occurrence (empty text). Consider keeping it as trace level to avoid cluttering debug logs.
| Logger.debug("Ignoring empty text response") | |
| Logger.trace("Ignoring empty text response") |
There was a problem hiding this comment.
.trace does not exists at Logger
| break | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
String comparison using line:sub(1, #assistant_header) could fail if assistant_header is empty (length 0). This would cause line:sub(1, 0) to return an empty string, potentially matching any line incorrectly. Add a guard to check if assistant_header is not empty before the comparison.
| if assistant_header == "" then | |
| return 0 | |
| end |
There was a problem hiding this comment.
I believe it should be able to match an empty line 🤔, this is iterating backwards so it is important that we match the first empty line it finds from the end to the beginning
|
Can we have the default as your print of the custom headers? it's what other editors do, and TBH I doubt someone will customize that |
I want to test this for some time first before making it the default. The code has some logic to get the last assistant message based on the header, and I want to stress this behavior more in the cases where the assistant header is empty. WDYT? |
Context
Header
## 👤 Youwas not being displayed for user messages:Before
After
After with custom headers
This PR also add the option to configure custom headers such as: