Add Dir Bookmarks #112
Add Dir Bookmarks #112
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds directory bookmark functionality to arrow.nvim, allowing users to bookmark and navigate to directories in addition to files. The PR also modernizes the codebase by replacing deprecated Neovim API calls with their current equivalents and adds visual highlighting for split mode operations.
Key changes:
- Introduces a parallel directory bookmark system with dedicated persistence functions and UI support
- Migrates from deprecated
nvim_buf_add_highlightandnvim_buf_set_optionAPIs tonvim_buf_set_extmarkandnvim_set_option_value - Adds new highlight group
ArrowSplitModefor visual feedback during horizontal/vertical split operations
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| lua/arrow/ui.lua | Refactors UI rendering to support dual bookmark types, migrates to extmark-based highlighting, adds bookmark type toggle functionality |
| lua/arrow/persist.lua | Implements directory bookmark persistence with save/load/toggle/clear operations, adds fallback for missing save_key_cached |
| lua/arrow/integration/icons.lua | Adds directory icon support and refactors provider selection logic based on explicit configuration |
| lua/arrow/init.lua | Adds new highlight groups, directory bookmark configuration, and default directory path list |
| lua/arrow/buffer_ui.lua | Updates deprecated API calls to modern equivalents, creates dedicated namespace for buffer highlights |
| README.md | Documents new directory bookmark feature, icon provider configuration, and toggle_bookmark_type mapping |
Comments suppressed due to low confidence (1)
README.md:9
- Corrected spelling: Remove extraneous 'L' from 'bookmarksL:' to 'bookmarks:'.
### Per Project / Global bookmarksL:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| quit = "q", | ||
| remove = "x", -- only used if separate_save_and_remove is true | ||
| next_item = "]", | ||
| prev_item = "[" |
There was a problem hiding this comment.
Missing comma after prev_item = "[" on the previous line. This will cause a syntax error in the Lua table definition.
| prev_item = "[" | |
| prev_item = "[", |
| }, | ||
| opts = { | ||
| show_icons = true, | ||
| icon_provider = 'nvim-web-devicons', -- or 'mini.icons' |
There was a problem hiding this comment.
The icon_provider value 'nvim-web-devicons' does not match the implementation in lua/arrow/integration/icons.lua which expects 'web_dev_icons' (line 31). This mismatch will cause the icon provider selection to fail. Change to 'web_dev_icons' to match the implementation.
| icon_provider = 'nvim-web-devicons', -- or 'mini.icons' | |
| icon_provider = 'web_dev_icons', -- or 'mini.icons' |
| end | ||
|
|
||
| return get_icon_from_mini(file_name) | ||
| end |
There was a problem hiding this comment.
The function does not return a value when provider is neither 'web_dev_icons' nor 'mini'. This can lead to nil values being returned unexpectedly. Add an else clause at the end that either throws an error about invalid provider or provides a default fallback.
| end | |
| end | |
| error("Invalid icon provider: " .. tostring(provider), vim.log.levels.ERROR) |
| local extension = vim.fn.fnamemodify(file_name, ":e") | ||
| local icon, hl_group = webdevicons.get_icon(file_name, extension, { default = true }) | ||
| if vim.fn.isdirectory(file_name) == 1 then | ||
| return "H", "Normal" |
There was a problem hiding this comment.
[nitpick] The hardcoded directory icon 'H' seems inconsistent with typical folder icons. Consider using a proper folder icon (e.g., '' or webdevicons' folder icon) or making it configurable. The choice of 'H' is unclear and may not visually represent a directory well.
| return "H", "Normal" | |
| local icon, hl_group = webdevicons.get_icon(file_name, "", { default = true }) | |
| return icon, hl_group |
| if bookmark_type == "dir" then | ||
| local dir_config = config.getState("dir_bookmark_config") | ||
| action = dir_config and dir_config.open_action |
There was a problem hiding this comment.
If dir_config.open_action is nil, action will be nil, leading to a potential error when trying to call action(fileName, vim.b.filename) at line 459. Add validation to ensure open_action exists or provide a fallback error message.
| { key = mappings.edit, text = string.format("%s Edit Arrow File", mappings.edit) }, | ||
| { key = mappings.clear_all_items, text = string.format("%s Clear All Items", mappings.clear_all_items) }, | ||
| { key = mappings.delete_mode, text = string.format("%s Delete Mode", mappings.delete_mode) }, | ||
| -- { key = mappings.toggle_bookmark_type, text = string.format("%s Switch Mode", mappings.toggle_bookmark_type) }, |
There was a problem hiding this comment.
This commented-out code should either be removed or have a clear explanation why it's disabled. Leaving commented code without context makes maintenance harder.
| -- { key = mappings.toggle_bookmark_type, text = string.format("%s Switch Mode", mappings.toggle_bookmark_type) }, |
| displayIndex = config.getState("index_keys"):sub(i, i) | ||
|
|
||
| vim.api.nvim_buf_add_highlight(buf, -1, "ArrowDeleteMode", i + 3, 0, -1) | ||
| -- vim.api.nvim_buf_add_highlight(buf, -1, "ArrowDeleteMode", i + 3, 0, -1) |
There was a problem hiding this comment.
This commented-out deprecated API call should be removed since it has been replaced with the extmark API. Keeping old code comments clutters the codebase.
| -- vim.api.nvim_buf_add_highlight(buf, -1, "ArrowDeleteMode", i + 3, 0, -1) |
2 participants
compressed.mov
split horizontal/verticalcommands