refactor: remove nvim-treesitter as a dependency

[TheLeoP]

DONE:

• refactor
  • copy and refactor functions previously owned by nvim-treesitter and no longer present on their main branch
  • move away from the nvim-treesitter modules based architechture
  • expose lua functions instead of creating keymaps ourselves
  • introduce a TSTextObjects.Range abstraction to deal with: TSNode, range4, range6, lsp_range, vim_range and fake nodes created using the metadata from multiple captures (i.e. @*.outer captures)
• update README
  • show new keymaps examples using vim.keymap.set directly
• update docs
  • same as README
• update tests
• update scripts
• add autoformat queries to CI (has also already been done on the master branch)

This addresses #503

Moved to a follow up PR (TODO)

• cache query results
• reimplement consistency tests using Lua (python was previously used for this)
• better define the user interface of the plugin
  • some mappings rhs have the form some_exposed_func while others have the form function() some_exposed_func() end
  • should other interfaces like the previously used user commands be considered?
• maybe include incremental selection as an, in textobject

[clason]

Thank you for tackling this! I think the best course here is to not merge this to master but make this a new parallel branch main (which will become the default branch when nvim-treesitter itself switches). The goal here is to make this work (only) in concert with the main branch of nvim-treesitter; mixing and matching main and master is not supported.

(This also means we don't have to complete the rewrite in a single PR.)

refactor the code again (to not only used the copied functions from nvim-treesitter)

💯 (and yes, this approach is what I would suggest as well)

check if autocmds on filetype are the best option

Probably not; I would enable this globally with a check for parsers and query that short-circuits them.

Also, it's perfectly fine (and often preferable) to have some global startup code in /plugin; the standard assumption is that the plugin should work without explicitly calling setup if you're fine with default options.

add some kind of on_attach like nvim-treesitter-context (?)

I consider nvim-treesitter-context to be the gold standard right now for a standalone treesitter plugin; I'd try to follow its lead as much as possible. (@lewis6991)

refactor attach.lua

Yes, attach/detach delendam esse. These are obsolete concepts. The assumption is that people enable these different functions themselves, however makes most sense for the functionality.

should vim.treesitter.query.get be cached?

delete configuration fields for keymaps and let users define the keymaps themselves (?)

Yes, definitely! We don't need yet another DSL for setting keymaps. (Although we could talk about reasonable default mappings as opt-in.)

Although possibly this is a special case here; maybe we could discuss instead exposing a single (Lua!) function that is easy to map?

nvim-treesitter is only being used inside of scripts (check-queries, minimal_init and update_readme), is this ok?

That is OK; you need nvim-treesitter (main!) to set up the test environment.

[lewis6991]

should vim.treesitter.query.get be cached?

Correct me if I'm wrong, but doesn't core already cache this via a weak table?

[theHamsta]

Are there any plans to keep the commands? We've added them because some users requested them because they wanted to use them in ext mode or for vinyl remappings. Not a strong requirement since all functionality is available via Lua functions, just being curious on what your idea for this PR was.

[theHamsta]

should vim.treesitter.query.get be cached?

Correct me if I'm wrong, but doesn't core already cache this via a weak table?

I believe you're right, they should be cached. Nvim-treesitter-textobjects might be doing additional caching of the query results if they are using still an own slow algorithm to aggregate query results

[clason]

Are there any plans to keep the commands? We've added them because some users requested them because they wanted to use them in ext mode or for vinyl remappings. Not a strong requirement since all functionality is available via Lua functions, just being curious on what your idea for this PR was.

I think we should take this opportunity to revisit the user API here, with all the lessons learned since. (Not necessarily in this PR.)

My preference would be to start with a clean slate and (re-)add stuff when it is requested with a compelling use case. (User commands are trivial to add in a plugin file, as we do in nvim-treesitter now.)

What I don't want to see again is a custom module for creating keymaps and user commands through setup().

[TheLeoP]

Thank you for tackling this! I think the best course here is to not merge this to master but make this a new parallel branch main (which will become the default branch when nvim-treesitter itself switches).

How should I tackle this? I see that I can change which branch the PR targets, but I can't select the main branch since it does not exit yet. @clason

[clason]

Now it does :)

[lucario387]

So make-range was never needed? 😱

[clason]

should vim.treesitter.query.get be cached?

Correct me if I'm wrong, but doesn't core already cache this via a weak table?

