-
Notifications
You must be signed in to change notification settings - Fork 4
luasnip-again #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
luasnip-again #70
Conversation
WalkthroughKey bindings for completion and snippet navigation in the blink and luasnip plugins were updated. Custom snippet navigation functions were removed, and snippet loading was switched to a directory-based approach. A new gitcommit snippet module was added, and a user command for listing snippets was introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Neovim
participant Blink
participant LuaSnip
User->>Neovim: Presses new completion/snippet keys
Neovim->>Blink: Triggers completion navigation (e.g., <C-l>, <C-h>)
Blink-->>Neovim: Provides completion suggestions
User->>Neovim: Presses snippet expansion key (<C-k>)
Neovim->>LuaSnip: Expands or jumps in snippet
User->>Neovim: Executes :SnipList
Neovim->>LuaSnip: Lists available snippets for filetype
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
plugins/nobbz/lua/nobbz/luasnip.lua (1)
29-29: Path concatenation lacks a trailing slash
script_path("luasnip")returns “…/luasnip” (no/).
luasnip.loaders.from_luaaccepts both styles, yet a trailing slash avoids edge-cases when the parent path does not include one.-lualoader.load({ paths = script_path("luasnip"), }) +lualoader.load({ paths = script_path("luasnip/"), })plugins/nobbz/lua/nobbz/luasnip/gitcommit.lua (1)
1-1: Remove stray debug print
vim.print("luasnip loading")is useful while developing, but it pollutes the message area every time snippets are (re)loaded.-vim.print("luasnip loading")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/nobbz/lua/nobbz/blink.lua(1 hunks)plugins/nobbz/lua/nobbz/luasnip.lua(1 hunks)plugins/nobbz/lua/nobbz/luasnip/gitcommit.lua(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`plugins/**/*.lua`: Please ensure all of the following: - Plugins are lazyloaded if possible through `nobbz.lazy` - `local` has to be used - coding follows the [luarocks styleguide...
plugins/**/*.lua: Please ensure all of the following:
- Plugins are lazyloaded if possible through
nobbz.lazylocalhas to be used- coding follows the luarocks styleguide, but with 2 spaces indentation
plugins/nobbz/lua/nobbz/blink.luaplugins/nobbz/lua/nobbz/luasnip/gitcommit.luaplugins/nobbz/lua/nobbz/luasnip.lua
🔇 Additional comments (2)
plugins/nobbz/lua/nobbz/luasnip.lua (1)
8-10: Potential key-mapping collision with<C-k>
<C-k>is bound here toluasnip.expand, but inblink.luait is also mapped toscroll_documentation_up.
Verify which plugin wins in Insert/Select mode — the losing mapping becomes unreachable.If both are required, consider a different expansion key (eg.
<C-e>which you freed in blink).plugins/nobbz/lua/nobbz/blink.lua (1)
60-67: Key-binding overlap:<C-k>now doubles as snippet expansion and doc-scroll
luasnip.luabinds<C-k>toluasnip.expand; here the same chord scrolls documentation.
Unless blink’s higher precedence disables snippet expansion, users will get unpredictable behaviour.Confirm precedence or remap one of the functions.
| local function list_snips() | ||
| local ft_list = luasnip.available()[vim.o.filetype] | ||
| local ft_snips = {} | ||
| for _, item in pairs(ft_list) do | ||
| ft_snips[item.trigger] = item.desc | ||
| end | ||
| vim.print(ft_snips) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
list_snips() crashes when no snippets exist for the current filetype
luasnip.available()[vim.o.filetype] can be nil; calling pairs() on nil raises an error.
- local ft_list = luasnip.available()[vim.o.filetype]
+ local ft_list = luasnip.available()[vim.o.filetype] or {}Add a short guard to keep the command safe in empty buffers.
📝 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.
| local function list_snips() | |
| local ft_list = luasnip.available()[vim.o.filetype] | |
| local ft_snips = {} | |
| for _, item in pairs(ft_list) do | |
| ft_snips[item.trigger] = item.desc | |
| end | |
| vim.print(ft_snips) | |
| local function list_snips() | |
| local ft_list = luasnip.available()[vim.o.filetype] or {} | |
| local ft_snips = {} | |
| for _, item in pairs(ft_list) do | |
| ft_snips[item.trigger] = item.desc | |
| end | |
| vim.print(ft_snips) |
🤖 Prompt for AI Agents
In plugins/nobbz/lua/nobbz/luasnip.lua around lines 18 to 24, the function
list_snips() crashes because it calls pairs() on nil when no snippets exist for
the current filetype. To fix this, add a guard clause that checks if ft_list is
nil before the for loop, and if so, return early or set ft_list to an empty
table to prevent the error.
| local function script_path(suffix) | ||
| local path = debug.getinfo(2, "S").source:sub(2):match("(.*/)") | ||
| if suffix then return path .. suffix end | ||
| return path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
script_path() uses the wrong stack-level and breaks when invoked indirectly
debug.getinfo(2, 'S') resolves to the caller of script_path, which is fine for the one internal call in this file, but it fails if the helper is reused from another module (it will then resolve the foreign caller’s path).
Using level 1 (the current function) is safer and matches the usual pattern.
-local function script_path(suffix)
- local path = debug.getinfo(2, "S").source:sub(2):match("(.*/)")
+local function script_path(suffix)
+ local path = debug.getinfo(1, "S").source:sub(2):match("(.*/)")📝 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.
| local function script_path(suffix) | |
| local path = debug.getinfo(2, "S").source:sub(2):match("(.*/)") | |
| if suffix then return path .. suffix end | |
| return path | |
| local function script_path(suffix) | |
| local path = debug.getinfo(1, "S").source:sub(2):match("(.*/)") | |
| if suffix then return path .. suffix end | |
| return path |
🤖 Prompt for AI Agents
In plugins/nobbz/lua/nobbz/luasnip.lua around lines 12 to 15, the script_path
function uses debug.getinfo with stack level 2, which points to the caller of
script_path and causes incorrect path resolution when called indirectly. Change
the stack level argument from 2 to 1 in debug.getinfo to refer to the current
function itself, ensuring the path is always correctly resolved regardless of
the caller.
| return { | ||
| s({ trig = "sc", desc = "short-cut ticket", }, | ||
| { t("[SC-"), i(1), t("]"), i(2), }), | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undefined identifiers s, t, i – snippet file will error at load time
luasnip does not expose these globals by default.
Declare them locally:
-return {
- s({ trig = "sc", desc = "short-cut ticket", },
- { t("[SC-"), i(1), t("]"), i(2), }),
-}
+local ls = require("luasnip")
+local s, t, i = ls.snippet, ls.text_node, ls.insert_node
+
+return {
+ s({ trig = "sc", desc = "short-cut ticket" }, {
+ t("[SC-"), i(1), t("] "), i(2),
+ }),
+}📝 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.
| return { | |
| s({ trig = "sc", desc = "short-cut ticket", }, | |
| { t("[SC-"), i(1), t("]"), i(2), }), | |
| } | |
| local ls = require("luasnip") | |
| local s, t, i = ls.snippet, ls.text_node, ls.insert_node | |
| return { | |
| s({ trig = "sc", desc = "short-cut ticket" }, { | |
| t("[SC-"), i(1), t("] "), i(2), | |
| }), | |
| } |
🤖 Prompt for AI Agents
In plugins/nobbz/lua/nobbz/luasnip/gitcommit.lua around lines 3 to 6, the
identifiers s, t, and i are used without being declared, causing errors at load
time. Fix this by locally requiring or importing these functions from the
luasnip module at the top of the file before their usage, ensuring they are
properly defined and available in the snippet file.
Summary by CodeRabbit
New Features
Improvements