Add customizable keymap for window_navigation and scrolling #79
Conversation
WalkthroughWindow navigation and scrolling keymap configs changed from booleans to nested tables with an Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Config as Config Parser
participant Keymaps as Keymaps Module
participant Neovim as Neovim API
User->>Config: supply config (may include nested keymaps)
Config->>Config: merge with default_config
Config->>Config: validate keymaps.window_navigation & keymaps.scrolling shapes
Config-->>Keymaps: return parsed config
alt window_navigation.enabled == true
Keymaps->>Neovim: set mappings (left, down, up, right) from config
else window_navigation.disabled
Keymaps-->>Neovim: skip window navigation mappings
end
alt scrolling.enabled == true
Keymaps->>Neovim: set terminal mappings (page_down, page_up) from config
else scrolling.disabled
Keymaps-->>Neovim: skip scrolling mappings
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
TheoBrigitte
force-pushed
the
window-navigation-keymap
branch
from
8ea546f to
90532ef
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)lua/claude-code/config.lua(3 hunks)lua/claude-code/keymaps.lua(2 hunks)tests/spec/config_validation_spec.lua(2 hunks)
🔇 Additional comments (7)
README.md (1)
149-160: LGTM! Documentation clearly reflects the new keymap structure.The documentation accurately describes the new nested configuration with
enabledflags and explicit key bindings for bothwindow_navigationandscrolling.tests/spec/config_validation_spec.lua (1)
150-162: LGTM! Test correctly validates the new nested structure.The test properly validates that
keymaps.window_navigation.enabledmust be a boolean and correctly mutates the nested property path.lua/claude-code/keymaps.lua (2)
112-141: LGTM! Window navigation keymaps correctly use the new config structure.The enabled check and terminal mode mappings properly reference the nested
config.keymaps.window_navigationfields.
174-190: LGTM! Scrolling keymaps correctly use the new config structure.The enabled check and scrolling mappings properly reference the nested
config.keymaps.scrollingfields.lua/claude-code/config.lua (3)
45-48: LGTM! Type annotations accurately reflect the new structure.The documentation clearly describes the nested table structure with
enabledflags and key bindings.
136-147: LGTM! Default configuration properly initializes the new nested structures.The default values provide sensible keybindings that match standard Vim navigation patterns.
366-404: Validation logic is comprehensive.After fixing the error messages mentioned in separate comments, the validation correctly covers all required fields for both
window_navigationandscrollingconfigurations.
| if type(keymaps.window_navigation.enabled) ~= 'boolean' then | ||
| return false, 'keymaps.window_navigation.enabled must be a table' | ||
| end |
There was a problem hiding this comment.
Fix incorrect error message.
Line 371 validates that enabled must be a boolean, but the error message incorrectly states "must be a table".
Apply this diff to fix the error message:
if type(keymaps.window_navigation.enabled) ~= 'boolean' then
- return false, 'keymaps.window_navigation.enabled must be a table'
+ return false, 'keymaps.window_navigation.enabled must be a boolean'
end🤖 Prompt for AI Agents
In lua/claude-code/config.lua around lines 370 to 372, the validation checks
whether keymaps.window_navigation.enabled is a boolean but the error message
incorrectly says "must be a table"; update the returned error string to state
"keymaps.window_navigation.enabled must be a boolean" so the message matches the
validation and aids correct debugging.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lua/claude-code/config.lua (2)
370-372: Fix incomplete error message.Line 371 validates that
enabledmust be a boolean, but the error message is incomplete - it says just "boolean" instead of following the pattern used elsewhere ("must be a boolean").Apply this diff to fix the error message:
if type(keymaps.window_navigation.enabled) ~= 'boolean' then - return false, 'keymaps.window_navigation.enabled boolean' + return false, 'keymaps.window_navigation.enabled must be a boolean' end
390-392: Fix typo in error message.Line 391 has a typo: "btable" should be "table".
Apply this diff to fix the typo:
if type(keymaps.scrolling) ~= 'table' then - return false, 'keymaps.scrolling must be a btable' + return false, 'keymaps.scrolling must be a table' end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lua/claude-code/config.lua(3 hunks)
🔇 Additional comments (4)
lua/claude-code/config.lua (4)
136-142: LGTM!The default window navigation keymaps are well-chosen, using standard vim motion keys that users expect.
143-147: LGTM!The default scrolling keymaps use standard vim scroll keys that users expect.
374-388: LGTM!The validation logic correctly checks all four directional key bindings with consistent error messages.
394-404: LGTM!The validation logic correctly checks the enabled flag and both scrolling key bindings with consistent error messages.
| -- @field window_navigation table|nil Window navigation keymaps | ||
| -- @field window_navigation.enabled boolean Enable window navigation keymaps | ||
| -- @field scrolling table|nil Scrolling keymaps | ||
| -- @field scrolling.enabled boolean Enable scrolling keymaps |
There was a problem hiding this comment.
Documentation is incomplete for window_navigation and scrolling fields.
The documentation only mentions the enabled field but omits the actual key binding fields. Users need to know about left, down, up, right for window_navigation and page_down, page_up for scrolling.
Apply this diff to complete the documentation:
-- @field window_navigation table|nil Window navigation keymaps
-- @field window_navigation.enabled boolean Enable window navigation keymaps
+-- @field window_navigation.left string Keymap to move to left window
+-- @field window_navigation.down string Keymap to move to down window
+-- @field window_navigation.up string Keymap to move to up window
+-- @field window_navigation.right string Keymap to move to right window
-- @field scrolling table|nil Scrolling keymaps
-- @field scrolling.enabled boolean Enable scrolling keymaps
+-- @field scrolling.page_down string Keymap to scroll down one page
+-- @field scrolling.page_up string Keymap to scroll up one page🤖 Prompt for AI Agents
In lua/claude-code/config.lua around lines 45 to 48, the docblock for
window_navigation and scrolling only documents the enabled boolean; update the
documentation to list the actual key binding fields: for window_navigation add
@field window_navigation.left string|nil, @field window_navigation.down
string|nil, @field window_navigation.up string|nil and @field
window_navigation.right string|nil; for scrolling add @field scrolling.page_down
string|nil and @field scrolling.page_up string|nil. Keep the existing enabled
fields and ensure the types and brief descriptions mirror the style of the
surrounding docs (e.g., indicate they are keymap strings and optional).
1 participant
Pull Request
Description
This PR provide the capability to set custom key mapping for
window_navigationandscrollingconfiguration sections.This helps keep a consistent motion when one is using different key mapping for motion as the default one provided here.
Type of Change
Please check the options that are relevant:
Checklist
Please check all that apply:
Additional Notes
Add any other context about the PR here.
Summary by CodeRabbit