-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Make editor remember the latest search register #5244
Make editor remember the latest search register #5244
Conversation
452abc2
to
70257ae
Compare
Update: Recent changes in the editor fixed the part where commands do not respect immediate register selection (i.e. That's why this PR is still relevant IMO, as the 2nd part of the PR is to make the editor remember the latest register used to kick off a search, and uses that by default unless a prefix is explicitly specified. In this case users can drop the prefix and simply press Also update the PR title to better reflect the only part left for this PR. |
yeah I think this is a nice QOL improvement and makes sense. Implementation is pretty small/unintrusive too 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems potentially too sticky to me: /
is meant to be the default when you don't provide a register but this change makes any register that you select the default until you explicitly select another one. I would prefer to see this as last_search_register: Option<char>
on the editor. The functions that start searches like searcher
and search_selection
would set/clear the field if a register is/isn't selected while the functions that continue a search like search_next_or_prev_impl
and make_search_word_bounded
would only read from the field.
Not really though? If you start a new search with just I too agree that if it were that sticky it's a problem, but that's not the case here. I also just tested my branch and it's indeed working as expected (i.e. reverts back to |
Ah yep I see now. Let's rename it to |
70257ae
to
544355e
Compare
Renamed the field the (Oops. Maybe I shouldn't have rebased... Now the compare button is pretty much useless) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Diff is small enough to review quickly so no worries
:) Maybe one more thing: we could move last_search_register
to editir.registers
. This avoids adding more field to Editor
Sure thing! But do you mean making this a Edit: I pushed the |
544355e
to
8dced74
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A public field is probably fine in this case 👍 I don't think it makes sense to encapsulate here. For me it's just about bundling related fields together instead of making Editor
the kitchen sink for everything
Resolves #5112.
This PR makes changes to the behavior of search commands:
search
/rsearch
/global_search
command;search_next
,search_prev
,extend_search_next
, andextend_search_prev
now uses the search register to continue the search;search_selection
now uses the selected register. It also updates the search register;make_search_word_bounded
defaults to using the search register to make thesearch_selection
-make_search_word_bounded
flow more ergonomic, while also accepting register selection.Overall, this PR should make the whole family of search commands play more nicely with register selection.