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

feat: 2-character "find next / prev pair" commands similar to f and F #12888

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

nik-rev
Copy link
Contributor

@nik-rev nik-rev commented Feb 16, 2025

2 new commands:

  • find_next_pair: Ask for 2 chars. Create selection at the next occurence of these 2 chars (accepts count). Bound to L
  • find_prev_pair: Ask for 2 chars. Create selection at the previous occurence of these 2 chars (accepts count). Bound to H

Differences from f/F: Does not drag the selection. Instead, creates a selection at those 2 characters. These commands are like a blend between goto_word and f/F motions.

Why

More granular control is desired when moving selections. This makes it easier to perform a range of text selections / transformation with fewer keystrokes and less overhead (see linked issue for more examples)

Note

Draft at the moment. Only find_next_pair "works" at the moment., lots of copy-pasting in the code. Will refactor tomorrow!

Closes #12878

@nik-rev nik-rev mentioned this pull request Feb 16, 2025
@ShiromMakkad
Copy link

How is the count accepted?

@nik-rev
Copy link
Contributor Author

nik-rev commented Feb 17, 2025

How is the count accepted?

Editor is responsible for handling the count, the commands can just read it

@nik-rev nik-rev marked this pull request as ready for review February 17, 2025 15:36
@the-mikedavis
Copy link
Member

Instead what I'd like to see for the use-case mentioned in #12898 is a refactor of the way that search works in select mode. Currently it adds a selection which is not really correct. It's fine to preserve that behavior in another command but it shouldn't be the behavior of extend_search_next/extend_search_prev. Instead it should extend like Kakoune's ?. the-mikedavis@7d69843

That change to the way search works in select mode should cover this use-case and is more interactive than these commands.

@nik-rev
Copy link
Contributor Author

nik-rev commented Feb 17, 2025

Instead what I'd like to see for the use-case mentioned in #12898 is a refactor of the way that search works in select mode. Currently it adds a selection which is not really correct. It's fine to preserve that behavior in another command but it shouldn't be the behavior of extend_search_next/extend_search_prev. Instead it should extend like Kakoune's ?. the-mikedavis@7d69843

That change to the way search works in select mode should cover this use-case and is more interactive than these commands.

Yes, I think that's much better! I just tested your commit and it works. What should be done to have it merged?

@the-mikedavis
Copy link
Member

It needs some cleaning up - I think the TODO comments can be dropped but Kakoune's behavior for ? should be checked. Plus it could use an integration test and it'd be nice to keep the current behavior of extend_search_next/extend_search_prev as separate commands.

@ShiromMakkad
Copy link

ShiromMakkad commented Feb 17, 2025

I'm imagining this as working for short hops, so avoiding pressing <Enter> at the end of the kaokune-style ? and just cutting it off at two characters would be nice (the count could be a config option). I think goto word works quite well for jumping across a large page. But I agree that v/, should mimic a kaokune search (but then how do you place an additional cursor at a specific location?). Both features being added makes sense to me.

Also, if the selection is going to cover the two characters, I think it should be under the goto menu, since that's how many of the commands in that menu work. Whereas f and t create a selection from the cursor to that location. So for binds on H and L, it should create a selection from the current location.

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

Successfully merging this pull request may close these issues.

Go to character
3 participants