File-scrolling by moving the cursor -> enable scrolling all the way to file ends

[Manosmer]

This is an attempt to fix #4491. Previously, file scrolling was performed by scrolling the view of the doc and, then, repositioning the cursor accordingly. In this version, I implemented the opposite. Now, the cursor is the one moving the view. In other words, the view now clamps around the cursor based on the cursor's offset. The implementation still respects the scolloff setting.

Not ready yet. It requires some further testing.
Note: I recommend building with --profile=opt. Otherwise, scrolling with the mousewheel is laggy even with the previous code's version.

[CptPotato]

Does this mean that with this change, if I Ctrl+u and then Ctrl+d I'm back to the initial position (unless I hit the document start/end)?

[Manosmer]

Does this mean that with this change, if I Ctrl+u and then Ctrl+d I'm back to the initial position (unless I hit the document start/end)?

Simply put, yes. And not only with Ctrl+u/Ctrl+d. This change affects all methods of file scrolling, including mousewheel, Ctrl+b/Ctrl+f etc. It is the doc's view now that has to adjust itself around the cursor.
In more technical terms, the offset calculated by the keybindings now moves the cursor and not the view. The view just wraps around the cursor to accommodate the need to keep the cursor inside the viewport.

I know that this means that, for example, if you wanted to peek under the cursor without moving the cursor it's now not possible. I could implement it so that the change has the same functionality as before and just detect when it has to make that final jump to the file edges, but the current implementation felt more natural to me.

To be fair, I think that if the previous functionality is preferable we should stick with the previous implementation in general. I believe that scrolling the viewport(just like in previous version) and making the cursor jump to file edges(just like it is described in the issue) are orthogonal, because the latter implies controlling the cursor and not the view. It would not make sense to me to scoll the view all the way to the end of the file and suddenly have a cursor jump at the end.

The other way we could do this is to implement just what neovim does. Stick the cursor to its relative position to the viewport no matter where that position is and scroll only the view until it hits the file bounds, where cursor movement takes over.

[CptPotato]

Thanks for elaborating. I prefer this new behavior.

[Manosmer]

Thanks for elaborating. I prefer this new behavior.

Thank you! I will take that as a compliment

[the-mikedavis]

I think that if the previous functionality is preferable we should stick with the previous implementation in general. I believe that scrolling the viewport... and making the cursor jump to file edges... are orthogonal

I agree: the new behavior seems backwards to me. Scrolling should move the view and the cursor should only move if necessary to keep it in view.

Being able to scroll so that we're on the last line should be possible though without changing this approach. In order to scroll to the last line, we should change this check:

helix/helix-term/src/commands.rs

Lines 1375 to 1381 in 46eb185

	let last_line = view.last_line(doc);
	if direction == Backward && view.offset.row == 0
	|| direction == Forward && last_line == doc_last_line
	{
	return;
	}

Currently it prevents scrolling after the last line is visible when scrolling forward. We can change the condition to so that we stop scrolling forward only when we're on the last line: view.offset.row == doc_last_line.saturating_sub(scrolloff).

With this change we will also need to switch the order of the min and max calls here:

helix/helix-term/src/commands.rs

Lines 1397 to 1400 in 46eb185

	let line = cursor
	.row
	.max(view.offset.row + scrolloff)
	.min(last_line.saturating_sub(scrolloff));

let line = cursor
    .row
    .min(last_line.saturating_sub(scrolloff))
    .max(view.offset.row + scrolloff);

[pascalkuthe]

This PR has gone stale and this was actually implemented im #5420