feat: add popup window mode for LLM responses#58
feat: add popup window mode for LLM responses#58tracyone wants to merge 1 commit intoCoderCookE:mainfrom
Conversation
Introduce 'popup' as a new option for g:llm_agent_split_direction. This mode uses Vim 8+ floating windows to display AI responses in a non-intrusive overlay. Includes automatic fallback to vertical split when popup support is unavailable. Updates README documentation to reflect the new configuration option and usage details. Signed-off-by: tracyone <tracyone@live.cn>
There was a problem hiding this comment.
Pull request overview
Adds a new 'popup' display mode for LLM responses, using Vim floating popup windows when available and falling back to a split otherwise.
Changes:
- Add popup window capability detection and popup creation logic in the Vimscript display path.
- Extend response rendering to support
'popup'as ag:llm_agent_split_directionmode with fallback behavior. - Update README configuration docs to mention the new
'popup'option.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| autoload/chatgpt.vim | Implements popup support and integrates it into the response display flow. |
| README.md | Documents 'popup' as a g:llm_agent_split_direction option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else | ||
| silent execute winheight(0)/g:split_ratio.'new '. chat_gpt_session_id | ||
| " Check if popup already exists and is valid | ||
| let popup_exists = exists('g:llm_agent_popup_winid') && g:llm_agent_popup_winid > 0 && popup_getpos(g:llm_agent_popup_winid) != {} | ||
|
|
||
| if !popup_exists | ||
| call chatgpt#create_popup_window(chat_gpt_session_id) | ||
| endif |
There was a problem hiding this comment.
The popup window ID is stored in a single global (g:llm_agent_popup_winid). chatgpt#display_response() can be called with different chat_gpt_session_id values (e.g., persistent session vs other commands), and this implementation will reuse the same popup even when the session buffer changes, causing the popup to show the wrong session or stop updating. Consider tracking popup winids per session id, or explicitly retarget the existing popup to the current session buffer when chat_gpt_session_id changes.
| " Get or create buffer | ||
| let bufnr = bufnr(a:chat_gpt_session_id) | ||
| if bufnr == -1 | ||
| let bufnr = bufadd(a:chat_gpt_session_id) | ||
| call bufload(bufnr) | ||
| call setbufvar(bufnr, '&buftype', 'nofile') | ||
| call setbufvar(bufnr, '&bufhidden', 'hide') | ||
| call setbufvar(bufnr, '&swapfile', 0) | ||
| call setbufvar(bufnr, '&ft', 'markdown') | ||
| call setbufvar(bufnr, '&syntax', 'markdown') | ||
| call setbufvar(bufnr, '&wrap', 1) | ||
| call setbufvar(bufnr, '&linebreak', 1) | ||
| endif |
There was a problem hiding this comment.
Buffer initialization (bufadd/bufload + setbufvar calls) is duplicated in chatgpt#create_popup_window() and again in the popup branch of chatgpt#display_response(). This makes it easy for the two paths to drift (e.g., option differences) and increases maintenance cost. Consider extracting a small helper to “ensure session buffer exists and is configured” and reuse it from both places.
| - **g:llm_agent_temperature**: Controls response randomness (0.0-1.0). Higher = more creative, lower = more focused. Default: 0.7 | ||
| - **g:llm_agent_lang**: Request responses in a specific language (e.g., `'Chinese'`, `'Spanish'`). Default: none (English) | ||
| - **g:llm_agent_split_direction**: Window split direction: `'vertical'` or `'horizontal'`. Default: `'vertical'` | ||
| - **g:llm_agent_split_direction**: Window display mode: `'vertical'`, `'horizontal'`, or `'popup'`. Default: `'vertical'`. The `'popup'` option uses Vim 8+ floating window for a non-intrusive overlay display. |
There was a problem hiding this comment.
README says the 'popup' option uses “Vim 8+” floating windows, but the code warns that it requires Vim 8.2+ with +popupwin (has('popupwin')). Update the README to match the actual minimum requirement and mention the automatic fallback behavior.
| - **g:llm_agent_split_direction**: Window display mode: `'vertical'`, `'horizontal'`, or `'popup'`. Default: `'vertical'`. The `'popup'` option uses Vim 8+ floating window for a non-intrusive overlay display. | |
| - **g:llm_agent_split_direction**: Window display mode: `'vertical'`, `'horizontal'`, or `'popup'`. Default: `'vertical'`. The `'popup'` option requires Vim 8.2+ with `+popupwin` support (`has('popupwin')`) for a non-intrusive overlay display; if popup windows are unavailable, it automatically falls back to a regular split. |
| " Update popup content | ||
| let winid = exists('g:llm_agent_popup_winid') ? g:llm_agent_popup_winid : 0 | ||
| if winid > 0 | ||
| call popup_settext(winid, getbufline(bufnr, 1, '$')) | ||
| " Scroll to bottom of popup | ||
| call win_execute(winid, 'normal! G') | ||
| endif |
There was a problem hiding this comment.
In popup mode the popup content is updated (popup_settext + scroll) before the new response is appended to the session buffer (setbufline happens later). This means the popup can display stale content (often one response behind), and it also won’t be refreshed after the buffer changes. Consider updating the popup after setbufline(), or avoid popup_settext entirely and rely on the buffer-backed popup created by popup_create(bufnr, ...) and just scroll it after writing.
| " Update popup content | |
| let winid = exists('g:llm_agent_popup_winid') ? g:llm_agent_popup_winid : 0 | |
| if winid > 0 | |
| call popup_settext(winid, getbufline(bufnr, 1, '$')) | |
| " Scroll to bottom of popup | |
| call win_execute(winid, 'normal! G') | |
| endif | |
| " Do not manually copy buffer text into the popup before the response | |
| " is written. The popup is buffer-backed, so it should reflect the | |
| " buffer contents after the later setbufline() update. |
| call setbufvar(bufnr, '&ft', 'markdown') | ||
| call setbufvar(bufnr, '&syntax', 'markdown') | ||
| call setbufvar(bufnr, '&wrap', 1) | ||
| call setbufvar(bufnr, '&linebreak', 1) |
There was a problem hiding this comment.
wrap and linebreak are window-local options in Vim; setting them via setbufvar(bufnr, '&wrap', ...) / setbufvar(bufnr, '&linebreak', ...) won’t reliably apply to the popup window (and may be ignored). Prefer setting these with the popup options (where supported) and/or via win_execute(winid, 'setlocal wrap linebreak') after creating the popup window.
| call setbufvar(bufnr, '&wrap', 1) | ||
| call setbufvar(bufnr, '&linebreak', 1) | ||
| endif | ||
|
|
||
| " Update popup content | ||
| let winid = exists('g:llm_agent_popup_winid') ? g:llm_agent_popup_winid : 0 | ||
| if winid > 0 |
There was a problem hiding this comment.
Same issue here: wrap/linebreak are window-local, so setting them with setbufvar() on the buffer won’t ensure the popup window actually uses those settings. Use win_execute() on the popup winid (or popup options) to apply window-local settings.
| call setbufvar(bufnr, '&wrap', 1) | |
| call setbufvar(bufnr, '&linebreak', 1) | |
| endif | |
| " Update popup content | |
| let winid = exists('g:llm_agent_popup_winid') ? g:llm_agent_popup_winid : 0 | |
| if winid > 0 | |
| endif | |
| " Update popup content | |
| let winid = exists('g:llm_agent_popup_winid') ? g:llm_agent_popup_winid : 0 | |
| if winid > 0 | |
| call win_execute(winid, 'setlocal wrap') | |
| call win_execute(winid, 'setlocal linebreak') |
Introduce 'popup' as a new option for g:llm_agent_split_direction. This mode uses Vim 8+ floating windows to display AI responses in a non-intrusive overlay. Includes automatic fallback to vertical split when popup support is unavailable. Updates README documentation to reflect the new configuration option and usage details.