Skip to content
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

add meta data for each add_snippets call #719

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions DOC.md
Original file line number Diff line number Diff line change
Expand Up @@ -2315,6 +2315,19 @@ require("luasnip.loaders").edit_snippet_files {
end
}
```
* `target_snippet`: `table:snippet` If you have config option `store_meta_data`
set to true, then passing a target snippet to the edit function will jump
the cursor to the location where the target snippet is declared.
Currently, this only works for snippets that have been loaded through
`loaders.from_lua`. NOTE: the edit function only uses props that are
returned from `ls.get_context()` so you can either supply a full snip or
the stripped down variant returned from `get_context`.
```lua
require("luasnip.loaders").edit_snippet_files({
target_snippet = my_snippet,
})
```


One comfortable way to call this function is registering it as a command:
```vim
Expand Down
16 changes: 15 additions & 1 deletion doc/luasnip.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
*luasnip.txt* For NVIM v0.8.0 Last change: 2023 January 08
*luasnip.txt* For NVIM v0.8.0 Last change: 2023 January 11

==============================================================================
Table of Contents *luasnip-table-of-contents*
Expand Down Expand Up @@ -2267,6 +2267,20 @@ there are multiple) the associated file to edit.
end
}
<
- `target_snippet`: `table:snippet` If you have config option `store_meta_data`
set to true, then passing a target snippet to the edit function will jump the
cursor to the location where the target snippet is declared. Currently, this
only works for snippets that have been loaded through `loaders.from_lua`. NOTE:
the edit function only uses props that are returned from `ls.get_context()` so
you can either supply a full snip or the stripped down variant returned from
`get_context`.


>
require("luasnip.loaders").edit_snippet_files({
target_snippet = my_snippet,
})
<


