neovim
Ensure `tstree:parse()` always return trees and changes
Hello!
While reading the lua source code around treesitter, I noticed the following:
When parsing from an invalidated state, tstree:parse() returns the trees and the changes.
https://github.com/neovim/neovim/blob/6cc6e11929ad76a2dc5204aed95cb9ed1dafde23/runtime/lua/vim/treesitter/languagetree.lua#L167
But when calling tstree:parse() again, with a valid state, the function returns only the trees.
:point_right: This would change the behavior of calls like tstree:parse()[1] expecting to get the list of trees, and instead getting the first tree.
https://github.com/neovim/neovim/blob/6cc6e11929ad76a2dc5204aed95cb9ed1dafde23/runtime/lua/vim/treesitter/languagetree.lua#L99
I believe this is a mistake
It seems that none of the functions have their return value type documented, is that intentional? :point_right: it's a bit hard to know what to expect from calling the function, other than trying them..
It seems that none of the functions have their return value type documented, is that intentional?
It's not; documentation is lacking since tree-sitter support is still experimental and in quite a bit of flux.
I think that we'd need to go the other way around: never return the changes an only notify them using the on_changed_tree callback.
The problem is that (as you noticed) it is not guaranteed for a plugin that it'll be the first one to call tstree:parse() and thus get the changes.
Regarding the documentation, types are lacking, and even some documentation are wrong, but that'll be done in a followup PR.
I think that we'd need to go the other way around: never return the
changesan only notify them using theon_changed_treecallback.
I've never used these directly, so I don't have an opinion. But I'd be happy to do the changes if you think it's right
I've never used these directly, so I don't have an opinion. But I'd be happy to do the changes if you think it's right
I indeed think it is, but that makes it a breaking change. Though this functionnality was not really used (I think), so it does not matter that much yet.
Not sure, I'd rather push that for 0.9 as it is not a really important change feature-wise.
Yes kind of. I'm not sure what the annotations are but we could go either way. I do see the appeal of returning non-nil.
This would change the behavior of calls like tstree:parse()[1] expecting to get the list of trees, and instead getting the first tree.
I don't think this is true. varargs and tables work very differently.
@bew can you rebase then, so we can test this with our current code/annotations?
I think that we'd need to go the other way around: never return the changes an only notify them using the on_changed_tree callback.
The problem is that (as you noticed) it is not guaranteed for a plugin that it'll be the first one to call tstree:parse() and thus get the changes.
I completely agree with this.
Briefly tested removing the second return value, and nothing broke (including functionaltests and nvim-treesitter). Want to switch the PR to do that instead?