feat(ALL): check children treesitter languages after parents#2407
feat(ALL): check children treesitter languages after parents#2407TheLeoP wants to merge 2 commits into
Conversation
|
Thanks for the PR!
There are already tests for injected languages (this one for 'mini.ai'). They use built-in Lua -> Vimscript injections. I think tests for this could use even Lua -> Vimscript -> Lua injections. Like here: vim.cmd([[
set cursorline
lua local a = 1
]])
There is less overall complexity if tree-sitter related (and even some other) code in 'mini.ai' and 'mini.surround' is kept as aligned as possible. As per this PR, please reduce code duplication. Both parents and children are meant to be processed exactly the same, right? In both 'mini.ai' and 'mini.surround' this processing is rather complicated, so even with two times duplication it adds a lot. |
I forgot about that test and I wrote it myself :p. Thanks for the pointer! Using Lua -> Vimscript -> Lua injections was indeed easier to avoid adding new queries.
I think I improved the alignment/code duplication between |
echasnovski
left a comment
There was a problem hiding this comment.
This indeed looks like a well structured change and improvement!
Apart from variable naming nit picks, could you please also:
- Add a concise note in each function's help about how it searches for the match.
- Add a 'mini.surround' test.
- Split into two commits for for 'mini.ai' and 'mini.surround'. Each with a concise description in
Details:list about why this is necessary. With alsoResolve #2397line afterwards.
Details: - Users may expect next/previous textobjects in injected languages located before/after the cursor to work, even if they are not part of the LanguageTree under cursor or its ancestors. This is specially important for languages that rely heavily on injections like markdown (injecting markdown_inline). - So, we fallback to searching in direct injected children languages if no textobject is found for current LanguageTree or its ancestors. Resolve nvim-mini#2397
Details: - Users may expect next/previous textobjects in injected languages located before/after the cursor to work, even if they are not part of the LanguageTree under cursor or its ancestors. This is specially important for languages that rely heavily on injections like markdown (injecting markdown_inline). - So, we fallback to searching in direct injected children languages if no textobject is found for current LanguageTree or its ancestors.
Are you referring to
This is about adding a comment to the private helper functions, right? Or is it about modifying the comments used to generate the documentation for the user-facing
I don't think a test for this change would be possible inside of |
| local res = {} | ||
| for i, outer in ipairs(outer_ranges) do | ||
| res[i] = { outer = outer, inner = H.get_biggest_nested_range(inner_ranges, outer) } | ||
| for i, outer in ipairs(outer) do |
| local lines = { | ||
| 'vim.cmd([[', | ||
| 'set cursorline', | ||
| 'lua local a = function () end', |
There was a problem hiding this comment.
| 'lua local a = function () end', | |
| 'lua local a = function() return true end', |
This should make it more robust: no space between function and () is usual (unless it is intentional?) and non-empty body (as empty function might be problematic).
Sorry, should have been more clear. Mentioning function's help in my mind means the text that is shown when doing And the explanation is very much needed because I just now (possibly) misremembered how this is supposed to work in the 'mini.ai' test. Is it intended and/or reasonable that the search is only done for "one level" children? In particular with this Lua code with cursor at the top, executing vim.cmd([[
set cursorline
lua local a = function() return true end
]])I think I would have expected all children to be used here, no matter however deep. Do you think it is reasonable?
The 'mini.ai' and 'mini.surround' are very aligned in how they find their targets (textobjects and surroundings). In this particular case, using T['gen_spec']['input']['treesitter()']['works with direct injected child language'] = function()
local lines = {
'vim.cmd([[',
'set cursorline',
'lua local a = function () end',
']])',
}
validate_find(lines, { 2, 0 }, { { 3, 14 } }, type_keys, 'sfn', 'F')
end |
This PR is only a proof of concept of what a fix for #2397 would look like.
Thoughts:
mini.surrounddoesn't seem to need this change since the cursor always needs to already be on top of the region with a treesitter injection. So, it will be parsed even before this PR when going up the tree.