From f5f97e18604286b00271557e21d533529a502671 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Sat, 25 Jul 2026 14:21:43 -0700 Subject: [PATCH] fix(terminal): don't leak window options into buffers that replace the cli window. Fixes #294 Performing :edit from the sidekick window left the new buffer with the terminal styling (no number column, fixed width, etc). The styling is now buffer-local so Neovim restores your own options, and sidekick lets go of the window when another buffer takes it over. --- lua/sidekick/cli/scrollback.lua | 1 + lua/sidekick/cli/terminal.lua | 38 ++++++++- lua/sidekick/config.lua | 4 +- tests/terminal_spec.lua | 136 ++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+), 6 deletions(-) create mode 100644 tests/terminal_spec.lua diff --git a/lua/sidekick/cli/scrollback.lua b/lua/sidekick/cli/scrollback.lua index fb8164a2..dc086ba9 100644 --- a/lua/sidekick/cli/scrollback.lua +++ b/lua/sidekick/cli/scrollback.lua @@ -127,6 +127,7 @@ function M:open(win_pos) terminal:bo(self.buf) vim.bo[self.buf].bufhidden = "wipe" vim.api.nvim_win_set_buf(terminal.win, self.buf) + terminal:wo() local term = vim.api.nvim_open_term(self.buf, {}) terminal:keys(self.buf) diff --git a/lua/sidekick/cli/terminal.lua b/lua/sidekick/cli/terminal.lua index ff8a6671..381032c7 100644 --- a/lua/sidekick/cli/terminal.lua +++ b/lua/sidekick/cli/terminal.lua @@ -49,6 +49,7 @@ local wo = { cursorcolumn = false, cursorline = false, fillchars = "eob: ", + foldcolumn = "0", list = false, listchars = "tab: ", number = false, @@ -72,7 +73,6 @@ local win_opts = { float = { focusable = true, relative = "editor", - style = "minimal", row = 0.5, col = 0.5, title = " Sidekick ", @@ -81,7 +81,6 @@ local win_opts = { ---@type vim.api.keyset.win_config split = { win = -1, - style = "minimal", }, } @@ -213,6 +212,13 @@ function M:start() callback = fix_cursorline, }) + vim.api.nvim_create_autocmd("BufWinEnter", { + group = self.group, + callback = function() + self:release_win() + end, + }) + local norm_cmd = vim.deepcopy(self.tool.cmd) ---@type string|string[] if vim.fn.has("win32") == 1 then local cmd1 = vim.fn.exepath(norm_cmd[1]) @@ -322,9 +328,32 @@ function M:fix_cursorline() if not self:win_valid() then return end + local wbuf = vim.api.nvim_win_get_buf(self.win) + if wbuf ~= self.buf and not (self.scrollback and wbuf == self.scrollback.buf) then + return -- another buffer took over the window + end self:wo({ cursorline = vim.fn.mode() ~= "t" and vim.api.nvim_get_current_win() == self.win }) end +--- Release the window when another buffer is shown in it (e.g. `:edit file`), +--- so the user keeps the window and hiding the terminal won't close it. +function M:release_win() + if not self:win_valid() then + return + end + local wbuf = vim.api.nvim_win_get_buf(self.win) + if wbuf == self.buf or (self.scrollback and wbuf == self.scrollback.buf) then + return + end + -- winfix{width,height} sticks to the window, so undo it explicitly. + -- All other window options are buffer-local and restored by Neovim itself. + vim.wo[self.win].winfixwidth = false + vim.wo[self.win].winfixheight = false + vim.w[self.win].sidekick_cli = nil + vim.w[self.win].sidekick_session_id = nil + self.win = nil +end + function M:on_ready() self.timer:start(0, SEND_DELAY, function() local next = table.remove(self.send_queue, 1) ---@type string? @@ -377,9 +406,10 @@ function M:open_win() self.win = vim.api.nvim_open_win(self.buf, false, opts) - if opts.vertical then + -- pin the window size, but only when an explicit size was configured (0 = auto) + if opts.vertical and opts.height then vim.wo[self.win].winfixheight = true - else + elseif not opts.vertical and opts.width then vim.wo[self.win].winfixwidth = true end vim.w[self.win].sidekick_cli = self.tool diff --git a/lua/sidekick/config.lua b/lua/sidekick/config.lua index d30e92e8..e6cfa743 100644 --- a/lua/sidekick/config.lua +++ b/lua/sidekick/config.lua @@ -51,8 +51,8 @@ local defaults = { -- Options used when layout is "left"|"bottom"|"top"|"right" ---@type vim.api.keyset.win_config split = { - width = 80, -- set to 0 for default split width - height = 20, -- set to 0 for default split height + width = 80, -- fixed width for the split. Set to 0 for a regular split that resizes with your layout + height = 20, -- fixed height for the split. Set to 0 for a regular split that resizes with your layout }, --- CLI Tool Keymaps (default mode is `t`) ---@type table diff --git a/tests/terminal_spec.lua b/tests/terminal_spec.lua new file mode 100644 index 00000000..ae96f11f --- /dev/null +++ b/tests/terminal_spec.lua @@ -0,0 +1,136 @@ +---@module 'luassert' + +local Config = require("sidekick.config") +local Terminal = require("sidekick.cli.terminal") + +describe("terminal", function() + local orig_number, orig_signcolumn, orig_watch, orig_width + local tmp_files, tmp_bufs + + local function open() + require("sidekick.cli").show({ name = "test" }) + -- cli.show is deferred to the next tick + vim.wait(1000, function() + local t = vim.tbl_values(Terminal.terminals)[1] ---@type sidekick.cli.Terminal? + return t ~= nil and t:is_open() or false + end) + local terminal = vim.tbl_values(Terminal.terminals)[1] ---@type sidekick.cli.Terminal? + assert.is_not_nil(terminal) + assert.is_truthy(terminal:is_open()) + assert.is_truthy(terminal:is_running()) + return terminal + end + + ---@param win integer + local function takeover(win) + local file = vim.fn.tempname() .. ".txt" + vim.fn.writefile({ "test" }, file) + vim.api.nvim_win_call(win, function() + vim.cmd.edit(file) + end) + table.insert(tmp_files, file) + table.insert(tmp_bufs, vim.api.nvim_win_get_buf(win)) + end + + before_each(function() + orig_number, orig_signcolumn = vim.o.number, vim.o.signcolumn + orig_watch, orig_width = Config.cli.watch, Config.cli.win.split.width + tmp_files, tmp_bufs = {}, {} + vim.o.number = true + vim.o.signcolumn = "yes" + Config.cli.watch = false + Config.cli.tools.test = { cmd = { "cat" } } + end) + + after_each(function() + for _, terminal in pairs(vim.tbl_extend("force", {}, Terminal.terminals)) do + terminal:close() + end + vim.wait(100) -- drain deferred cli callbacks before removing the tool + for _, buf in ipairs(tmp_bufs) do + if vim.api.nvim_buf_is_valid(buf) then + vim.api.nvim_buf_delete(buf, { force = true }) + end + end + for _, file in ipairs(tmp_files) do + vim.fn.delete(file) + end + Config.cli.tools.test = nil + Config.cli.watch = orig_watch + Config.cli.win.split.width = orig_width + vim.o.number = orig_number + vim.o.signcolumn = orig_signcolumn + end) + + it("applies window options to the terminal window", function() + local terminal = open() + local expected = { + number = false, + relativenumber = false, + signcolumn = "no", + statuscolumn = "", + winbar = "", + foldcolumn = "0", + winfixwidth = true, + } + for option, value in pairs(expected) do + assert.are.same(value, vim.api.nvim_get_option_value(option, { win = terminal.win }), option) + end + assert.is_not_nil(vim.w[terminal.win].sidekick_cli) + assert.are.same(terminal.id, vim.w[terminal.win].sidekick_session_id) + end) + + it("does not pin the window size for auto-sized splits", function() + Config.cli.win.split.width = 0 + local terminal = open() + assert.are.same(false, vim.api.nvim_get_option_value("winfixwidth", { win = terminal.win })) + assert.are.same(false, vim.api.nvim_get_option_value("winfixheight", { win = terminal.win })) + end) + + it("releases the window when another buffer replaces the terminal", function() + local terminal = open() + local win = terminal.win + takeover(win) + + -- Neovim restores the user's window options for the new buffer + assert.are.same(true, vim.api.nvim_get_option_value("number", { win = win })) + assert.are.same("yes", vim.api.nvim_get_option_value("signcolumn", { win = win })) + -- sidekick undoes what sticks to the window and lets go of it + assert.are.same(false, vim.api.nvim_get_option_value("winfixwidth", { win = win })) + assert.is_nil(vim.w[win].sidekick_cli) + assert.is_nil(vim.w[win].sidekick_session_id) + assert.is_falsy(terminal:is_open()) + -- the session keeps running in the background + assert.is_truthy(terminal:is_running()) + end) + + it("does not re-apply options to a released window", function() + local terminal = open() + local win = terminal.win + + -- keep the window held to exercise the fix_cursorline guard directly + vim.opt.eventignore = "BufWinEnter" + takeover(win) + vim.opt.eventignore = "" + + terminal:fix_cursorline() + assert.are.same(true, vim.api.nvim_get_option_value("number", { win = win })) + + terminal:release_win() + assert.are.same(false, vim.api.nvim_get_option_value("winfixwidth", { win = win })) + assert.is_falsy(terminal:is_open()) + end) + + it("reopens a styled window after the previous one was taken over", function() + local terminal = open() + local old_win = terminal.win + takeover(old_win) + assert.is_falsy(terminal:is_open()) + + terminal:show() + assert.is_truthy(terminal:is_open()) + assert.are_not.equal(old_win, terminal.win) + assert.are.same(false, vim.api.nvim_get_option_value("number", { win = terminal.win })) + assert.are.same(true, vim.api.nvim_get_option_value("winfixwidth", { win = terminal.win })) + end) +end)