mfussenegger / nvim-treehopper

Region selection with hints on the AST nodes of a document powered by treesitter
GNU General Public License v3.0
426 stars 17 forks source link

Pluggable target selection engine #23

Open ggandor opened 2 years ago

ggandor commented 2 years ago

Specifically thinking about Leap, besides Hop.

It would be win-win for everyone:

I'm a bit confused about the current state of things though: Treehopper depends on Hop for jumping, but why is Hop necessary for what is basically an api.nvim_win_set_cursor call? On the other hand, if Hop is a dependency, why not use Hop for hinting - why a redundant implementation for Visual mode?

Anyway, integrating with Leap would be super simple, you simply get the node positions (optionally sort them in a preferred order), and feed it to Leap that takes care of the rest. You can also give it a custom action callback, that can be a wrapper around the range selection functionality already implemented here for Visual/OP mode.

If we'd want to show the same label in multiple places (start & end of the node), that would require some tweaks in the Leap API, but obviously doable. Also, there is no need to respect the "global" defaults of Leap, the highlights (or anything else) can be customized for treehopping, via autocommands. (Check https://github.com/ggandor/leap.nvim#extending-leap.)

yioneko commented 1 year ago

@ggandor Is the same label feature on plan? The idea of this plugin is really simple, and I do not think things like leap-ast.nvim are reinventing the wheel, not to mention that the current implementation of treehopper is really weird and confusing. If leap could add support for the same label feature and add snippet for this (like extending-leap), treehopper could be completely superseded and users are free from one more plugin which is not well maintained. IMO the "select parent" feature should not be extracted as a separate plugin since this is really, really simple. I do think that user (like me) prefer 20-30 loc config to enjoy the extensibility of leap instead of a third party plugin.

mfussenegger commented 1 year ago

Treehopper depends on Hop for jumping, but why is Hop necessary for what is basically an api.nvim_win_set_cursor call? On the other hand, if Hop is a dependency, why not use Hop for hinting - why a redundant implementation for Visual mode?

That's partly historic. When I created this plugin, hop didn't have an extension API The move functionality was added later. Partly to see what can be done with the hop API. And there was no reason to change the region selection.

If we'd want to show the same label in multiple places (start & end of the node), that would require some tweaks in the Leap API, but obviously doable.

That kind of hinting is a major factor, and something I'd like to play with further. E.g. in one line there are often many nodes and it can become hard to see where one starts and ends. I have in mind to play around with virtual lines to improve that, e.g. to have something like this:

  coroutine.wrap(function() move(opts) end)()
                 ▲          ▲  ▲     ▲   ▲
                 │          ├a─┘     │   │
                 │          └───b────┘   │
                 └──────────c────────────┘

treehopper is really weird and confusing.

I'd call it simple and straightforward 🤷‍♂️

one more plugin which is not well maintained.

It does what it was designed to do, not having a constant flow of commits doesn't mean it's not maintained.

yioneko commented 1 year ago

That's partly historic. When I created this plugin, hop didn't have an extension API The move functionality was added later. Partly to see what can be done with the hop API. And there was no reason to change the region selection.

Yes, personally I understand the reason behind the design. I've used treehopper (previous ts-hint) for nearly one year and tried to implement the same function with hop extension API but frustrated with the missing same label feature. But once that feature could be implemented, why using a separate plugin for nearly a simple loop to traverse nodes ? For the selection part, most of the users already have a more generic util to select node from nvim-treesitter, and might be upstreamed in the future. (ok, I forget the lsp region selection feature. I've never used it 😂)

E.g. in one line there are often many nodes and it can become hard to see where one starts and ends. I have in mind to play around with virtual lines to improve that, e.g. to have something like this:

That is awesome and I really appreciate your idea! However, that depends on whether you have time to implement or the community like to contribute. I'd like just discussing core implemented function here, which even has some very frustrating issue currently like #17 since the day of creation. (I know it is partly from confusing design of nvim treesitter itself, but this is just from user's perspective, who usually do not have enough time to dig into the treesitter stuff to find out the reason)

I'd call it simple and straightforward 🤷‍♂️

Again, from user's perspective, multiple hint providers and integrated only with hop (from users of leap) is strange.

It does what it was designed to do, not having a constant flow of commits doesn't mean it's not maintained.

@mfussenegger I admit that I didn't express my thinking here so well, sorry about that if you feel offensive. What I meant here is that compared with hop (or leap), treehopper seems to receive less care as a jumping plugin with a self-implemented hinting engine. Since the generic jumping interface become mature, I prefer simple idea like treehopper could be placed in REAMDE or wiki. Most of the users should prefer several lines of code to a separate plugin (at lease me).

ggandor commented 1 year ago

Is the same label feature on plan?

In some form, yes, it could be useful for leap-spooky. This is basically a subset of the more general problem spooky tries to solve (targeting regions/text objects instead of specific positions).

IMO the "select parent" feature should not be extracted as a separate plugin since this is really, really simple

I actually agree. Also, I'm not sure anymore that this should be solved with labeling at all, since it cannot be solved in a really satisfying way, by design, as the edges of AST nodes can overlap each other, as opposed to native Vim text objects. The incremental selection feature already provided by treesitter is probably more intuitive and should be enough most of the time for the "select current (n-th) parent" task.

ggandor commented 1 year ago

I have in mind to play around with virtual lines to improve that, e.g. to have something like this:

Eeh... this occurred to me as well, but this is a typical over-engineered gimmick that looks cool from a distance, and the gifs would earn you a bunch of likes on community platforms, but by the time you figure out the proper hint to use from such a diagram, you would have finished the task long ago with simple incremental selection. A more serious issue is that virtual lines necessarily move the buffer areas you might want to operate on. If there's the constant danger that the object itself I've been looking at is suddenly shifted up or down by n lines, and first I have to reorient myself before doing anything, that makes the whole concept dysfunctional.