helix: Add match operator

[fantacell]

This is an implementation of matching like "m i (", as well as "] (" and "[ (" in helix_mode with a few supported objects and a basis for more.

Release Notes:

• Added helix operators for selecting text objects

[ConradIrwin]

Thanks! This seems reasonable to me.

There's been a lot of talk about moving helix out into more of its own thing; I think we should keep that option open and push for more of the code to live in helix.rs or the helix directory instead of under normal/ (which is already a bit of dumping ground).

(e.g. select.rs can just move in there; as could some of the motion commands).

On the flip side. Are there other helix motions that want to be able to use object.range? If so, we could update range to behave differently in helix mode.

[fantacell]

I don't get it: Why are the tests failing? They pass on my laptop and marks work when running this in vim normal mode.

[fantacell]

On the flip side. Are there other helix motions that want to be able to use object.range? If so, we could update range to behave differently in helix mode.

There are also ]f and such, maybe they can be implemented this way.

[ConradIrwin]

Looks like the test is failing on the merge of main and this branch, but passes on both branches separately. Let me know if you need help debugging why..

[ConradIrwin]

@fantacell should this still be a draft?

[fantacell]

* add the "[" and "]" operators to helix

* fix more inconsistencies between vim and helix

* add more tests

I'm still working on those, but I think I'm almost done.

[fantacell]

I didn't know when writing this that helix itself doesn't have motions like selecting the next brackets ]( (yet?), but I think they can still be useful.

[fantacell]

Right now typing something like [" quickly will start to select a register, rather than selecting the previous " pair. I have to wait until the status bar shows [ after pressing [ and I don't really know how to fix that.

[fantacell]

I think feature wise this is ready, more can be added later. But I think implementing this similarly to to word motions might not have been so good; paragraph and sentence text objects are pretty slow in debug builds. mip is slower than vip. I have a few more ideas for a bit more performance with this way, but maybe they can be implemented using regexes? I feel like buffer and project search are incredibly fast.

Other than that it seems fine in helix mode, but @ConradIrwin by moving the keybindings around between contexts we broke the tests for vim mode. Before, the first char of "] {" in VimControl was prioritized over "]" in vim_operator, now "{" in VimControl seems prioritized over the second char of "] {" in vim_mode. Should that happen? I mean, assuming VimControl is a higher context than the other two, shouldn't it be the other way around?

[ConradIrwin]

Hmm; in theory iterating over strings manually shouldn't be very notably slower than regexes. Are we doing a lot of co-ordinate translations between DisplayPoint and Point etc.?

Looks like the clippy monster is not happy yet

[fantacell]

It should be appeased now.

Hmm; in theory iterating over strings manually shouldn't be very notably slower than regexes. Are we doing a lot of co-ordinate translations between DisplayPoint and Point etc.?

Oh, yes actually, DisplayPoint to offset and back. I read that one blogpost on the conversion, so maybe I can just pass offsets themselves around between functions. Regexes mainly because of the more complicated syntax like nested brackets and lookahead using fancy_regex. What I'm doing instead of this last one in particular is really unoptimized. But it was just an idea.

[ConradIrwin]

Happy to look into it if it's really slow – for large long-range queries we should use tree-sitter; but for smaller things character iteration should be fine (as long as we're not backtracking too much..)

[fantacell]

It feels ok to me in release builds right now. But I can store more state in between searches to avoid backtracking, that would be needed for "m i m" anyway. I could do that and switch to only offsets, either now in this PR or later. If it is ok for now, the only problem left is the keybinding conflict.

[fantacell]

Much better. m i p is still slower than v i p, but more comparable. I only removed all the unnecessary conversions between DisplayPoint and offset.
I think this is ready when the keybinding conflict is solved, but I have no idea how to do that.

[romaninsh]

hey @fantacell great job with this PR. Although it looks like a lot of boundary-related logic got duplicated, if you have @ConradIrwin blessing for this - it probably is the correct way to go. Lovely to see helix-related operators being implemented thoroughly.

I'd be interested to look into how we can integrate with #34798 next.

[fantacell]

hey @fantacell great job with this PR. Although it looks like a lot of boundary-related logic got duplicated, if you have @ConradIrwin blessing for this - it probably is the correct way to go. Lovely to see helix-related operators being implemented thoroughly.

thanks! About the duplication: In vim mode every object kind handles the finding itself, which is hard to access to then find the "]" and "[" ranges. In this PR, the only object specific thing is what is and isn't a start / end depending on the surrounding chars. The finding itself is then only done by the "]", "[", "mi" and "ma" operators. Also, "vi" and "mi" ranges are in many cases slightly different and I didn't want to add tons of checks for mode to then cover every edge case. An advantage of this would be that tree-sitter text objects should be easy to add, as well as kakoune style "]" and "[", which is how helix "]p" and "[p" seem to actually work. I just haven't found a way to express that with the keymap. If you (or anyone) have an idea for that, it would be welcome.

I'd be interested to look into how we can integrate with #34798 next.

How to access this over the keymap does seem like a real problem right now.

[ConradIrwin]

Sorry for the delay here, I've been wrapped up in https://agentclientprotocol.com for the last few weeks.

I think the problem is just we were missing the operator vim binding mode in the keymap; pushed a fix. The helix test is still failing though...

Feeling close though :D.

@Romanisch - re duplication; I think it's a better structure overall if helix matching is "consistently slightly different" to not try and share too much code (because it ends up with a bunch of nested booleans and ifs in the middle of stuff that's already quite fiddly). So it seems clearer to have two methods that do the similar-but-not-quite-the-same things.

(In v0 of helix mode, re-using vim code made more sense because the differences didn't matter)

[fantacell]

Yes; thank you for fixing the keybinding stuff, it was nice to see green for once. But I found a bug with m i " that starts a few commits ago. It shouldn't be hard to fix, but I need a little more time.
Then there is the [ p which doesn't work like the other objects in helix, so what it does now is kind of wrong. But maybe that's ok for now?

[romaninsh]

@fantacell, I have combined some of my previous work with the now closed Select mode PR into this draft:

#37748

I found it quite hard to figure out how to operate helix_move_cursor in select move, since perform collapse of selection. It's not longer possible to wrap it as i've done before.

Do you have some idea how to implement it without regressions?

[fantacell]

@romaninsh Hm right now the helix functions all kind of "do" and don't "return". You could add something like motion.move_point(...), but for helix that returns a head and optionally a tail. Then there could be a function like helix_extend_selectionsand helix_move_cursor could call either that or helix_new_selections with the output of the new method.

Though just to be sure: Have you seen the last comment #34136 (comment)?

[romaninsh]

@fantacell i've seen that comment from @eliaperantoni and I asked to credit them for the change. However I also wanted to add some of my fixes in my PR, making helix mode more sticky and added some simplifications.

Also i have grouped up keybindings which apply to both normal and visual mode, to avoid duplication, while keeping visual mode clean otherwise.

i'll think a bit longer on how to properly re-use your movement functions in select mode, i can't see any clear way to implement now.

[eliaperantoni]

No worries at all about the credits! The fact that I'm mentioned here is credit enough ❤️. I got a bit busy with a new job and I couldn't continue my PR but it's totally okay that someone else is working on this :) Thanks for the contribution @romaninsh this looks great!