Does this mean the memoize in nvim-treesitter (indent, locals) is obsolete now?

[clason]

Nice; want to make a PR to remove it from the main branch? (After local testing, which -- at least for indents -- you're in the best position to do.)

[lucario387]

Sadly, my previous comment was incorrect.

vim.treesitter.query.{get,parse} is cached, but the result of it is used for memoize of locals of indents, so neovim core's caching is for the parsing, while nvim-treesitter's caching is for the handling of such parse

[clason]

So we will need the same sort of memoization here for the textobjects query results, right?

[lucario387]

So we will need the same sort of memoization here for the textobjects query results, right?

Would be nice, I don't think there's one right now though

[TheLeoP]

So make-range was never needed? 😱

I didn't understand the codebase enought to know what make-range does, so I deleted it. I just added it again (because I couldn't make the test pass because parameter.outer in Python uses make-range), sorry :c.

[lucario387]

@TheLeoP basically we create a new node capture from the two node captures that is specified for make-range,

[TheLeoP]

@clason Is there a specific part of the code left that you would like to be refactored? I still haven't give too much atention to test, docs or scripts, I wanted to focus on the plugin functionality first.

[clason]

@clason Is there a specific part of the code left that you would like to be refactored? I still haven't give too much atention to test, docs or scripts, I wanted to focus on the plugin functionality first.

I think you are right that we should first make sure everything works with nvim-treesitter main and that all tests pass; afterwards it's much easier to make changes one by one (e.g., going through the backported utility functions to see if they can be replaced by Neovim core APIs).

Also, we have an autoformatter for queries now, which could be added to CI here ;)

[ObserverOfTime]

Can you keep the keymap configuration in setup?

What I don't want to see again is a custom module for creating keymaps and user commands through setup().

Disregard this comment. :^)

[clason]

No, this comment stands. I want to see the module framework gone. If you want to set keymaps (through this plugin), use the Neovim API directly.

[ObserverOfTime]

Yes, I didn't mean it should remain a module.

[clason]

@TheLeoP this looks like it's in a good place -- can you take care of the remaining items?

[TheLeoP]

@clason. Will do. Question: are the current vimdocs generated? Or do they have to be manually updated?

[clason]

They are manually written. It's OK to have a minimal (but correct) version for the initial PR.

[TheLeoP]

Is it expected to document the lua API being used for keymaps on the vimdocs/README?

[TheLeoP]

The query files test is working again c:, but the python tests seem to be broken indeed and I don't know enought python to look into them.

I wouldn't like to keep blocking this PR. @clason would it be ok to remove the python tests, as you mentioned, and move native lua test to a follow up PR?

[TheLeoP]

This PR should be ready to merge

[clason]

Can you squash your commits and update the PR/commit message (what's changed, what's still todo)? Then I'll merge.

[TheLeoP]

I've squashed all my commits and updated the PR and commit messages. Let me know if there is something else I may be missing

[clason]

You may need to rebase on main as well (rebase -i main and drop all but your commit), otherwise the merge won't be clean.

[clason]

Guess we need to rebase on master. Let me try to do this properly, you don't have to worry about it.

[clason]

Ok, CI is green.

@TheLeoP Before you delete (or pull!) your branch, please check the diff here against your local changes to make sure I did not force-push some of them away (in which case, please make a follow-up PR from latest main). Thank you so much for tackling this and for your patience through the (too) long process!

I'm looking forward to follow-up PRs (by you and other interested parties) cleaning up the old legacy design and adding a proper keymap setup (@ObserverOfTime).

@kiyoon @ribru17 Note that this means that master is essentially frozen:

• no further big changes to the plugin (bugfixes are fine, but only if they can be rebased on cleanly -- otherwise they should go to main directly)
• query changes still go to master as usual, but remember to regularly git rebase master on main; git checkout --theirs for non-query files will deal with conflicts.

[ribru17]

Awesome, thank you so much for your work @TheLeoP and @clason! Is there a roadmap (or general idea) for what must happen before master is updated to reflect these changes?

[clason]

Yes: hell must freeze over.

[clason]

Less facetiously: these changes will never make it to master; what will happen is that the main branch will become the default branch that receives all future updates and master becomes essentially a legacy branch for people unable to move to the shiny new world. This will happen together with the same change for nvim-treesitter (and other plugins we control), so is described by its roadmap.

[ribru17]

Makes sense, thank you