fix: layout shift#18
Conversation
There was a problem hiding this comment.
Pull request overview
Adds regression coverage for a “layout shift” scenario in the Zen integrations test suite, ensuring side integrations don’t disturb existing top/bottom splits and correcting expected split sizing.
Changes:
- Added new integration tests covering preservation of existing top/bottom integrations when opening left/right side integrations.
- Added a regression test for cursor placement when closing a stacked top split above a top integration.
- Updated expected right-side split widths to match the new sizing behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| child.cmd("Git commit --allow-empty") | ||
|
|
||
| -- close the commit editor (abort the commit) | ||
| child.cmd("bdelete!") | ||
|
|
There was a problem hiding this comment.
This test invokes :Git commit --allow-empty, which can start a real git commit process and may leave persistent state in the checkout (e.g., .git/index.lock) or fail due to missing git user identity. That can make the CI test suite flaky and may impact later test cases executed in the same repo. Prefer simulating the “stacked top split” deterministically (e.g., create an aboveleft split above the fugitive window and close it) without running an actual commit.
There was a problem hiding this comment.
@copilot i dont want fakes/mocks i want real behaviour init a git repo of needed.
There was a problem hiding this comment.
Done in d288c90. The test now initializes a real temporary git repo inside the child Neovim (via vim.fn.system), sets user.name and user.email locally, and chdirs into it — so :Git commit --allow-empty runs against a genuine git repo with proper identity, no mocks needed.
Agent-Logs-Url: https://github.com/sand4rt/zen.nvim/sessions/a652929b-24dc-4980-b03f-05568d133994 Co-authored-by: sand4rt <17591696+sand4rt@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for _, integration in ipairs(opts[position] or {}) do | ||
| ---@diagnostic disable-next-line: undefined-field | ||
| if type(integration) == "table" and is_filetype(filetype, integration.filetype) then | ||
| if type(integration) == "table" and is_filetype(filetype, integration.filetype) then |
There was a problem hiding this comment.
There’s an extra indentation level on the if type(integration) == "table" ... line in is_buff_integration, which makes the block harder to read and looks like an accidental formatting regression. Running StyLua (or aligning indentation manually) should fix it.
| if type(integration) == "table" and is_filetype(filetype, integration.filetype) then | |
| if type(integration) == "table" and is_filetype(filetype, integration.filetype) then |
| state[vim.api.nvim_get_current_tabpage()].left = create_window("left") | ||
| vim.cmd("wincmd l") | ||
| end | ||
|
|
||
| local right_file_types = { "dapui_scopes", "neotest-summary", "zen-right" } | ||
| local right_file_types = { "zen-right" } | ||
| for _, integration in ipairs(opts.right) do | ||
| if type(integration) == "table" and integration.filetype ~= "*" then | ||
| append_filetype(right_file_types, integration.filetype) | ||
| end | ||
| end | ||
| remove_file_type(right_file_types, file_type) | ||
| if not filetypes_visible(right_file_types) then | ||
| state[vim.api.nvim_get_current_tabpage()].right = create_window("right") | ||
| vim.cmd("wincmd h") |
There was a problem hiding this comment.
state[vim.api.nvim_get_current_tabpage()] can be nil in tabs where the VimEnter/TabNew autocmd returned early (e.g., window too small or get_vsplits() >= 2). In that case, indexing state[...].left/right here will throw. Consider ensuring the per-tab entry exists before assigning (e.g., initialize state[tab] with { left = -1, right = -1 } when missing, or add a small helper like ensure_state(tab)).
07cf97c to
f8bfa77
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| local function get_main_width() | ||
| local width = opts.main and opts.main.width | ||
| if type(width) == "function" then | ||
| width = width() | ||
| end | ||
| if type(width) ~= "number" then | ||
| return default_width | ||
| end | ||
| return width | ||
| local width = options.main and options.main.width | ||
| if type(width) == "function" then width = width() end | ||
| return type(width) == "number" and width or options.main.width | ||
| end |
There was a problem hiding this comment.
get_main_width() can return a non-number when options.main.width is a string (e.g. vim.wo.colorcolumn as suggested in README) or when a width function returns a non-number. That will break arithmetic in get_padding_width() and comparisons like vim.o.columns <= get_main_width(). Consider normalizing to a number (e.g. tonumber(...) / parse colorcolumn) and falling back to the default width (148) when the value can’t be coerced to a number.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
65b3251 to
5898cc8
Compare
Closes: #12