BTW the motivation behind trying to merge it into core is that the "stabilization" done by the plugin is sometimes not fast enough in faster terminals. Thus resulting in flickering when restoring the topline. That and the overall gimmickyness of the plugin, never changing the scroll position at all is of course better than saving and restoring it.

I'm not a neovim dev, but I have some immediate thoughts:

• First, this is cool, I would be happy to see it merged!
• I think you should also add documentation. E.g. in runtime/doc/quickref.txt and runtime/doc/options.txt.
• Would it make sense to add a test? Not sure how to write such a test, though.

* I think you should also add documentation. E.g. in `runtime/doc/quickref.txt` and `runtime/doc/options.txt`.

Definitely. I refrained from doing so as I was still looking for general feedback on the feasibility of the PR. The PR turned out to be easier than I imagined though so I guess I can draft the help docs.

* Would it make sense to add a test? Not sure how to write such a test, though.

Not sure about this either.

I'm also wondering about the etiquette of opening PRs agains vim. I have no interest in doing so myself but I quickly checked and this part of core vim seems similar to nvim still.

I'm also wondering about the etiquette of opening PRs agains vim. I have no interest in doing so myself but I quickly checked and this part of core vim seems similar to nvim still.

The etiquette is very much to contribute to Vim, if at all appropriate :)

In this case, you are right that this PR is directly applicable to Vim. The best course of action is then indeed to first make the PR to Vim, see what they think, and if they merge it, port that patch.

This way both projects profit from the new feature, and we avoid needless divergence in case Vim decides on slight changes to, say, the abbreviated option name.

