Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRelocates a sentinel check in vimcmd entry processing, replaces verbose keymap parsing with Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lua/fzf-lua/path.lua (1)
669-691:⚠️ Potential issue | 🟡 MinorRemove unused
optsargument inactions.luacall tokeymap_to_entry.The function signature at
path.lua:670accepts onlystr, but the call site inactions.lua:694still passes a secondoptsargument:local entry = path.keymap_to_entry(selected[1], opts)Change to:
local entry = path.keymap_to_entry(selected[1])The call site in
previewer/builtin.lua:1898correctly uses the single-argument signature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/fzf-lua/path.lua` around lines 669 - 691, The call to path.keymap_to_entry is passing an unused second argument; update the call in actions.lua (the line that currently does local entry = path.keymap_to_entry(selected[1], opts)) to pass only the string: local entry = path.keymap_to_entry(selected[1]); ensure you only remove the extra opts argument and verify there are no other call sites passing a second parameter to M.keymap_to_entry (function keymap_to_entry in path.lua) so signatures remain consistent.
🧹 Nitpick comments (1)
lua/fzf-lua/path.lua (1)
682-690: Implementation usesvim.fn.mapargeffectively.The switch from verbose map parsing to
vim.fn.maparg(keymap, mode, false, true)is cleaner and more reliable. The Lua callback detection viadebug.getinfocorrectly extracts source file and line number for jump-to-definition functionality.Minor note: Line 689's comment
-- if entry then return M.entry_to_file(entry, opts) endappears to be leftover from the refactor and could be removed.🧹 Remove stale comment
end end - -- if entry then return M.entry_to_file(entry, opts) end return { mode = mode, key = keymap, vmap = vim.inspect(vmap) } end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/fzf-lua/path.lua` around lines 682 - 690, Remove the leftover commented code on the final return path: delete the stale comment line "-- if entry then return M.entry_to_file(entry, opts) end" so the function only uses the vmap / debug.getinfo logic; ensure nothing else is dependent on M.entry_to_file and leave the rest of the block (vmap = vim.fn.maparg(...), debug.getinfo handling, and the final return { mode = mode, key = keymap, vmap = vim.inspect(vmap) }) unchanged.
🤖 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/providers/colorschemes.lua`:
- Around line 102-104: The fallback path for utils.with doesn't enforce
verbose=0 so highlight output can be incorrect; patch the highlights retrieval
(the utils.with call that invokes vim.api.nvim_exec2) to ensure verbose=0 is
always applied: if vim._with is available use it, otherwise temporarily save
vim.o.verbose, set vim.o.verbose = 0, call vim.api.nvim_exec2("highlight", {
output = true }), then restore the original vim.o.verbose; keep this change
local to the highlights call so utils.with and the highlights variable behave
the same across Neovim versions.
---
Outside diff comments:
In `@lua/fzf-lua/path.lua`:
- Around line 669-691: The call to path.keymap_to_entry is passing an unused
second argument; update the call in actions.lua (the line that currently does
local entry = path.keymap_to_entry(selected[1], opts)) to pass only the string:
local entry = path.keymap_to_entry(selected[1]); ensure you only remove the
extra opts argument and verify there are no other call sites passing a second
parameter to M.keymap_to_entry (function keymap_to_entry in path.lua) so
signatures remain consistent.
---
Nitpick comments:
In `@lua/fzf-lua/path.lua`:
- Around line 682-690: Remove the leftover commented code on the final return
path: delete the stale comment line "-- if entry then return
M.entry_to_file(entry, opts) end" so the function only uses the vmap /
debug.getinfo logic; ensure nothing else is dependent on M.entry_to_file and
leave the rest of the block (vmap = vim.fn.maparg(...), debug.getinfo handling,
and the final return { mode = mode, key = keymap, vmap = vim.inspect(vmap) })
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1bb3f6ab-ad20-4da2-9265-9c8c1b159d50
📒 Files selected for processing (5)
lua/fzf-lua/actions.lualua/fzf-lua/path.lualua/fzf-lua/previewer/builtin.lualua/fzf-lua/providers/colorschemes.lualua/fzf-lua/types.lua
|
|
1 participant
Summary by CodeRabbit
Bug Fixes
Improvements
Documentation