mod for side panel mode and buffer copy#12
Conversation
|
this is what I need :-) |
|
@KKrampis I'd like to merge this, can you resolve the new conflicts first? |
There was a problem hiding this comment.
Pull Request Overview
This PR adds two new configuration options to the Codex Neovim plugin: the ability to display Codex in a side panel (vertical split) instead of a floating window, and the ability to capture Codex output in a normal buffer instead of a terminal buffer (enabling vim yank/paste operations).
Key changes:
- Added
panelconfiguration option to toggle between floating window and side panel display modes - Added
use_bufferconfiguration option to capture stdout/stderr in a normal buffer for copy/paste functionality - Updated repository references from
johnseth97tokkrampis
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| lua/codex/init.lua | Implements new open_panel() function and use_buffer mode with jobstart callbacks for capturing output; adds configuration options |
| README.md | Documents the new panel and use_buffer configuration options; updates repository owner references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| on_stdout = function(_, data) | ||
| if not data then return end | ||
| for _, line in ipairs(data) do | ||
| if line ~= '' then | ||
| vim.api.nvim_buf_set_lines(state.buf, -1, -1, false, { line }) | ||
| end | ||
| end | ||
| end, | ||
| on_stderr = function(_, data) | ||
| if not data then return end | ||
| for _, line in ipairs(data) do | ||
| if line ~= '' then | ||
| vim.api.nvim_buf_set_lines(state.buf, -1, -1, false, { '[ERR] ' .. line }) | ||
| end | ||
| end | ||
| end, | ||
| on_exit = function(_, code) | ||
| state.job = nil | ||
| vim.api.nvim_buf_set_lines(state.buf, -1, -1, false, { | ||
| ('[Codex exit: %d]'):format(code), | ||
| }) | ||
| end, |
There was a problem hiding this comment.
The on_stdout, on_stderr, and on_exit callbacks don't verify that state.buf is still valid before writing to it. If the user closes the buffer while the job is still running, nvim_buf_set_lines will throw an error. Add a buffer validity check:
if not vim.api.nvim_buf_is_valid(state.buf) then return endat the start of each callback.
| -- capture stdout/stderr into normal buffer | ||
| state.job = vim.fn.jobstart(cmd_args, { | ||
| cwd = vim.loop.cwd(), | ||
| stdout_buffered = true, | ||
| on_stdout = function(_, data) | ||
| if not data then return end | ||
| for _, line in ipairs(data) do | ||
| if line ~= '' then | ||
| vim.api.nvim_buf_set_lines(state.buf, -1, -1, false, { line }) | ||
| end | ||
| end | ||
| end, | ||
| on_stderr = function(_, data) | ||
| if not data then return end | ||
| for _, line in ipairs(data) do | ||
| if line ~= '' then | ||
| vim.api.nvim_buf_set_lines(state.buf, -1, -1, false, { '[ERR] ' .. line }) | ||
| end | ||
| end | ||
| end, | ||
| on_exit = function(_, code) | ||
| state.job = nil | ||
| vim.api.nvim_buf_set_lines(state.buf, -1, -1, false, { | ||
| ('[Codex exit: %d]'):format(code), | ||
| }) | ||
| end, | ||
| }) | ||
| else | ||
| -- use a terminal buffer |
There was a problem hiding this comment.
[nitpick] Inconsistent inline comment style. Line 175 uses -- capture stdout/stderr into normal buffer (lowercase, no period) while line 203 uses -- use a terminal buffer (lowercase, no period). For consistency with the function-level comment on line 88 which uses proper capitalization and punctuation, consider standardizing all comments to use sentence case with proper punctuation.
| local win = vim.api.nvim_get_current_win() | ||
| vim.api.nvim_win_set_buf(win, state.buf) |
There was a problem hiding this comment.
[nitpick] The open_panel() function uses vim.cmd('vertical rightbelow vsplit') which creates a new empty buffer before switching to state.buf. This temporarily creates an unnecessary buffer. Consider using vim.api.nvim_open_win(state.buf, true, {...}) with appropriate split configuration, or explicitly delete the temporary buffer after the split.
| local win = vim.api.nvim_get_current_win() | |
| vim.api.nvim_win_set_buf(win, state.buf) | |
| local win = vim.api.nvim_get_current_win() | |
| local tmp_buf = vim.api.nvim_win_get_buf(win) | |
| vim.api.nvim_win_set_buf(win, state.buf) | |
| -- Delete the temporary buffer if it's not the same as state.buf and is listed | |
| if tmp_buf ~= state.buf and vim.api.nvim_buf_is_valid(tmp_buf) then | |
| -- Only delete if the buffer is listed (to avoid deleting user buffers) | |
| if vim.api.nvim_buf_get_option(tmp_buf, 'buflisted') then | |
| vim.api.nvim_buf_delete(tmp_buf, { force = true }) | |
| end | |
| end |
| panel = false, -- Open Codex in a side-panel (vertical split) instead of floating window | ||
| use_buffer = false, -- Capture Codex stdout into a normal buffer instead of a terminal buffer | ||
| }, | ||
| }``` |
There was a problem hiding this comment.
The code block syntax is malformed. Line 54 shows `}````, which combines the closing brace with the three backticks. These should be on separate lines for proper markdown formatting:
},
}
(closing backticks on next line)
| }``` | |
| } |
| if config.panel then open_panel() else open_window() end | ||
|
|
||
| if not state.job then | ||
| -- assemble command |
There was a problem hiding this comment.
[nitpick] The inline comment -- assemble command should be capitalized and end with a period for consistency with documentation standards: -- Assemble command.
| -- assemble command | |
| -- Assemble command. |
| if config.use_buffer then | ||
| -- capture stdout/stderr into normal buffer | ||
| state.job = vim.fn.jobstart(cmd_args, { | ||
| cwd = vim.loop.cwd(), | ||
| stdout_buffered = true, | ||
| on_stdout = function(_, data) | ||
| if not data then return end | ||
| for _, line in ipairs(data) do | ||
| if line ~= '' then | ||
| vim.api.nvim_buf_set_lines(state.buf, -1, -1, false, { line }) | ||
| end | ||
| end | ||
| end, | ||
| on_stderr = function(_, data) | ||
| if not data then return end | ||
| for _, line in ipairs(data) do | ||
| if line ~= '' then | ||
| vim.api.nvim_buf_set_lines(state.buf, -1, -1, false, { '[ERR] ' .. line }) | ||
| end | ||
| end | ||
| end, | ||
| on_exit = function(_, code) | ||
| state.job = nil | ||
| vim.api.nvim_buf_set_lines(state.buf, -1, -1, false, { | ||
| ('[Codex exit: %d]'):format(code), | ||
| }) | ||
| end, | ||
| }) |
There was a problem hiding this comment.
When using use_buffer = true, the buffer is not a terminal buffer, so users cannot interact with the Codex CLI (send input). The jobstart call doesn't configure stdin handling. If the Codex CLI requires user input, consider either:
- Adding
stdin = 'pipe'and implementing an input mechanism - Documenting that
use_buffer = trueis read-only and doesn't support interactive sessions - Using
pty = trueto enable terminal-like behavior while still capturing output
Currently, this creates a non-interactive experience that may not work correctly with Codex CLI.
| cmd = 'codex', | ||
| model = nil, -- Default to the latest model | ||
| autoinstall = true, | ||
| panel = false, -- if true, open Codex in a side-panel instead of floating window |
There was a problem hiding this comment.
[nitpick] The inline comments for the new config options use inconsistent spacing. Line 15 has panel = false, with extra spaces, while line 16 has use_buffer = false, with normal spacing. Align the equals signs consistently for better readability.
| panel = false, -- if true, open Codex in a side-panel instead of floating window | |
| panel = false, -- if true, open Codex in a side-panel instead of floating window |
Added the option to have it as buffer / split panel within neovim (can be enabled with true-false in the plugin configuration file). Also allows for vim normal mode yanking of output from codex and pasting into any other open buffer.