Show Content summary Instead of argumentsText to Display Tool Calls
#30
Conversation
joaopluigi
commented
Aug 19, 2025
joaopluigi
commented
| Before | After |
|---|---|
 |
 |
There was a problem hiding this comment.
Pull Request Overview
Updates the tool call display logic to show a more user-friendly summary instead of raw arguments text, improving the UI presentation of tool calls in the sidebar.
- Replaces tool call arguments display with summary text for better readability
- Adds text replacement functionality to update tool call status from preparation to completion
- Refactors message display logic to separate user-facing content from debug logging
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lua/eca/utils.lua | Simplifies line splitting by using vim.split instead of custom implementation |
| lua/eca/sidebar.lua | Updates tool call display to show summary instead of arguments, adds text replacement logic |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lua/eca/sidebar.lua
Outdated
| -- IMPORTANT: Return immediately - do NOT display anything for toolCallPrepare | ||
| return | ||
| elseif content.type == "toolCalled" then | ||
| local tool_text = "" |
There was a problem hiding this comment.
The variable tool_text is initialized as an empty string but only conditionally assigned a value. Consider initializing it to a meaningful default value or restructuring the logic to ensure it's always assigned before use.
| local tool_text = "" | |
| local tool_text = nil |
Copilot uses AI. Check for mistakes.
| changed = true | ||
| break | ||
| end | ||
| end |
There was a problem hiding this comment.
The function searches through all buffer lines from bottom to top and performs string operations on each line. For large buffers, this could be inefficient. Consider adding a limit on how many lines to search or using a more targeted approach if the expected location is known.
Copilot uses AI. Check for mistakes.
lua/eca/sidebar.lua
Outdated
|
|
||
| tool_text_completed = tool_text_completed .. (content.summary or content.name or "Tool call completed") | ||
|
|
||
| if not self:_replace_text(tool_text, tool_text_completed) then |
There was a problem hiding this comment.
If tool_text is an empty string (when self:_display_tool_call() is not called), the replacement will fail and fall back to adding a new message. This could result in unexpected behavior where tool completion messages are always added as new messages instead of replacing existing ones.
| if not self:_replace_text(tool_text, tool_text_completed) then | |
| if tool_text ~= "" then | |
| if not self:_replace_text(tool_text, tool_text_completed) then | |
| self:_add_message("assistant", tool_text_completed) | |
| end | |
| else |
Copilot uses AI. Check for mistakes.
| end | ||
| return lines | ||
| return vim.split(text, "\n", { plain = true, trimempty = false }) | ||
| end |
There was a problem hiding this comment.
I changed this because it treated texts with two consecutive \n as one \n. Also, vim.split seems more straightforward.
There was a problem hiding this comment.
This is a capture group, either /r or /n or \r\n
There was a problem hiding this comment.
Do you think that not capturing "[^\r\n]+" will be a problem or it should be fine?
There was a problem hiding this comment.
I believe \r\n is a newline on windows, \n is a newline for unix systems.
There was a problem hiding this comment.
Well, it seems that on Windows, when using vim.split(..., '\n'), it would be necessary to remove \r left in the line, but I am still not sure if this needs to be handled (or would not make difference on that case).
