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

Search from each selection #5672

Open
the-mikedavis opened this issue Jan 24, 2023 · 16 comments
Open

Search from each selection #5672

the-mikedavis opened this issue Jan 24, 2023 · 16 comments
Assignees
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements

Comments

@the-mikedavis
Copy link
Member

the-mikedavis commented Jan 24, 2023

The current search (/, n and N) only searches from the primary cursor. In select mode (v/) this can be used to add matches of the search pattern as new cursors.

It would also be useful to be able to search from every cursor. Instead of adding new selections for the next match from the primary cursor, this search would replace or extend the current selection(s) (depending on normal or select mode).

This behavior would be the same as textobjects like ]p or ]f. This could be used together with #1383 to implement regex-based textobjects like searching for numbers, dates, version control markers (#4647), etc.. And I think it could be useful on its own in some multi-cursor scenarios and some single-cursor scenarios like #5668.

There might be some overlap with this and #4635 with how search matches would be computed.

@fennewald
Copy link

Ran into a case were running a macro with multiple cursors will cause the macro to behave differently for primary vs the others. Would definitely appreciate this!

@xxxserxxx
Copy link

I want to comment here a work-around, for anyone (e.g., future me) searching for how to accomplish this (until such time as something better is implemented):

v<Alt-/>[search term]<Alt-->

and then trim the selection as necessary.

@rcorre
Copy link
Contributor

rcorre commented Apr 18, 2024

@xxxserxxx what are Alt-/ and Alt--? I think those bindings might have changed.

I found a decent workaround, which is maybe the same as you're suggesting?

[keys.select]
n = ["extend_search_next", "merge_selections"]
N = ["extend_search_prev", "merge_selections"]

It can have unexpected behavior if you're starting with multiple cursors though.

@xxxserxxx
Copy link

@rcorre <Alt-/> used to search without changing your selection. <Alt--> still joins multiple selections together -- at least, until all the keybindings are changed again. I don't know what is now bound to, or if it was just removed.

Your solution is frustratingly almost what I want; but I (and what people are requesting in this issues, I think) don't just want to keep searching for the same things. I want to start at some point and then interactively search forward for some pattern, extending the current selection. Think:

aaa
bbb
ccc
ddd
eee
fff
ggg

I want to jump to ccc and select everything between ccc and fff. So: /ccc<Enter>x gets me started... but how do you get to fff extending your current selection? extend_search_next only works if your use case is to select everything between two identical search terms.

Now that <A-/> is gone, I don't have a work-around suggestion for this extremely common situation.

@sibouras
Copy link

sibouras commented Apr 18, 2024

you could /ccc<Enter>C/fff then <alt>--

@the-mikedavis
Copy link
Member Author

A-/ has never been bound in the keymap - you can search for it with git log -S 'A-/' and git log -S "alt!('/".

For that use-case we could respect the mode in //? so that it behaves like n/N in select mode. Currently it's hard-coded to move the primary cursor rather than add a cursor like you would with n/N in select mode. (That's why @sibouras's workaround works - searching in normal mode moves the primary cursor and leaves the rest unchanged.) Then you could merge selections with A-minus to select the whole range.

@rcorre
Copy link
Contributor

rcorre commented Apr 18, 2024

but how do you get to fff extending your current selection?

vgw works, using the new extend_to_word (not saying that fixes the issue, but just another workaround to be aware of).

@xxxserxxx
Copy link

A-/ has never been bound in the keymap - you can search for it with git log -S 'A-/' and git log -S "alt!('/".

Yeah, I don't remember what I meant to write; I had A-/ bound to something, or I just typed it in from a faulty memory. The entire work-around was so klugy I rarely used it. @sibouras' solution does the same thing and is cleaner.

Cognitively, being in visual mode and having selected some text, and having searching forward (or backward) causing the selection to be lost -- I'm trying to understand why that is the current behavior. Is that the intuitive behavior for someone, or is it some technical difficulty?

@the-mikedavis
Copy link
Member Author

I believe it's just an oversight. Currently select mode has no effect in search/rsearch but I don't think that's intentional

@sibouras
Copy link

it would make more sense for gw to respect mode also like / n N, and you can get the old behavior by merge_selections

@the-mikedavis
Copy link
Member Author

That isn't about respecting mode. search/rsearch handle select mode differently than all other commands by adding selections rather than extending them. gw's behavior is the way select mode should generally work.

@sibouras
Copy link

ah i see, it would be nice though when in select mode gw selects a new word, it would make swapping words easier eg.

vegw then rotate_selection_content_forward instead of having to type or regex find the word, and we can have the old behavior by merge_selections

@thomasschafer
Copy link
Contributor

thomasschafer commented Jun 2, 2024

If we're thinking that / should respect mode, presumably n should do the same, which would conflict with the current behaviour of n in visual mode adding a selection at the next occurrence of the search term - would we want a new keymap for this existing behaviour?

I want to try and get a PR up to fix this issue, but just want to confirm the desired behaviour before I get started.

@pascalkuthe
Copy link
Member

@the-mikedavis is already testing an Implementation of this that we discussed that matches what we want so starting from scratch with a new PR probably doesn't make sense

@thomasschafer
Copy link
Contributor

Ah, thanks for letting me know! I wasn't aware this was being worked on.

As an aside, I also noticed a slight bit of inconsistency in that using gw to jump ahead maintains the selection when in visual mode, but using gw to jump to a word within the selection does not. Assuming this is unintentional, and the expected behaviour is more like that of Vim where the anchor would remain fixed and the selection cursor would just move backwards, I'd be interested to know if this has also been fixed as part of the above implementation or if this is something to be looked at separately? Happy to have a crack at it myself if the latter.

Screen.Recording.2024-06-02.at.14.16.48.mov

@the-mikedavis the-mikedavis self-assigned this Jun 3, 2024
@the-mikedavis
Copy link
Member Author

That's intentional: #8875 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements
Projects
None yet
Development

No branches or pull requests

7 participants