(If Vim rejects the feature, we can reopen this PR, now with the knowledge that we don't need to take upstream into account.)

The etiquette is very much to contribute to Vim,

I guess I'll do that then.

• This is definitely wanted and will likely become a Nvim default, so thank you.
• Also makes sense to see if upstream Vim wants this, please let us know the result of that.

Adding a screen test is easy, see test/functional/example_spec.lua for example. You can run the test like this:

TEST_FILE=test/functional/example_spec.lua make functionaltest

I didn't find an existing test file where this would obviously fit, but maybe one of these:

test/functional/ui/screen.lua
test/functional/ui/screen_basic_spec.lua
test/functional/ui/multigrid_spec.lua

else we might need a new file, test/functional/ui/window_spec.lua.

Are you sure we would want this as default? Isn't the point of the scrolling to ensure the split doesn't cover the last position of the cursor?

Isn't the point of the scrolling to ensure the split doesn't cover the last position of the cursor?

The current :split behavior scrolls to keep the cursor at the same relative position, e.g. 30% or 50%.

I would expect 'splitscroll=false' to scroll if the cursor position would be hidden, since there is not yet an internal concept of a window with an off-screen cursor. This PR needs tests for both cases in any case.

(btw, does this imply we need a number or flags option instead of a boolean? because there are several possible behaviors.)

Hmm, sounds like the current behaviour is naive/simple and now we just want to make it "smart". I'd contemplate not even making an option for this and just change the current behaviour, unless there's a good argument I'm not realising.

(btw, does this imply we need a number or flags option instead of a boolean? because there are several possible behaviors.)

Can you list these?

Also note we need to make sure we always respect 'scrolloff'.

Hmm, sounds like the current behaviour is naive/simple and now we just want to make it "smart"

Is this PR not just allowing currently existing "smart" behavior to be disabled?

(btw, does this imply we need a number or flags option instead of a boolean? because there are several possible behaviors.)

Yes this makes sense, the option was also available in the plugin. There it was named force as in "force stabilizing even when cursor is behind newly opened window". Not sure what an apt flag name would be if the option name stays splitscroll. Perhaps it should be reconsidered?

Not sure what an apt flag name would be if the option name stays splitscroll. Perhaps it should be reconsidered?

• could just be a number option, see 'laststatus'.
• future: also consider vertical scroll

(btw, does this imply we need a number or flags option instead of a boolean? because there are several possible behaviors.)

Can you list these?

1. scroll to maintain relative position (current behavior)
2. don't scroll except as needed to keep cursor row visible (and subject to 'scrolloff')
3. never scroll, even if cursor row will be hidden or 'scrolloff' would be violated

Are you sure we would want this as default? Isn't the point of the scrolling to ensure the split doesn't cover the last position of the cursor? I'd contemplate not even making an option for this and just change the current behaviour

Even when the cursor would not be hidden behind the split, currently scrolling happens just to "keep the same relative position". To me this is unnecessary but I guess it could be up to personal preference. Hence the option, but perhaps not necessary if we think the current default is undesired.

Keeping the relative position is a valid use-case. Could avoid a new option by changing the core behavior, then users who want "keep relative position" could use a WinScroll (which apparently also means "WinResize"...?) event, but idk if WinScroll/WinResize gives the necessary info in v:event yet.

But that doesn't resolve the question about letting the cursor row get hidden: that does not seem like a good default behavior, and raises a lot of questions. If we are going to entertain the idea of letting the cursor row get hidden, that would need to be a third choice.

I thought it could look something like the latest commit. Perhaps the solution is too naive though, I never use scrolloff so I'm not sure what to consider.

Vim upstream will consider the feature if we can agree on what it will do. They will require a test as well, testdir/test_window_cmd.vim seems appropriate. Will adding a test there be sufficient or are the legacy tests deprecated here?

Will adding a test there be sufficient

Yes

Keeping the relative position is a valid use-case. Could avoid a new option by changing the core behavior, then users who want "keep relative position" could use a WinScroll (which apparently also means "WinResize"...?) event, but idk if WinScroll/WinResize gives the necessary info in v:event yet.

Ok, let's just introduce an option.

But that doesn't resolve the question about letting the cursor row get hidden: that does not seem like a good default behavior, and raises a lot of questions. If we are going to entertain the idea of letting the cursor row get hidden, that would need to be a third choice.

I'm working on the assumption that any new behaviour we add, doesn't break 'scrolloff', and, never hides (or moves) the cursor. For that reason I don't think we should add a 3rd form of behaviour.

In terms of how we do it, I think we should just add 'splitoptions', even if it only contains one option.

I thought it could look something like the latest commit. Perhaps the solution is too naive though, I never use scrolloff so I'm not sure what to consider.

We need to check if the cursor will remain in the window and obeys 'scrolloff'. If it doesn't then we need to scroll appropriately, and not necessarily the old scroll behaviour, but instead the minimum amount.

For that reason I don't think we should add a 3rd form of behaviour.

I don't see any harm in adding the third option for those who prefer it. It was the default for my plugin, and never got any complaints. I agree that the cursor position should never violate scrolloff for the active window. However, I think delaying the scrolling to when the cursor is moved back the window where scrolling would be required to honor scrolloff (current behavior of this PR for splitscroll=2), results in better UX.

but instead the minimum amount.

Yeah I considered this too and I agree it would probably be better behavior but I wasn't sure it would be worth the hassle.

I think I'll stop updating this PR until some movement happens in vim/vim#10682. Furthermore it seems like we still haven't agreed on what the option should be so perhaps updating either PR is futile but I thought I would cross post https://github.com/vim/vim/pull/10682/files#r917474316 to see if there's any feedback here.

Any particular reason this was moved to the 0.9 milestone btw @clason? Is it too late in the 0.8 cycle? Is anything else needed for this PR? I can keep updating it with patches as vim bug reports/improvements keep coming in but 0.9 seems far away.

Yes, it's (much) too late; we've been in feature freeze for a week already.

Yes, it's (much) too late; we've been in feature freeze for a week already.

Whoops, totally missed that. Haven't really kept up with neovim development while working on the vim patch😅

Cross posting from vim/vim#10682 (comment):

Adding the option to keep the topline the same might actually be simpler to implement.

I think it might amount to simply guarding this clause behind the option. If users really want this I could look into it but the option shouldn't get needlessly complicated imo.

So yeah circling back to this after some initial user bug reports/improvements to the feature, I really do think providing this option is as simple as guarding the winrow != prev_winrow clause behind an alternate option value. We are then simply (in)validating botline for each window, and correcting the cursor position for curwin.

To reiterate, instead of having the previously proposed options for scroll behavior depending on whether or not the cursor would be valid ({"normal", "noscroll", "neverscroll"}, we dropped this because changing the cursor position was considered not to be that egregious when splitting a window, and as a compromise we add the previous cursor position to the jumplist).

Now the proposal is the have 3 separate scroll behaviors for 'splitscroll' because user(s)(@ychin) might prefer one over the other:

• "normal": default behavior, keep the same relative cursor position in both new and old split.
• "counterscroll": keep the text that was already visible in a window on the same screenlines for each window that is resized/moved by a split command, effectively "counter-scrolling" the topline to keep the text stable. (current 'nosplitscroll' behavior).
• "noscroll": keep the same topline in all windows, effectively only keeping the text stable in the uppermost split.

Not sure "counterscroll" is very telling, can anyone think of a better name for these option values? Do any more users see the need for the third option? See vim/vim#11258 for the proposal.

LGTM. I pushed a fixup commit with some small changes.

Yay! I see I missed some of my own (style) changes from the vim patches. So thanks for the fixup and merge!

Unfortunately maintainers weren't able to review this before the 0.8 release. I would be willing to make a backport PR but AFAIK 0.x.y releases are only for bug fixes.