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 follows screen #46

Merged
merged 4 commits into from
Jun 28, 2021
Merged

Conversation

quark-zju
Copy link
Contributor

Add a config option to control whether search should follow the current screen (line position).

The default is yes, to match less/vim behavior.

Match less / vim behavior: after scrolling up or down, if the match becomes
out of the current screen, re-search from the current screen.

The old behavior is preserved by setting search_follow_screen to false.
It seems "x of y matches on z lines" doesn't get updated properly on
matching next/prev. Make it so.
Make n N load the last search pattern automatically. Matches less behavior.

Resolves markbt#38.
@markbt
Copy link
Owner

markbt commented Jun 28, 2021

Thanks! This was on my TODO-list.

Rather than making this a config option, I think it would be better if these were new keybinding actions and match movements (Action::NextMatchOnScreen => MatchMotion::NextOnScreen, etc.). We can change the default bindings to use the OnScreen variants to match less, but if you also want the behaviour of moving exactly through the matches (even if you've scrolled away to look at something else briefly), we can have an extra shortcut for that.

Makes it possible to mix use different search behaviors.
@markbt
Copy link
Owner

markbt commented Jun 28, 2021

Looks good!

Not for this change, but a difference with less that this highlights is that we always jump so that the target line is in the center, which can look odd for "next in screen" as it may make us jump in the opposite direction to recenter things.

I think the way to fix that is to make scroll_to take an option for how we want to scroll to a particular target. The options should be AtTop, AtCenter, AtBottom and MakeVisible. The first 3 should be fairly self explanatory. MakeVisible should prefer not to scroll, but if the target line is off-screen, then scroll until it becomes visible.

(BTW, another thing that needs to be implemented is dealing with matches that are not in the visible part of the line (either off to the right if wrapping is off, or not on the first line if wrapping is on. This is on my TODO list, but I haven't had time yet)

@markbt markbt merged commit e17b318 into markbt:master Jun 28, 2021
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.

None yet

2 participants