refactor(ghost): extract helper functions and use dispatch tables #8
Conversation
Reduce cyclomatic complexity across ghost modules by: - Extracting small, focused helper functions from large monolithic functions - Using dispatch tables for update type handling in receiver/response/test - Breaking down context capture, health checks, and status building - Removing luacheck ignore annotations no longer needed Signed-off-by: assagman <ahmetsercansagman@gmail.com>
Summary of ChangesHello @assagman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors several core modules within ghost.nvim to enhance code organization and maintainability. The primary goal was to break down complex functions into smaller, more manageable helper functions and to replace cumbersome nested conditional statements with more elegant and extensible dispatch tables. These changes aim to make the codebase easier to understand, test, and extend in the future, without introducing any breaking changes to the plugin's existing functionality or API. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent refactoring that significantly improves the structure and maintainability of the codebase. Breaking down large functions into smaller, focused helpers in modules like context, health, response, and status makes the logic much easier to follow. The use of dispatch tables to replace long if/elseif chains in receiver, response, and test is a great pattern that enhances clarity and extensibility. The changes are well-executed and align perfectly with the goals stated in the PR description. I've left one minor suggestion in lua/ghost/test.lua to further reduce some code duplication, in line with the spirit of this refactoring.
| local function handle_test_update(update, output_lines) | ||
| local inner = update.update or update | ||
| local update_type = inner.sessionUpdate | ||
|
|
||
| local handlers = { | ||
| message_delta = function() | ||
| local content = inner.content | ||
| if content and type(content) == "table" then | ||
| for _, item in ipairs(content) do | ||
| if item.type == "text" and item.text then | ||
| io.write(item.text) | ||
| io.flush() | ||
| table.insert(output_lines, item.text) | ||
| end | ||
| end | ||
| end | ||
| end, | ||
| message = function() | ||
| local content = inner.content | ||
| if content and type(content) == "table" then | ||
| for _, item in ipairs(content) do | ||
| if item.type == "text" and item.text then | ||
| io.write(item.text) | ||
| io.flush() | ||
| table.insert(output_lines, item.text) | ||
| end | ||
| end | ||
| end | ||
| end, | ||
| tool_call = function() | ||
| print("\n[Tool: " .. (inner.title or "unknown") .. " (" .. (inner.kind or "?") .. ")]") | ||
| end, | ||
| tool_call_update = function() | ||
| local status = inner.status or "unknown" | ||
| print("[Tool update: " .. (inner.toolCallId or "?") .. " - " .. status .. "]") | ||
| end, | ||
| plan_update = function() | ||
| print("[Plan updated]") | ||
| end, | ||
| plan = function() | ||
| print("[Plan updated]") | ||
| end, | ||
| } | ||
|
|
||
| local handler = handlers[update_type] | ||
| if handler then | ||
| handler() | ||
| else | ||
| print("[Update: " .. vim.inspect(update):sub(1, 200) .. "]") | ||
| end | ||
| end |
There was a problem hiding this comment.
This is a great refactoring to use a dispatch table. To further improve it and reduce code duplication, you can extract the shared logic for message_delta/message and plan_update/plan into separate helper functions. This makes the handlers table even cleaner and more declarative.
local function handle_test_update(update, output_lines)
local inner = update.update or update
local update_type = inner.sessionUpdate
local function handle_text_message()
local content = inner.content
if content and type(content) == "table" then
for _, item in ipairs(content) do
if item.type == "text" and item.text then
io.write(item.text)
io.flush()
table.insert(output_lines, item.text)
end
end
end
end
local function handle_plan_update()
print("[Plan updated]")
end
local handlers = {
message_delta = handle_text_message,
message = handle_text_message,
tool_call = function()
print("\n[Tool: " .. (inner.title or "unknown") .. " (" .. (inner.kind or "?") .. ")]")
end,
tool_call_update = function()
local status = inner.status or "unknown"
print("[Tool update: " .. (inner.toolCallId or "?") .. " - " .. status .. "]")
end,
plan_update = handle_plan_update,
plan = handle_plan_update,
}
local handler = handlers[update_type]
if handler then
handler()
else
print("[Update: " .. vim.inspect(update):sub(1, 200) .. "]")
end
end
There was a problem hiding this comment.
Pull request overview
This PR refactors multiple modules in ghost.nvim to improve code organization and maintainability by extracting large functions into smaller, focused helper functions and replacing nested conditionals with dispatch tables for better readability and extensibility.
Changes:
- Extract complex conditional logic into dispatch tables for cleaner update handling
- Break down large functions into smaller, single-purpose helper functions
- Improve code organization while preserving all existing functionality
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lua/ghost/test.lua | Extracted handle_test_update function with dispatch table for update types |
| lua/ghost/status.lua | Split build_status_content into focused section builder functions |
| lua/ghost/response.lua | Extracted text processing logic and update handlers with dispatch table |
| lua/ghost/receiver.lua | Refactored session update processing into separate handlers with dispatch table |
| lua/ghost/health.lua | Broke down health check into focused functions for each check type |
| lua/ghost/context.lua | Extracted buffer info, cursor, and selection gathering into helper functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lua/ghost/test.lua
Outdated
| local handlers = { | ||
| message_delta = function() | ||
| local content = inner.content | ||
| if content and type(content) == "table" then | ||
| for _, item in ipairs(content) do | ||
| if item.type == "text" and item.text then | ||
| io.write(item.text) | ||
| io.flush() | ||
| table.insert(output_lines, item.text) | ||
| end | ||
| end | ||
| end | ||
| end, | ||
| message = function() | ||
| local content = inner.content | ||
| if content and type(content) == "table" then | ||
| for _, item in ipairs(content) do | ||
| if item.type == "text" and item.text then | ||
| io.write(item.text) | ||
| io.flush() | ||
| table.insert(output_lines, item.text) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
The handlers for message_delta and message contain identical logic. Consider extracting this into a single shared handler function to eliminate the duplication.
| local handlers = { | |
| message_delta = function() | |
| local content = inner.content | |
| if content and type(content) == "table" then | |
| for _, item in ipairs(content) do | |
| if item.type == "text" and item.text then | |
| io.write(item.text) | |
| io.flush() | |
| table.insert(output_lines, item.text) | |
| end | |
| end | |
| end | |
| end, | |
| message = function() | |
| local content = inner.content | |
| if content and type(content) == "table" then | |
| for _, item in ipairs(content) do | |
| if item.type == "text" and item.text then | |
| io.write(item.text) | |
| io.flush() | |
| table.insert(output_lines, item.text) | |
| end | |
| end | |
| end | |
| local function handle_message_content() | |
| local content = inner.content | |
| if content and type(content) == "table" then | |
| for _, item in ipairs(content) do | |
| if item.type == "text" and item.text then | |
| io.write(item.text) | |
| io.flush() | |
| table.insert(output_lines, item.text) | |
| end | |
| end | |
| end | |
| end | |
| local handlers = { | |
| message_delta = function() | |
| handle_message_content() | |
| end, | |
| message = function() | |
| handle_message_content() |
|
Thanks for the review suggestions! I've addressed all the identified issues: Changes MadeDeduplicated Handlers
Impact: Reduced code from 29 to 20 lines (-9 lines) Review Status✅ All review threads resolved
Ready for re-review! 🎉 |
|
Thanks for the update, @assagman! I appreciate you addressing the feedback and providing a clear summary of the changes. I will proceed with a re-review of the pull request. |
Summary
This PR refactors several modules in ghost.nvim to improve code organization and maintainability by extracting large functions into smaller, focused helper functions and replacing nested conditionals with dispatch tables.
Changes
safe_get_bufnr,get_buffer_info,get_cursor_info,get_selection_info)check_nvim_version,check_config,check_backend_deps, etc.)update_handlers)Breaking Changes
None - this is a pure internal refactoring with no API changes.
Testing
Checklist