-
Notifications
You must be signed in to change notification settings - Fork 307
fix(snippets): filter out incompatible builtin snippets #2242
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
base: main
Are you sure you want to change the base?
Conversation
|
This does not skip only "friendly snippets" but all snippets that fail to parse (mainly nested placeholders). We could try to add more check and try to fix some of them (which I did for options), this method is very naive though. mini.nvim process snippet nodes char by char which is way more powerful but also complex. Don't know if we want to go on this path. |
|
Perhaps it'd make sense to only skip friendly snippets that fail to parse and log an error when user snippets fail to parse? Otherwise we'd be silently ignoring issues with the user's config. Also it'd be great if you could double check the performance as iirc snippet loading had significant overhead already. Cache likely makes it a non-issue though |
|
Sounds good! I'll do some additional testing and get back to you when I'll be on the right track. |
2cf21ff to
f9684e2
Compare
|
Pushed an update. I tried to fix snippets that were not coming from the user configuration (most likely from friendly-snippets) and added a message when parsing fails for user snippets; otherwise, they are silently discarded. I also attempted to filter out unsupported snippets using a regex as a lightweight pre-check before applying the LSP grammar, but my testing was inconclusive. As for the numbers, around 170 friendly-snippets were fixed. There is some overhead from the gsub operations and LSP parsing, but performance seems fine to me. Terraform has over 600 snippets, and it loads correctly in nvim even on my older machine. Below are the current results (fastest to slowest) using the provided benchmark methods. Current logic - Cold run (load registry + trigger completion per filetype)New logic WITH FIXES - Cold run (load registry + trigger completion per filetype)New logic NO FIX - Cold run (load registry + trigger completion per filetype)Benchmark source-- nvim --cmd "set rtp+=~/Development/nvim/blink.cmp" --cmd "set rtp+=~/.local/share/nvim/lazy/friendly-snippets" -l bench/snippet.lua
-- ┏━┓┏━┓┏━╸┏━┓
-- ┣━┫┣┳┛┃╺┓┗━┓
-- ╹ ╹╹┗╸┗━┛┗━┛
local iterations = tonumber((vim and vim.v and vim.v.argv and vim.v.argv[8]) or 1) or 1
local ft_snippet = (vim and vim.v and vim.v.argv and vim.v.argv[9]) or 'markdown'
print('Running benchmark for ' .. iterations .. ' iterations - ' .. ft_snippet)
-- ┏━┓┏━┓╺┳╸┏━┓
-- ┃ ┃┣━┛ ┃ ┗━┓
-- ┗━┛╹ ╹ ┗━┛
---@type blink.cmp.SnippetsOpts
local config = {
friendly_snippets = true,
get_filetype = function() return ft_snippet end,
search_paths = {
vim.fn.stdpath('config') .. '/snippets/vscode',
},
}
-- ┏┓ ┏━╸┏┓╻┏━╸╻ ╻
-- ┣┻┓┣╸ ┃┗┫┃ ┣━┫
-- ┗━┛┗━╸╹ ╹┗━╸╹ ╹
local snip = require('blink.cmp.sources.snippets.default')
local context = require('blink.cmp.completion.trigger.context').new({
id = 0,
mode = 'default',
providers = { 'snippets' },
line = '',
initial_trigger_kind = 'manual',
initial_trigger_character = '',
trigger_kind = 'manual',
initial_selected_item_idx = 1,
})
local builtin_snippets = snip.new(config)
local function benchmark(iterations)
iterations = iterations or 10
local total_time = 0
for i = 1, iterations do
local start_time = os.clock()
local completion_items = 0
builtin_snippets:get_completions(context, function(res)
completion_items = #res.items
end)
local elapsed = os.clock() - start_time
print(string.format('Run %d: %.5f seconds, snippets: %d, filetype: %s', i, elapsed, completion_items, ft_snippet))
total_time = total_time + elapsed
end
local avg_time = total_time / iterations
print(string.format('Average time over %d runs: %.5f seconds, filetype %s', iterations, avg_time, ft_snippet))
return avg_time
end
benchmark(iterations)#!/usr/bin/env sh
# Usage:
# time ./snip > /dev/null 2>&1
# ./snip 2>&1 | grep 'Run 1:' | sort -k8
fs_path=~/.local/share/nvim/lazy/friendly-snippets
for ft in $(find $fs_path/snippets -maxdepth 1 -name '*.json' -exec basename {} \; | sed 's/.json//' | sort); do
nvim --cmd "set rtp+=~/Development/nvim/blink.cmp" --cmd "set rtp+=$fs_path" -l bench/snippet.lua 1 $ft
done |
Skip early incompatible builtin snippets using LSP grammar.
f9684e2 to
3bca6d8
Compare
Skip early incompatible builtin snippets using LSP grammar.
Closes #2028