[WIP] row-wise scrolling

[zwegner]

trafficstars

This will almost surely break some things. Right now it just changes C-E/Y behavior to scroll row-wise when in wrap mode.

There's some weird behavior with the cursor now near the bottom of the screen, or when it would move off the top.

[bfredl]

Nice work! Only glitch I noticed so far: with set wrap list, and using C-E/C-Y scrolling (and narrow screen so that most lines are wrapped), every screenline gets prepended by "precedes" listchar which only should be used in nowrap mode. Turning off list however shows the 'showbreak' char correctly only on continuation lines.

[zwegner]

Yes, I see the problem there. It looks like, despite the documentation, the "precedes" listchar was used in wrap mode if w_skipcol is nonzero, which makes sense. What doesn't make sense is that it doesn't check what row is being rendered, so every line of the screen has the "precedes" char. This bug is present in master actually (without these changes), if you view one line longer than the entire screen and the cursor past the first screen's worth of characters. The fix is pretty simple (just check that row==0 when w_skipcol>0)

[bfredl]

if you view one line longer than the entire screen and the cursor past the first screen's worth of characters. The fix is pretty simple (just check that row==0 when w_skipcol>0)

Indeed. Would you mind break out this fix to a separate PR (with test) for master?

[zwegner]

Indeed. Would you mind break out this fix to a separate PR (with test) for master?

Sure thing.

Also, it looks like some more scrolling tests need to be written for master, since this now passes the test suite (aside from linting), and it really shouldn't :)

[bfredl]

I don't think <c-e> and <c-y> in particular have been tested much, rather just scrolling that results from cursor movement ( i e j, k, [N]G and so forth).

[g-i-o-r-g-i-o]

Thank you very much, this is a fantastic news, I've been needing this so much for all these years :-) I'm not sure that I'll be able to compile it from your repository, but I'll try. First thing, I'm going immediately to sponsor nvim right now!

update: I'll test it at home on ubuntu, because compiling at work under windows requires a lot of unholy software installations

[zwegner]

I've been slowly improving this over the last couple weeks. Here's some notes.

Things I'd like feedback on

• The option. I've used "scrollrowwise" or "scrw" for short. I've made this buffer-local, even though it might initially seem to be more related to windows than buffers. My reasoning is that I was primarily working on this feature to complement terminal reflow, where the option should be set by default in terminal buffers, no matter which window they're in. Additionally, it would make sense to want to set the option only on certain filetypes, or buffers with very long lines.
• Interactions with resizing. Right now the first character on the screen is used as the anchor point when resizing, which leads to weird behavior when scrolling up: the first character in a line might not line up with column 0, so when the screen is scrolled up enough to show the first character, the whole line will shift to the left. It's unclear what the "right" behavior should be here, since there's not a natural anchor point when resizing.

Work left to do

• Add documentation
• The <C-U>/<C-D> methods of scrolling don't redraw properly with "number" set. These (and other scrolling methods) still need to be changed to work row-wise.
• Fix bugs. There's at least a couple I haven't tracked down causes for yet.
• Handle folds
• Handle scrolloff
• Clean up invalidation. The implementation now just punts on efficiently redrawing the window after row-wise scrolling, and just invalidates the entire screen. Additionally, the wp->w_valid handling should be checked. I'm not entirely sure which pieces of data should be incrementally updated and which should be marked invalid.
• Non-determinism in tests. I sometimes get "Screen changes were received after the expected state" error. I'm not yet sure why this is happening.)
• Move the advance_line_ptr_by_width/get_line_width functions. I just threw them in right above the new scrolling functions. These could probably be used in other places of the codebase, too.

Performance

This is not particularly efficient. (n)vim just was't designed to support very long lines properly, and it would take some major restructuring to change this. There are many O(n) algorithms used on lines, not just in my new code. For example, strlen is used all over the place to get the length of a line, even though this could be derived from the memline data structures. Even worse, there are a ton of places throughout the codebase that rely on accessing lines as raw, contiguous bytes. There's basically no way to edit a very long line without reallocating and copying the entire line. Trying to cache information about buffer lines (for instance, how many screen columns they take up) would be very difficult.

