potentially fix luasnip #69
Conversation
WalkthroughThe configuration for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant blink.cmp
participant luasnip
User->>blink.cmp: Trigger completion
blink.cmp->>luasnip: Check if inside snippet
alt Inside snippet and blink popup not visible
blink.cmp-->>User: Activate snippet completion
else Not inside snippet and in normal mode
blink.cmp->>luasnip: Unlink current snippet
blink.cmp-->>User: Snippet completion inactive
end
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
plugins/nobbz/lua/nobbz/blink.lua (1)
6-69: 💡 Verification agent🧩 Analysis chain
Implement lazy loading as required by coding guidelines.
The configuration doesn't follow the lazy loading requirement specified in the coding guidelines. All plugins should be lazy loaded through
nobbz.lazy.Please verify how other plugin configurations in the project implement lazy loading and apply the same pattern here:
🏁 Script executed:
#!/bin/bash # Description: Find examples of nobbz.lazy usage in other plugin files # Expected: Examples of lazy loading patterns to follow fd -e lua . plugins/ --exec grep -l "nobbz.lazy" {} \;Length of output: 519
🏁 Script executed:
#!/bin/bash # Inspect the nobbz.lazy pattern in init.lua sed -n '1,200p' plugins/nobbz/lua/nobbz/lazy/init.luaLength of output: 1513
#!/bin/bash # Locate the add_specs call in the lazy specs file grep -R "add_specs" -n plugins/nobbz/lua/nobbz/lazy/specs.lua
🏁 Script executed:
#!/bin/bash # Check for existing blink.nvim entry in lazy specs grep -R "blink.nvim" -n plugins/nobbz/lua/nobbz/lazy/specs.lua || echo "No blink.nvim spec found"Length of output: 121
Add Blink plugin to lazy-loading specs
Blink is currently configured eagerly via a direct
blink.setup()call. Per project guidelines, all plugins must be loaded throughnobbz.lazy. Please:
Refactor
plugins/nobbz/lua/nobbz/blink.luaso it returns asetup()function instead of invokingrequire("blink").setup()at load time.In
plugins/nobbz/lua/nobbz/lazy/specs.lua, register Blink withrequire("nobbz.lazy").add_specs({ … }), mirroring patterns in other specs. For example:require("nobbz.lazy").add_specs({ { "plugin-author/blink.nvim", event = "InsertEnter", dependencies = { "L3MON4D3/LuaSnip" }, config = function() require("nobbz.blink").setup({ -- your existing options }) end, }, })
🧹 Nitpick comments (1)
plugins/nobbz/lua/nobbz/blink.lua (1)
7-17: Snippet configuration logic looks sound but could be simplified.The custom
activefunction addresses the LuaSnip interaction issue mentioned in the PR title. The logic correctly manages snippet state based on popup visibility and current mode.Consider simplifying the logic:
active = function() - if luasnip.in_snippet() and not blink.is_visible() then - return true - else - if not luasnip.in_snippet() and vim.fn.mode() == "n" then luasnip.unlink_current() end - return false - end + local in_snippet = luasnip.in_snippet() + if not in_snippet and vim.fn.mode() == "n" then + luasnip.unlink_current() + end + return in_snippet and not blink.is_visible() end,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/nobbz/lua/nobbz/blink.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.lua
🔇 Additional comments (1)
plugins/nobbz/lua/nobbz/blink.lua (1)
1-2: LGTM! Good practice to explicitly require dependencies.The explicit requires improve code clarity and follow the local declaration guideline properly.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the configuration logic for snippet handling in the Blink plugin to enhance compatibility with snippet usage and popup visibility.
- Refactored snippet configuration by introducing an "active" function to control snippet behavior
- Adjusted logic to unlink snippets in normal mode when not in an active snippet session
| if luasnip.in_snippet() and not blink.is_visible() then | ||
| return true | ||
| else | ||
| if not luasnip.in_snippet() and vim.fn.mode() == "n" then luasnip.unlink_current() end |
There was a problem hiding this comment.
[nitpick] Consider caching the result of 'luasnip.in_snippet()' at the start of the active function to avoid duplicate function calls and improve clarity. For example, assign its value to a local variable and reuse it in subsequent checks.
| if luasnip.in_snippet() and not blink.is_visible() then | |
| return true | |
| else | |
| if not luasnip.in_snippet() and vim.fn.mode() == "n" then luasnip.unlink_current() end | |
| local in_snippet = luasnip.in_snippet() | |
| if in_snippet and not blink.is_visible() then | |
| return true | |
| else | |
| if not in_snippet and vim.fn.mode() == "n" then luasnip.unlink_current() end |
Summary by CodeRabbit