One comfortable way to call this function is registering it as a command:
Expand Down
12 changes: 12 additions & 0 deletions lua/luasnip/config.lua
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@ local defaults = {
region_check_events = "User None",
delete_check_events = "User None",
store_selection_keys = nil, -- Supossed to be the same as the expand shortcut

-- TODO: generates a function which, given a filename produces the metadata
-- that shall badded to snippets loaded by one of the loaders.
--
-- @params table:
-- may contain: filename
-- ...
store_meta_data = false, -- For each snippet, eg. source path.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd like this to be a table, which can accept keys to determine what is stored. Most capable would be just a function which receives all available data and creates the metadata for the snippet, but I think this is overdoing it, since we probably don't need much flexibility there.

Still, making this a table would somewhat future-proof it, like

metadata = {sourcefile=true, line=true, custom = somefn}

Copy link
Author

@molleweide molleweide Jan 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using a subtable is good idea. I don't understand your reasoning about the function but it makes sense to do this properly in case we come up with more use cases.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!
My idea with the function was to allow complete customization of what is put into the metadata, essentially by calling a function like

function loader_metadata(filename, line)
	return {source=filename, source_line = line}
end

in the loaders.

Since there isn't this much customization possible (I think), letting the user set some keys for what exactly should be stored in the metadata-table seems like a more straightforward, and easier to use simplification.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ill try to making it into a func and see how it feels

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, this is the only thing left to do.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would one get the source line of the snippet in this function? Is there something builtin to luasnip already?

Oh, that's very hard to do for lua and vscode, I used it because I wanted an example for why more flexibility might be good.
I think we should not determine all lines up-front (except if it's trivial, like in the snipmate-loader), since only a tiny subset of this information will be useful, and it should, I think, be calculated on-demand

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not very hard, non-trivial

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the location for where the meta data loader should be called is within the session.snippets_colletion right, so we need to local session = require("luasnip.session") and then pass along snippets and opts to store_meta_data().

I had in mind that add_snippets receives the complete metadata (which may be nil), and we make the config-option specific to the loaders

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not very hard, non-trivial

Aight, for now my feed keys thingy works good nuff

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right! heuristics like that should do it for 99%

store_meta_data2 = function(opts)
return {}
end,

ext_opts = {
[types.textNode] = {
active = { hl_group = "LuasnipTextNodeActive" },
Expand Down
27 changes: 27 additions & 0 deletions lua/luasnip/extras/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,31 @@ local function match(index, _match, _then, _else)
return F(func, index)
end

-- TODO: MOVE TO OWN MODULE
local function jump_to_snip(snip)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this deserves its own module, then we can cleanly provide different heuristics for finding the correct snippet (after making that configurable ofc :D)
For example, finding in snipmate/lua/vscode could be made more precise by writing it specifically for that type of snippet

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. i'll move this into its own module.

local meta_data =
require("luasnip.session.snippet_collection").get_meta_data_by_snip_id(
snip.id
)
local source = meta_data.source
if source then
vim.cmd("edit " .. source)
if snip.name then
local feed_str = vim.api.nvim_replace_termcodes(
"/" .. snip.name .. "<CR>",
true,
true,
true
)
vim.fn.feedkeys(feed_str, "n")
end
else
print(
"No source found for id: " .. snip.id .. " with name: " .. snip.name
)
end
end

return {
lambda = lambda,
match = match,
Expand Down Expand Up @@ -161,4 +186,6 @@ return {
--alias
l = lambda,
m = match,

jump_to_snip = jump_to_snip,
}
60 changes: 51 additions & 9 deletions lua/luasnip/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,66 @@ local function default_snip_info(snip)
}
end

local function available(snip_info)
Copy link
Owner

@L3MON4D3 L3MON4D3 Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay afaict this can also be achieved with either the old available (scope=buf) or get_snippets (everything else). Even though this design is nice, separate functions are backward compatible, so I think we should stick with that

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick note: my addition should be backwards compatible.
I'll get back with another reply later.

Copy link
Author

@molleweide molleweide Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allright, so I think I understand what you mean but could you tell me a bit more specifically what you'd like me to do. Is my modification worth keeping in a modified state or should we scrap it completely?

My idea was that it made sense to put the logic in available so that one could filter all possible combinations via snip_info but I am also down with splitting it up somehow.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, of course it is backward compatible 🤦
What I don't like about this is that there's no good reason for snip_info to not be a part of opts, except staying backward compatible.

I think we'll have to scrap it :/
An alternative would be to expose it in addition to available (and implement available using it), but all of its functionality can be covered better by get_snippets (since get_snippets can access the implementation of snippet_collection)

Copy link
Author

@molleweide molleweide Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative would be to expose it in addition to available (and implement available using it), but all of its functionality can be covered better by get_snippets (since get_snippets can access the implementation of snippet_collection)

Yeah this makes sense. I'll try to add a new successor func that only uses the opts param. I'm thinking about calling it filter_snippets(opts) or retrieve_snippets(opts).

---Retrieve available snippets
---@param snip_info function: takes a snip and returns whatever props you want
--- from the snippet
---
---@param opts: May contain:
--- scope <nil|string("buf","everything")|table(filetypes)
--- buf: returns snippets for current buffers filetypes
--- everything: returns all loaded snippets
--- table: allows you to filter snippet subsets by filetype(s)
--- include_autosnippets bool
---
---@return table: the resulting table of ft snip mappings after fitlers have
-- been applied
local function available(snip_info, opts)
opts = opts or {}
opts.scope = opts.scope or "buf"
opts.include_autosnippets = opts.include_autosnippets or true
snip_info = snip_info or default_snip_info

local fts = util.get_snippet_filetypes()
local res = {}
for _, ft in ipairs(fts) do
res[ft] = {}
for _, snip in ipairs(get_snippets(ft)) do

local function validate_and_append_to_res(ft, t_snips)
for _, snip in ipairs(t_snips) do
if not snip.invalidated then
table.insert(res[ft], snip_info(snip))
end
end
for _, snip in ipairs(get_snippets(ft, { type = "autosnippets" })) do
if not snip.invalidated then
table.insert(res[ft], snip_info(snip))
end

local function filter_all_snippets(t_snips)
for ft, snips in pairs(t_snips) do
if
opts.scope == "everything" or vim.tbl_contains(opts.scope, ft)
then
if not res[ft] then
res[ft] = {}
end
validate_and_append_to_res(ft, snips)
end
end
end

if opts.scope == "buf" then
local fts = util.get_snippet_filetypes()
for _, ft in ipairs(fts) do
res[ft] = {}
validate_and_append_to_res(ft, get_snippets(ft))
if opts.include_autosnippets then
validate_and_append_to_res(
ft,
get_snippets(ft, { type = "autosnippets" })
)
end
end
elseif opts.scope == "everything" or type(opts.scope) == "table" then
filter_all_snippets(get_snippets())
if opts.include_autosnippets then
filter_all_snippets(get_snippets(nil, { type = "autosnippets" }))
end
end

return res
end

Expand Down
5 changes: 5 additions & 0 deletions lua/luasnip/loaders/from_lua.lua
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ local util = require("luasnip.util.util")
local str_util = require("luasnip.util.str")
local ls = require("luasnip")
local log = require("luasnip.util.log").new("lua-loader")
local session = require("luasnip.session")

local M = {}

Expand Down Expand Up @@ -83,6 +84,9 @@ local function load_files(ft, files, add_opts)
ft,
file_snippets,
vim.tbl_extend("keep", {
-- TODO: redo `source` here by passing the entire meta-table
-- metadata =
source = file,
type = "snippets",
key = "__snippets_" .. file,
-- prevent refresh here, will be done outside loop.
Expand All @@ -93,6 +97,7 @@ local function load_files(ft, files, add_opts)
ft,
file_autosnippets,
vim.tbl_extend("keep", {
source = file,
type = "autosnippets",
key = "__autosnippets_" .. file,
-- prevent refresh here, will be done outside loop.
Expand Down
48 changes: 48 additions & 0 deletions lua/luasnip/session/snippet_collection.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
local session = require("luasnip.session")

-- store snippets by some key.
-- also ordered by filetype, eg.
-- {
Expand Down Expand Up @@ -128,6 +130,33 @@ local by_id = setmetatable({}, {
__mode = "v",
})

-- If `add_snippets` opts.source and config.store_meta_data then each call to
-- add snippets stores the source file and potentially other meta data in this
-- table which then each snippet points to so that one can `jump-to-snip`
-- by calling `loaders.edit_snippet_files({ target_snippet = snip_context_x })`
local snippets_meta_data = {}
local snip_id_to_meta_map = {}

local function initialize_snippet_meta_data(opts)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think this mechanism is good, but we need to split it up a bit:

  • config generates a function which, given a filename produces the metadata that shall be added to snippets loaded by one of the loaders. We should not pass the filename directly, but wrap it in a table like {filename=<the filename>}, since we can then add more arguments and the function will stay comfortable to use (no need to write a handler like function(_, _, _, theOneArgINeed) ... end)
  • this function is used in loaders to generate the metadata for the snippets (no ifs or something like that, we can just call the function. This is flexible enough to probably not require any changes, except if new parameters are added)
  • add_snippets receives metadata-table, which is associated with the snippets added in this call, unconditionally

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I have to ponder your reply and come back later with a revised version.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking forward to it :D
If you have any questions or other ideas, go ahead, I'd love to find something better (especially the association of snippet with its metadata is a bit annoying, since we have to do that id-redirection. Can't think of a better way though :/)

Copy link
Author

@molleweide molleweide Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, yeah I cant really come up with something better either atm.
Let's optimize it later and I'll stick to the id mapping for now.

-- If we use two funcs one for module and one for each snip then the following
-- block would obviously have to be modified to work with that
--
-- config.store_snippets_data = {
-- get_module_module_data = function(opts) return { source = opts.source },
-- for_each_snippet = function(ft, snip, opts) ... end
-- }

-- func / table
if type(session.config.store_meta_data) == "function" then
-- NOTE: use function is still a bit wip
table.insert(snippets_meta_data, session.config.store_meta_data(opts))

-- if bool
elseif session.config.store_meta_data then
table.insert(snippets_meta_data, { source = opts.source })
end
end

-- ft: any filetype, optional.
function M.clear_snippets(ft)
if ft then
Expand Down Expand Up @@ -239,7 +268,14 @@ end
local current_id = 0
-- snippets like {ft1={<snippets>}, ft2={<snippets>}}, opts should be properly
-- initialized with default values.
--
-- opts.source = path to snip module which is assigned in
-- `from_lua.load`
function M.add_snippets(snippets, opts)
if session.config.store_meta_data then
initialize_snippet_meta_data(opts)
end

for ft, ft_snippets in pairs(snippets) do
local ft_table = by_ft[opts.type][ft]

Expand Down Expand Up @@ -270,6 +306,14 @@ function M.add_snippets(snippets, opts)
table.insert(by_prio[snip.snippetType][snip.priority][ft], snip)
table.insert(by_ft[snip.snippetType][ft], snip)
by_id[snip.id] = snip

if session.config.store_meta_data then
snip_id_to_meta_map[snip.id] = #snippets_meta_data
-- TODO: (wip) following line would store single snippet specific data
-- we are not sure yet how this would be done but I leave it here
-- for reference
-- snippets_meta_data[#snippets_meta_data].snippet_specific[snip.id] = session.config.store_meta_data.each_snippet(ft, snip, opts)
end
end
end

Expand Down Expand Up @@ -311,4 +355,8 @@ function M.get_id_snippet(id)
return by_id[id]
end

function M.get_meta_data_by_snip_id(id)
return snippets_meta_data[snip_id_to_meta_map[id]]
end

return M