Conversation
This seems regression of abacb06. Before that commit, we actually rely on an old vim behavior: https://github.com/neovim/neovim/blob/5b0ad4a0607498616b984c63ae1a6903cb835c66/src/nvim/window.c#L3036 When close a window, vim try to avoid to focus a quickfix window: (even your "last accessed window" is alraedy a quickfix window...) > If the cursor goes to the preview or the quickfix window, try > finding another window to go to.
📝 WalkthroughWalkthroughA single line was added to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lua/fzf-lua/actions.lua`:
- Line 207: The current fallback blindly sets the current window using
vim.fn.winnr("#") via vim.api.nvim_set_current_win even if that alternate window
is a quickfix/preview or otherwise unsuitable; change the logic in the block
using vim.api.nvim_set_current_win(...) to validate the target window first:
resolve the candidate window id (from vim.fn.winnr("#")/win_getid), check the
associated buffer's buftype (vim.api.nvim_buf_get_option(buf, "buftype")) and
window properties (vim.api.nvim_win_get_config or win_gettype) to reject
quickfix/preview/floating windows, and if rejected iterate through
vim.api.nvim_list_wins() to pick the first suitable edit window (buftype == ""
and not preview/quickfix/floating) before calling vim.api.nvim_set_current_win;
if no suitable window is found, do not switch windows. Ensure you update the
code around vim.api.nvim_set_current_win, vim.bo.bt, vim.fn.winnr, and win_getid
references.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| ---@param bufedit boolean? | ||
| ---@return string? | ||
| M.vimcmd_entry = function(vimcmd, selected, opts, bufedit) | ||
| vim.api.nvim_set_current_win(vim.bo.bt == "quickfix" and vim.fn.win_getid(vim.fn.winnr("#")) or 0) |
There was a problem hiding this comment.
Window fallback still allows preview/quickfix targets
Line 207 only checks whether the current window is quickfix, then blindly switches to winnr("#"). If the alternate window is preview/quickfix (or not a good edit target), edit actions can still run in the wrong window.
💡 Suggested fix
- vim.api.nvim_set_current_win(vim.bo.bt == "quickfix" and vim.fn.win_getid(vim.fn.winnr("#")) or 0)
+ local function is_edit_target(winid)
+ if not winid or winid == 0 or not vim.api.nvim_win_is_valid(winid) then
+ return false
+ end
+ local buf = vim.api.nvim_win_get_buf(winid)
+ local bt = vim.api.nvim_get_option_value("buftype", { buf = buf })
+ local is_preview = vim.api.nvim_get_option_value("previewwindow", { win = winid })
+ return bt ~= "quickfix" and not is_preview
+ end
+
+ if vim.bo.bt == "quickfix" or vim.wo.previewwindow then
+ local target = vim.fn.win_getid(vim.fn.winnr("#"))
+ if not is_edit_target(target) then
+ for _, winid in ipairs(vim.api.nvim_tabpage_list_wins(0)) do
+ if is_edit_target(winid) then
+ target = winid
+ break
+ end
+ end
+ end
+ if is_edit_target(target) then
+ vim.api.nvim_set_current_win(target)
+ end
+ end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| vim.api.nvim_set_current_win(vim.bo.bt == "quickfix" and vim.fn.win_getid(vim.fn.winnr("#")) or 0) | |
| local function is_edit_target(winid) | |
| if not winid or winid == 0 or not vim.api.nvim_win_is_valid(winid) then | |
| return false | |
| end | |
| local buf = vim.api.nvim_win_get_buf(winid) | |
| local bt = vim.api.nvim_get_option_value("buftype", { buf = buf }) | |
| local is_preview = vim.api.nvim_get_option_value("previewwindow", { win = winid }) | |
| return bt ~= "quickfix" and not is_preview | |
| end | |
| if vim.bo.bt == "quickfix" or vim.wo.previewwindow then | |
| local target = vim.fn.win_getid(vim.fn.winnr("#")) | |
| if not is_edit_target(target) then | |
| for _, winid in ipairs(vim.api.nvim_tabpage_list_wins(0)) do | |
| if is_edit_target(winid) then | |
| target = winid | |
| break | |
| end | |
| end | |
| end | |
| if is_edit_target(target) then | |
| vim.api.nvim_set_current_win(target) | |
| end | |
| end |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lua/fzf-lua/actions.lua` at line 207, The current fallback blindly sets the
current window using vim.fn.winnr("#") via vim.api.nvim_set_current_win even if
that alternate window is a quickfix/preview or otherwise unsuitable; change the
logic in the block using vim.api.nvim_set_current_win(...) to validate the
target window first: resolve the candidate window id (from
vim.fn.winnr("#")/win_getid), check the associated buffer's buftype
(vim.api.nvim_buf_get_option(buf, "buftype")) and window properties
(vim.api.nvim_win_get_config or win_gettype) to reject quickfix/preview/floating
windows, and if rejected iterate through vim.api.nvim_list_wins() to pick the
first suitable edit window (buftype == "" and not preview/quickfix/floating)
before calling vim.api.nvim_set_current_win; if no suitable window is found, do
not switch windows. Ensure you update the code around
vim.api.nvim_set_current_win, vim.bo.bt, vim.fn.winnr, and win_getid references.
There was a problem hiding this comment.
Literally rewrite in lua...
There was a problem hiding this comment.
Not sure if we need that complex.
There was a problem hiding this comment.
@phanen Fair point — Neovim's own window selection logic (referenced in the PR description via window.c) already avoids landing on preview/quickfix windows when resolving the alternate window, so reimplementing those heuristics in Lua is redundant overhead. The one-liner is the right call.
(∩^o^)⊃━☆ﾟ.*･｡ﾟ
✏️ Learnings added
Learnt from: phanen
Repo: ibhagwan/fzf-lua PR: 2673
File: lua/fzf-lua/actions.lua:207-207
Timestamp: 2026-04-16T16:45:08.605Z
Learning: In fzf-lua (lua/fzf-lua/actions.lua), `M.vimcmd_entry` uses `vim.fn.win_getid(vim.fn.winnr("#"))` to resolve the alternate window when the current window is quickfix. Do not suggest reimplementing window-avoidance heuristics (skip preview/quickfix) in Lua — Neovim's own C-level window selection logic already handles this. Trust the built-in behavior.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ibhagwan
Repo: ibhagwan/fzf-lua PR: 2634
File: lua/fzf-lua/win.lua:240-277
Timestamp: 2026-03-27T17:15:12.021Z
Learning: In fzf-lua (lua/fzf-lua/win.lua), the SIGWINCH handler registered via `FzfWin.on_SIGWINCH` and `toggle_preview()` operate in completely separate async phases across separate processes (Neovim and fzf). The SIGWINCH callback only returns a fzf action string to the external fzf process — it never calls `toggle_preview()`. Conversely, `toggle_preview()` only sends POSIX signal 28 asynchronously via `self:SIGWINCH(...)`; fzf receives the signal later, fires its `resize` event, and then the handler runs. There is no synchronous re-entry path between these two sides. Do not flag double-apply / re-entry bugs in this flow.
|
To elaborate, this fix: if we use picker when focus on a quickfix window, it will change the quickfix window's buffer. I'm not sure anyone like this... if anyone want, this can be opt-in by custom something like |
I agree we shouldn’t change the quick fix buffer, more so we should match the behavior that existed before. Maybe it’s better the fix should be in |
I'll consider it, it work, but due to the vim behavior, when abort the picker with ctrl-c/esc, the cursor won't leave in quickfix window. I believe there should be a better way. |
|
Actually this is not only about quickfix and vim behavior. I think we also don't want to override some other buffer like outline/file-explorer/codecompanion... Currently I use this fa.vimcmd_entry = (function(orig)
return function(...)
api.nvim_set_current_win(vim.bo.bt ~= '' and fn.win_getid(fn.winnr('#')) or 0)
return orig(...)
end
end)(fa.vimcmd_entry) |
Agree, some buffers should remain
if this works well for you we can use it, even if we have to add some hardcoded buffer types / filetypes for exclusion in the future that’s ok. |
3 participants
This seems regression of abacb06.
Before that commit, we actually rely on an old vim behavior:
https://github.com/neovim/neovim/blob/5b0ad4a0607498616b984c63ae1a6903cb835c66/src/nvim/window.c#L3036
When close a window, vim try to avoid to focus a quickfix window:
(even your "last accessed window" is alraedy a quickfix window...)
Summary by CodeRabbit