Annoying aspects of the (n)vim codebase

• The scroll directions are reversed in the code vs. the documentation. I did this to match the rest of the code, which is backwards for some reason.
• w_cursor.col is a byte offset, not a column, despite being a colnr_T. Most other uses of colnr_T are actual screen columns.
• colnr_T is an int. It's unclear how hard it would be to make this a long, if we ever wanted to support 2GB+ lines

[bfredl]

Nice, I will take a look.

I've made this buffer-local, even though it might initially seem to be more related to windows than buffers. My reasoning is that I was primarily working on this feature to complement terminal reflow, where the option should be set by default in terminal buffers, no matter which window they're in. Additionally, it would make sense to want to set the option only on certain filetypes, or buffers with very long lines.

Note window options are (to some extent) actually being tracked per buffer. If you create buffer 1 in a window, and setlocal some window options, then every new window displaying buffer 1 will also use the same options for that buffer (until you change them locally for that window). What window option provides (over a buffer option) is that you can show a buffer in two windows at the same time and use a different option value for each. I think this would make sense for 'scrollrowwise'

[bfredl]

w_cursor.col is a byte offset, not a column, despite being a colnr_T. Most other uses of colnr_T are actual screen columns.

In general "column" actually refers to byte counts, while "vcols" are used for screen columns.

Improving long-line support in general is hard. It is a bit of a chicken-and-egg problem. Memline cannot effectively store buffer text as framents of lines (i e store a single line across multiple nodes in the tree, or replace the data structure altogether), as edit/screen code expects to get/write lines as a continous char *. There is no use for some part of the editing/screen code to start support line fragments, until memline has been refactored to actually use multiple fragments per line.

The "compromise" I could think of is to have memline use a better representation internally, and temporarily support two interfaces (i e transparently translate back/forth to the char * representation when needed), thought this would probably mean that nvim gets less effective for many operations during the transition period (i e until almost all code has been ported over).

[zwegner]

Note window options are (to some extent) actually being tracked per buffer. If you create buffer 1 in a window, and setlocal some window options, then every new window displaying buffer 1 will also use the same options for that buffer (until you change them locally for that window). What window option provides (over a buffer option) is that you can show a buffer in two windows at the same time and use a different option value for each. I think this would make sense for 'scrollrowwise'

Interesting. How is this different from copying options from the current window? Is there some API for, say, splitting a window, but opening a different buffer in the split, where the second buffer's options are used for the new window instead of the window it was split from? If the second buffer is already open in multiple windows, which window does it copy options from? I guess I just don't fully understand the semantics of global/buffer/window options.

I'm not sure which is a more important use case for having different values of the row-wise option: viewing the same buffer in multiple windows like you say, or viewing multiple buffers in the same window. For instance, <C-^> between a terminal window and some code.

[zwegner]

In general "column" actually refers to byte counts, while "vcols" are used for screen columns.

Right, I just had to figure this out by reading the uses in the code, it doesn't seem to be documented anywhere. And there's lots of different variables with col/curs in the name, and they all tend to use colnr_T.

Improving long-line support in general is hard. It is a bit of a chicken-and-egg problem. Memline cannot effectively store buffer text as framents of lines (i e store a single line across multiple nodes in the tree, or replace the data structure altogether), as edit/screen code expects to get/write lines as a continous char *. There is no use for some part of the editing/screen code to start support line fragments, until memline has been refactored to actually use multiple fragments per line.

The "compromise" I could think of is to have memline use a better representation internally, and temporarily support two interfaces (i e transparently translate back/forth to the char * representation when needed), thought this would probably mean that nvim gets less effective for many operations during the transition period (i e until almost all code has been ported over).

Agreed. This is pretty much what I had in mind, but it would be a huge amount of work. It's maybe off-topic here though, is there some open bug or anywhere more appropriate to discuss this?

[bfredl]

API for, say, splitting a window, but opening a different buffer in the split, where the second buffer's options are used for the new window instead of the window it was split from?

This is the default (if the second buffer already was opened)

