Unimpaired movements cause display to scroll unnecessarily

[rcorre]

Summary

Sometimes I want to jump to a nearby diagnostic, maybe even on the same line my cursor is currently on.
Sometimes this causes the display to scroll up or down, which is jarring.
If the diagnostic is already visible, we shouldn't scroll (except as needed to respect scrolloff).
Navigating to a diagnostic on the same line as the cursor certainly shouldn't scroll.

https://asciinema.org/a/1HAs9gPmEAaCrkpBJ2zSloC9X

Reproduction Steps

I tried this:

1. hx example.cpp
2. 40j
3. ]d

I expected this to happen:
Cursor moves to diagnostic, display stays at same scroll level.

Instead, this happened:
Cursor moves to diagnostic, display scrolls down.

Helix log

~/.cache/helix/helix.log

please provide a copy of `~/.cache/helix/helix.log` here if possible, you may need to redact some of the lines

[helix.log](https://github.com/helix-editor/helix/files/10886882/helix.log)

Platform

Linux

Terminal Emulator

st 0.8.5

Helix Version

22.12-294-g6494fc1d

[pascalkuthe]

This actually is on purpose as we manually call align_view (instead of just letting ensure_cursor_in_view handle it. The rational is that if you jump to a diagnostic then you want that to be in the center of the screen to see the context/what is going on.

That being said we are not consistent here (anything TS based or goto_next_change). Many other unimpaired motions do not center the view. CC @the-mikedavis what is the intended behavior here and do we want to make this configurable?

[the-mikedavis]

Ah, this looks like an oversight by me in #4713. I agree with

If the diagnostic is already visible, we shouldn't scroll (except as needed to respect scrolloff).

I think that align_view was to make the goto_*_diag commands closer to the diagnostic picker. It makes sense for the diagnostic picker to center the view IMO but for these unimpaired commands it's jarring and inconsistent with the others as you say. I think we should remove the align_view calls 👍

[the-mikedavis]

(Plus you can accomplish the current behavior by using the align_view_center command after the goto_*_diag commands.)

[archseer]

Shouldn't we actually use the same code we use for jumping to search results? ensure_cursor_in_view_center: center if offscreen, but not if it's on the same screen