hlissner / evil-snipe

2-char searching ala vim-sneak & vim-seek, for evil-mode
MIT License
340 stars 25 forks source link

When finding a match outside of the visible screen, make it center? #11

Closed ghost closed 9 years ago

ghost commented 9 years ago

Currently, it seems like when you find a match outside of the visible buffer, the match is on the very last visible line in the buffer.

I.e., searching for ge shows def genome_estimator(species): with the rest of the function not visible, which feels very awkward.

hlissner commented 9 years ago

Rather than hack this in as an edge case for buffer scoped searches, I'll figure out a more elegant solution this weekend. In the meantime, here is a potential workaround:

(defadvice evil-snipe--seek (after evil-snipe-center-after-search activate)
  (evil-scroll-line-to-center (line-number-at-pos)))
hlissner commented 9 years ago

I've done something slightly different. Instead of auto-centering the window (which I find a bit jarring), if evil-snipe-auto-scroll is non-nil, then the window will "scroll" along with the cursor, so the cursor stays on the same virtual line.

Let me know what you think. If you still prefer auto-centering, I'll make evil-snipe-auto-scroll accept 'center as a setting.

ghost commented 9 years ago

Sounds much better, can't wait to try it out!

ghost commented 9 years ago

The auto-scrolling you implemented is good, but I think we can make it feel more natural.

The first thing to implement would be to not scroll if the next match is within the visible buffer.

The second thing to implement would be to ensure that there are at least 3 lines visible above or below after finding a match (currently, if you search from the very first line your match will be shown at the very top of the buffer too (it often feels very unnatural not being able to see your match in its context).

The second point above means that you should scroll if the next match is very close to the top or the bottom of the buffer even if visible (this contradicts point one).

I wanted to try to implement the first one to begin with. In the internal function evil-snipe--seek there are the following lines:

(when evil-snipe-auto-scroll
  (setq new-orig-point (point))
  (evil-scroll-line-down (- (line-number-at-pos) (line-number-at-pos orig-point)))
  (goto-char new-orig-point))

I tried to change them to the following:

(when evil-snipe-auto-scroll
  (setq new-orig-point (point))
  (when (or (> window-start new-orig-point)
            (< window-end new-orig-point))
    (evil-scroll-line-down (- (line-number-at-pos) (line-number-at-pos orig-point))))
  (goto-char new-orig-point))

Where window-start and window-end were set to the values of their corresponding functions in the let* at the beginning of evil-snipe--seek. This made the function act strangely, unfortunately.

Do you agree to that these changes would be good, and if so do you have any tips for implementing them? I wanted to make some pull requests, but I am an elisp newb still.

hlissner commented 9 years ago

...ensure that there are at least 3 lines visible above or below after finding a match...

Question: is there any reason why (setq scroll-margin 3) can't achieve this effect for you?

The second thing to implement would be to not scroll if the next match is within the visible buffer...

This was how it used to be. You can get this by disabling evil-snipe-auto-scroll. Now, if you just want to make sure there are X lines surrounding an off-screen (or edge) match, I think scroll-margin is your guy. If you want the window to center on the match (if it's outside the visible buffer), that is something I can implement, but my impression was that it was quite jarring.

Let me know what you think.

ghost commented 9 years ago

How about something like natural-scroll, that does not scroll unless outside of the visible buffer or the match is very close to the edge? Thanks for the tips.

You are probably right that not centering the matches is better, I imagine it would be kinda irksome too, trying to imagine it.

hlissner commented 9 years ago

Forgive me, but I'm confused. What is natural-scroll?

I mentioned scroll-margin (a built-in emacs variable) because it ensures whenever scrolling occurs that there are at least N lines between the top/bottom of the window and the cursor. Was that not what you wanted?

ghost commented 9 years ago

Ah, so no need to change the function, I can just set (scroll-margin). Thanks.

So by setting that variable and disabling autoscroll I should get the behavior I want. Thanks again.