If the second buffer is already open in multiple windows, which window does it copy options from?

I think the window where the buffer was most recently shown. At least if the buffer is currently visible that takes precedence.

I'm not sure which is a more important use case for having different values of the row-wise option: viewing the same buffer in multiple windows like you say, or viewing multiple buffers in the same window. For instance, <C-^> between a terminal window and some code.

But the point is that window options support both, using setlocal. When you switch buffer within window it will reuse the value previously set for that buffer (from the same window if possible, otherwise from some other window).

[bfredl]

It's maybe off-topic here though, is there some open bug or anywhere more appropriate to discuss this?

Not what I know of. Most recently we have discussed this on gitter.. Feel free to open an issue if you want to.

[zwegner]

But the point is that window options support both, using setlocal. When you switch buffer within window it will reuse the value previously set for that buffer (from the same window if possible, otherwise from some other window).

Ah, ok. I clearly need to read up on how options really work, then.

[justinmk]

For example, strlen is used all over the place to get the length of a line, even though this could be derived from the memline data structures

https://github.com/neovim/neovim/issues/859 at least somewhat covers this.

[unclechu]

What’s left to implement apart from resolving the conflicts?

[unclechu]

I also wonder if it’s even possible (takes less effort than rewriting it from scratch) to resolve the conflicts taking into account that last commit was made a year ago?

[zwegner]

I also wonder if it’s even possible (takes less effort than rewriting it from scratch) to resolve the conflicts taking into account that last commit was made a year ago?

Haha, no :) I just rebased it to master without much trouble (though I had locally rebased it a while back, so it wasn't as diverged as it appeared). Most of the changes in this branch are to pieces of code that are rarely touched...

What’s left to implement apart from resolving the conflicts?

I forget the exact details of what still needs to be done, but this branch isn't too far from being mergeable IMO (note that this doesn't mean it's ready for mass usage, just that it's ready to be an opt-in experimental feature).

The two main things this still needs are documentation (noting the bad performance with long lines, and possible bugs), and more testing, particularly around the interactions of row-wise scrolling with the numerous features that interact with it like folding.

[zwegner]

A couple more things I thought of looking through some of my WIP uncommitted stuff:

• IIRC there are known breakages with this feature combined with breakindent or cpoptions+=n. I believe @justinmk said he was fine with removing cpoptions entirely, which would be nice (but out of scope here).

• I have a few "XXX" comments scattered about. In each place there's logic I don't think is correct (and it'd take some study to determine the right thing to do)

[justinmk]

IIRC there are known breakages with this feature combined with breakindent or cpoptions+=n. I believe @justinmk said he was fine with removing cpoptions entirely, which would be nice (but out of scope here).

Yes. No way will cpoptions=n block this feature.

(cpoptions=n is very strange feature anyway: if horizontal space is so precious why is anyone using :set number instead of :set ruler ...? Also Nvim UIs are extensible so features like cpoptions=n are just completely random)

[yorik1984]

Waiting this feature!

[matu3ba]

Workaround (bash) for use case "copy last used command to reproduce broken output with sufficient width": Alt-o and Alt-h to get into visual mode for the vi keybindings. Then use f for file lookup or v for a tmp file that can be copied from.

This assumes that ~/.inputrc contains

set editing-mode vi
set show-mode-in-prompt on

and has the drawback, that the code on the line is executed after the editor instance with tmp buffer is closed.

[luukvbaal]

Looks like Bram has taken it upon himself to start implementing this feature for vim as well. I was wondering if you are still up for porting/updating this PR if/when it becomes stable in vim @zwegner? IDK if the vim implementation has borrowed from this PR at all but I'm sure you will be more comfortable with the relevant parts of the code nevertheless. If not though I might be willing to look into it as this is currently a caveat for my new 'splitscroll/keep' option (which is what seems to have prompted Bram to work on this feature🎉).

[zwegner]

I can't say I'm thrilled at the idea of throwing away the huge amount of work I did here and porting Bram's code, sorry. Like I said before this wasn't too far from being mergeable. Not sure how far along his work is since its now split over several patches but it looks completely independent of mine.