All going back to categories - #41
Merged
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
Walkthrough
Changes
Sequence DiagramsequenceDiagram
actor User
participant CategoryPicker as "Category Picker"
participant CommandPicker as "Command Picker"
participant Snacks as "Snacks UI"
User->>CategoryPicker: open CommandPalette
Snacks->>CategoryPicker: render (layout: vscode, focus:list)
User->>CategoryPicker: select category
CategoryPicker->>CommandPicker: select_command(commands, on_back)
Snacks->>CommandPicker: render (layout: vscode, focus:list)
alt User presses BS/C-h with empty input
User->>CommandPicker: BS/C-h
CommandPicker->>Snacks: close picker
CommandPicker->>CategoryPicker: call on_back (re-open)
Snacks->>CategoryPicker: render
else User deletes character (input non-empty)
User->>CommandPicker: BS/C-h (delete)
end
opt User selects a command
User->>CommandPicker: select item
CommandPicker->>CommandPicker: if not nil then execute (fn or vim.cmd)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lua/nikero/command_palette.lua (1)
102-109: Remove redundant nil checks.After the guard on line 102,
commandis guaranteed to be non-nil, making thecommand andchecks on lines 104 and 106 redundant.♻️ Simplified conditionals
}, function(command) if not command then return end - if command and type(command[2]) == "function" then + if type(command[2]) == "function" then command[2]() - elseif command and type(command[2]) == "string" then + elseif type(command[2]) == "string" then vim.cmd(command[2]) end end)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/nikero/command_palette.lua` around lines 102 - 109, After the early return "if not command then return end", remove the redundant nil checks in the subsequent branches: replace "if command and type(command[2]) == 'function' then" and "elseif command and type(command[2]) == 'string' then" with checks that rely only on command[2] (e.g., "if type(command[2]) == 'function' then" and "elseif type(command[2]) == 'string' then") so the logic uses the guaranteed non-nil command and directly tests command[2]; keep the existing calls to command[2]() and vim.cmd(command[2]) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lua/nikero/command_palette.lua`:
- Around line 102-109: After the early return "if not command then return end",
remove the redundant nil checks in the subsequent branches: replace "if command
and type(command[2]) == 'function' then" and "elseif command and
type(command[2]) == 'string' then" with checks that rely only on command[2]
(e.g., "if type(command[2]) == 'function' then" and "elseif type(command[2]) ==
'string' then") so the logic uses the guaranteed non-nil command and directly
tests command[2]; keep the existing calls to command[2]() and
vim.cmd(command[2]) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fbd0da90-bf40-47bb-930d-7229896c5298
📒 Files selected for processing (1)
lua/nikero/command_palette.lua
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lua/nikero/command_palette.lua (2)
112-121: Consider extracting shared Snacks layout config.The
{ layout = { preset = "vscode" }, focus = "list" }options are duplicated betweenshow(line 115) andselect_command(lines 72-73). Consider extracting to a module-level constant:local PICKER_LAYOUT = { layout = { preset = "vscode" }, focus = "list" }Then merge into
snacks_optsand use directly inshow. This is minor—current code is perfectly functional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/nikero/command_palette.lua` around lines 112 - 121, Extract the duplicated snacks layout table into a module-level constant (e.g. local PICKER_LAYOUT = { layout = { preset = "vscode" }, focus = "list" }) and replace the inline tables in CommandPalette:show and CommandPalette:select_command by merging or assigning this constant into their snacks_opts; update show to use PICKER_LAYOUT (or vim.tbl_extend if you need to preserve other keys) instead of the inline `{ layout = { preset = "vscode" }, focus = "list" }` so both functions reference the same PICKER_LAYOUT symbol.
6-6: Type annotation should markon_backas optional.The implementation guards against
nilon line 68, so the parameter is effectively optional. Consider updating the annotation:----@field select_command fun(self: CommandPalette, commands: Command[], on_back: fun()) +---@field select_command fun(self: CommandPalette, commands: Command[], on_back?: fun())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/nikero/command_palette.lua` at line 6, The type annotation for CommandPalette.select_command marks the on_back parameter as required but the implementation guards against nil, so make on_back optional in the signature; update the annotation for select_command (the method on the CommandPalette type) to accept on_back as an optional callback (e.g., allow nil) so it matches the runtime guard around on_back in the implementation.
🤖 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/nikero/command_palette.lua`:
- Around line 74-78: The call picker.input:get() in the back_or_delete action is
invalid for Snacks.nvim; change the check to use the correct API (e.g., read the
current input line via picker.input.win:line() or the filter via
picker:filter.pattern) so the condition becomes "if current input is not empty
then return backspace"; update the back_or_delete function to use
picker.input.win:line() or picker:filter.pattern instead of picker.input:get().
---
Nitpick comments:
In `@lua/nikero/command_palette.lua`:
- Around line 112-121: Extract the duplicated snacks layout table into a
module-level constant (e.g. local PICKER_LAYOUT = { layout = { preset = "vscode"
}, focus = "list" }) and replace the inline tables in CommandPalette:show and
CommandPalette:select_command by merging or assigning this constant into their
snacks_opts; update show to use PICKER_LAYOUT (or vim.tbl_extend if you need to
preserve other keys) instead of the inline `{ layout = { preset = "vscode" },
focus = "list" }` so both functions reference the same PICKER_LAYOUT symbol.
- Line 6: The type annotation for CommandPalette.select_command marks the
on_back parameter as required but the implementation guards against nil, so make
on_back optional in the signature; update the annotation for select_command (the
method on the CommandPalette type) to accept on_back as an optional callback
(e.g., allow nil) so it matches the runtime guard around on_back in the
implementation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75d20d5a-cd9c-484f-9c27-7ee30aea5e88
📒 Files selected for processing (1)
lua/nikero/command_palette.lua
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request enhances the
CommandPaletteinlua/nikero/command_palette.luaby improving the user experience when navigating and selecting commands, particularly by adding a "back" functionality and customizing keybindings and layout. The most important changes are grouped below:Navigation and UX improvements:
select_commandmethod now accepts anon_backcallback, enabling users to go back to the previous menu (e.g., the category list) when pressing backspace or a specific key combination. [1] [2]go_backfunction and integrated it into the command picker, so that pressing<BS>(backspace) or<C-h>(Ctrl+H) in either the input or list view will trigger the back action if the input is empty.snacksoptions to provide a more "VSCode-like" experience and ensure consistent navigation behavior.Refactoring and API changes:
showmethod so that when a user navigates back from a command group, the category selection menu is shown again by passing a callback toselect_command.