Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRefactored buffer-opening/control flow in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FZF as "fzf window"
participant Actions as "actions.lua"
participant Buffer as "Vim Buffer"
participant Window as "Vim Window / Opener"
User->>FZF: selects entry
FZF->>Actions: call vimcmd_entry(entry, opts)
Actions->>Actions: compute layoutcmd (split/open/ctag handling)
Actions->>Buffer: call vim.fn.bufadd(relpath) -> bufnr
Actions->>Buffer: check is_curbuf / can_replace_buf
alt replaceable
Actions->>Window: set_buf(bufnr) (may preserve/restore bufhidden)
else not replaceable
Actions->>Window: open new buffer/window via layoutcmd
end
Actions->>Window: run opener (open/split/quickfix) and set cursor/search
Window-->>User: buffer opened / command executed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
|
this should be a bug: |
|
On another note the CI is so flakey it’s so annoying I have to run jobs again and again to see green lol |
fix: set_buf fail but no errmsg when buf is modified && unamed && empty fix: remove opts.silent guard for non-annoying msg
will_replace_buf seems to mean "wipe unnamed empty buffers"
`:call nvim_buf_set_text(0, 0, 0, 0, 0, []) | FzfLua files`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Around line 323-325: The comparator in lua/fzf-lua/actions.lua compares a.col
directly which can nil-crash; update the sort comparator (the block comparing
a.filename, a.lnum, a.col) to normalize missing column values before comparing
(e.g., set local ac = a.col or 0 and local bc = b.col or 0 or use tonumber on
each) and then compare ac < bc; keep the earlier filename and lnum comparisons
unchanged and ensure the comparator always returns a boolean.
- Line 258: The early-return that used to guard terminal buffers was reduced to
only checking filetype (vim.bo.ft == "fzf"); restore a terminal-buffer guard so
the function exits for terminal buffers too. Update the conditional in
lua/fzf-lua/actions.lua (the if vim.bo.ft == "fzf" then return end line) to also
check buftype (e.g., if vim.bo.ft == "fzf" or vim.bo.buftype == "terminal" then
return end) so terminal-mode incompatible operations are skipped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| if utils.is_term_buffer(0) and vim.bo.ft == "fzf" then | ||
| return | ||
| end | ||
| if vim.bo.ft == "fzf" then return end |
There was a problem hiding this comment.
Terminal-buffer guard was narrowed too much
Line 258 now only checks vim.bo.ft == "fzf". This can let execution continue in other terminal buffers and hit terminal-mode-incompatible operations later in the function. Re-adding a terminal-buffer guard avoids this regression.
💡 Suggested fix
- if vim.bo.ft == "fzf" then return end
+ if vim.bo.ft == "fzf" or utils.is_term_buffer(0) then return 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.
| if vim.bo.ft == "fzf" then return end | |
| if vim.bo.ft == "fzf" or utils.is_term_buffer(0) then return 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 258, The early-return that used to guard
terminal buffers was reduced to only checking filetype (vim.bo.ft == "fzf");
restore a terminal-buffer guard so the function exits for terminal buffers too.
Update the conditional in lua/fzf-lua/actions.lua (the if vim.bo.ft == "fzf"
then return end line) to also check buftype (e.g., if vim.bo.ft == "fzf" or
vim.bo.buftype == "terminal" then return end) so terminal-mode incompatible
operations are skipped.
| if a.filename ~= b.filename then return a.filename < b.filename end | ||
| if a.lnum ~= b.lnum then return a.lnum < b.lnum end | ||
| return a.col < b.col |
There was a problem hiding this comment.
Potential sort crash when col is missing
At Line 325, return a.col < b.col can error when either side is nil (common for entries without a column). Please normalize col before sorting.
💡 Suggested fix
- return a.col < b.col
+ return (a.col or 0) < (b.col or 0)📝 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.
| if a.filename ~= b.filename then return a.filename < b.filename end | |
| if a.lnum ~= b.lnum then return a.lnum < b.lnum end | |
| return a.col < b.col | |
| if a.filename ~= b.filename then return a.filename < b.filename end | |
| if a.lnum ~= b.lnum then return a.lnum < b.lnum end | |
| return (a.col or 0) < (b.col or 0) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lua/fzf-lua/actions.lua` around lines 323 - 325, The comparator in
lua/fzf-lua/actions.lua compares a.col directly which can nil-crash; update the
sort comparator (the block comparing a.filename, a.lnum, a.col) to normalize
missing column values before comparing (e.g., set local ac = a.col or 0 and
local bc = b.col or 0 or use tonumber on each) and then compare ac < bc; keep
the earlier filename and lnum comparisons unchanged and ensure the comparator
always returns a boolean.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/api_spec.lua (1)
207-209: Remove redundant load wait in this__after_openhook.On Line 207–209, this wait duplicates the framework-level synchronization:
helpers.FzfLua.*already waits for_G._fzf_load_called == truebefore running__after_open(seelua/fzf-lua/test/helpers.lua, around Line 334 and Line 350). Keeping both adds avoidable delay and can worsen timing flakiness.♻️ Proposed simplification
- __after_open = function() - child.wait_until(function() return child.lua_get([[_G._fzf_load_called]]) == true end) - end,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/api_spec.lua` around lines 207 - 209, The __after_open hook currently calls child.wait_until checking child.lua_get([[_G._fzf_load_called]]), which duplicates the framework-level synchronization already performed by helpers.FzfLua.*; remove that child.wait_until(...) block from the __after_open hook in tests/api_spec.lua so the test relies on the existing helper wait and avoids redundant delays and flakiness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/api_spec.lua`:
- Around line 207-209: The __after_open hook currently calls child.wait_until
checking child.lua_get([[_G._fzf_load_called]]), which duplicates the
framework-level synchronization already performed by helpers.FzfLua.*; remove
that child.wait_until(...) block from the __after_open hook in
tests/api_spec.lua so the test relies on the existing helper wait and avoids
redundant delays and flakiness.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0dc4a250-e6c3-40a3-8483-238aea49733c
📒 Files selected for processing (3)
lua/fzf-lua/test/helpers.luatests/api_spec.luatests/win_spec.lua
✅ Files skipped from review due to trivial changes (1)
- tests/win_spec.lua
🚧 Files skipped from review as they are similar to previous changes (1)
- lua/fzf-lua/test/helpers.lua
ci: use ubuntu-slim on small job
|
Move lint/vimdoc job to ubuntu-slim. https://docs.github.com/en/actions/reference/runners/github-hosted-runners#single-cpu-runners |
2 participants
Summary by CodeRabbit