Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pluggable target selection engine #23

Open
ggandor opened this issue Aug 8, 2022 · 5 comments
Open

Pluggable target selection engine #23

ggandor opened this issue Aug 8, 2022 · 5 comments

Comments

@ggandor
Copy link

ggandor commented Aug 8, 2022

Specifically thinking about Leap, besides Hop.

It would be win-win for everyone:

  • This plugin could focus on one thing - there would be no need to reinvent the wheel, i.e. hinting and the selection of targets, which is supposed to be more robust/flexible in Leap/Hop, since that is the very focus of the aforementioned plugins.
  • Both groups of users could be directed to this plugin, and there would be no need to develop/maintain e.g. leap-ast.nvim.

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
Copy link
Contributor

yioneko commented Nov 3, 2022

@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
Copy link
Owner

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
Copy link
Contributor

yioneko commented Nov 4, 2022

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
Copy link
Author

ggandor commented Nov 5, 2022

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 ggandor changed the title Pluggable hint/jump engine Pluggable target selection engine Nov 5, 2022
@ggandor
Copy link
Author

ggandor commented Nov 5, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants