feat!: remove lsp interop

[clason]

One of the goals of the nvim-treesitter refactor towards 1.0 is to aggressively delimit the scope of each individual plugin to improve maintainability. Long-term, this plugin should only define textobjects and leave their manipulation to generic builtin Vim methods.

As the lowest-hanging feature, remove the LSP interoperability ("peek definition"). If such a feature is desired, it should be reimplemented (as a generic plugin) without shoehorning it into the LSP handlers.

The next feature on the chopping block would be swap, which belongs more in nvim-treesitter-refactor. Arguably out of scope, too, is move, but since this is something Helix offers out of the box, I'd default to keeping it here for now; but ideally "move to next <textobject>" should be a Neovim API.

[ofseed]

I have a fix for swap in https://github.com/nvim-treesitter/nvim-treesitter-textobjects/pull/663, if swap is decided to be removed, feel free to close it. I have never used it because there's another plugin that was implemented better, just tried it out of curiosity and found that it didn't work as expected.

Whatever, I've fixed all of this plugin's features, lol.

[ofseed]

When this plugin only defines textobjects, shouldn't it be merged into nvim-treesitter?

[clason]

I was thinking more "move out", not "remove", so the fix is still valuable (so I merged it). Note that the other plugin will also be broken by the removal of the module system.

When this plugin only defines textobjects, shouldn't it be merged into nvim-treesitter?

That is an option under discussion, yes. But that goes against the goal of making nvim-treesitter less of a critical piece of infrastructure (i.e., by having wasm parser auto-installed in Nvim core), so it's not a clear-cut decision.

(Indent is different since it's intended (hah!) to be upstreamed eventually, and it's small enough to keep in nvim-treesitter until then. I'm not yet convinced the current textobject module is small enough for that, even after the user-facing features are removed.)

[clason]

@ofseed swap and move are safe, don't worry (but I would still like to see the whole "repeatable move" just be default and not exported as a separate functionality).

[kiyoon]

@clason the problem of "repeatable move" is that it needs to remap the default keybindings (; and ,) and there's a possibility that many plugin intercepts this. Also, for the default f, F, t, T to work with it, you need to remap all of them too! So I would leave it as an optional opt-in feature

[clason]

Mostly thinking about stuff like

Furthermore, you can make any custom movements (e.g. from another plugin) repeatable with the same keys.

I'm OK with remapping ; and f and friends to be opt-in.

(The issue is that this plugin has a way too large API surface, which I'd like to see reduced on main.)

[kiyoon]

You you'd like to remove the API interface for the repeatable movement? I think it's very handy that you can add custom movements repeatable though.

[clason]

Yeah, but shouldn't this be a separate plugin then? (Which probably already exists...)

[kiyoon]

https://github.com/ghostbuster91/nvim-next

Not sure if it's actively maintained but yeah, during the work it became a separate project. But in my opinion, this is a very simple module that just remembers the previous move command and making it separate and maintaining a whole new plugin seemed too much of a work and more importantly, unfriendly to the users

[clason]

Well, my point is that the alternative is unfriendly to maintainers ;) If you advertise the feature, you have to keep it working and field issue reports about (in)compatibility with a million other plugins -- all this for something that has nothing to do with treesitter...

Especially if there is a good alternative.

[kiyoon]

I get your point, that it has nothing to do with treesitter. I think the API is not widely used though, I don't remember any issues with reported about the feature. I think people just use it for nvim-treesitter-textobjects and be happy about it as is.

[clason]

Yes, and we absolutely should keep it for those ;) I'm just saying we shouldn't advertise it as a general-purpose API and instead point people to the plugin you mention. (And as a consequence, possibly not even expose these features in favor of a setup option -- which we have the opportunity of rethinking from scratch on main.)

[kiyoon]

ok, I'm okay with making a new simple plugin and maintaining it myself. But again, the repeatable movement feature has been asked many times and it is expected that many users want such feature, so we keep the internal functionality as is.

I think all you need to do is to remove from README about the custom movement API.

[clason]

ok, I'm okay with making a new simple plugin and maintaining it myself.

I was under the assumption that nvim-next basically covers this use case; I'm certainly not asking you to do extra work. And, yes, I'm not suggesting removing repeatable movements for the movements provided by this plugin.

[kiyoon]

Personally, I'm more comfortable with having a simple API than having a setup function and pre-defined integrations to plugins which might cause issues with lazy-loading and dependency problems.

I might be wrong about the plugin but, I would make a plugin at least for myself anyway. The one good thing that nvim-treesitter-textobjects had this feature, in my opinion, was that it is a widely-used plugin and it somewhat made a standard, and gave inspiration to other users on how to set this up.

[clason]

Personally, I'm more comfortable with having a simple API than having a setup function

In general I would agree, but if it involves exposing a dozen slightly different convenience wrappers, the equation changes.

pre-defined integrations to plugins which might cause issues with lazy-loading and dependency problems.

Yes, which is why I don't want those in this plugin ;)

Just to give context: nvim-treesitter is moving -- where possible -- towards an integration and incubator project for upstream. Since some form of textobjects are considered for core integration, this covers this repo as well (as opposed to nvim-treesitter-refactor or nvim-treesitter-context, which are out of scope and thus under less scrutiny).

I might be wrong about the plugin but, I would make a plugin at least for myself anyway. The one good thing that nvim-treesitter-textobjects had this feature, in my opinion, was that it is a widely-used plugin and it somewhat made a standard, and gave inspiration to other users on how to set this up.

Sure, that makes sense -- and I'd be happy to link to yours instead of nvim-next! It's perfectly fine to have a minimal version here that suits our purpose, and a more full-featured and configurable for general purposes.

The problem is that having a plugin become so popular that other plugins rely on it means that it becomes very hard to make changes that may be necessary for the core functionality but would impact other plugins that rely on the API (a lesson hard-learned); it also means that if this plugin ever becomes obsolete, archived, or unmaintained, many plugins would break. (See plenary.) The hard-won lesson is that "infrastructure plugins" may be useful early on but sooner or later become a massive point of failure.

[kiyoon]

I'm happy to see the refactor to make the functionality simple and pluggable! I'm definitely down to that approach thus I agree that we need to make dependency as clean as possible.

What happened to plenary? Is it obsolete? I don't see that it is archived or something.

[clason]

What happened to plenary? Is it obsolete? I don't see that it is archived or something.

It's not, but it is bitrotting due to low maintenance (not blaming the maintainers, mind you -- they are doing an admirable job with little time; they just don't have the bandwidth of developing it in response to newer Neovim APIs). And it can't be archived since so many plugins rely on it (including this one, for tests).