kevin/neovim
1
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2024-12-19 18:55:14 -07:00
Code Issues Packages Projects Releases Wiki Activity

fix(scrollbind): properly take filler/virtual lines into account

Browse Source 
Problem:

`'scrollbind'` does not work properly if the window being scrolled
automatically contains any filler/virtual lines (except for diff filler
lines).

This is because when the scrollbind check is done, the logic only
considers changes to topline which are represented as line numbers.

Solution:

Write the logic for determine the scroll amount to take into account
filler/virtual lines.

Fixes #29751
This commit is contained in:
Lewis Russell 2024-07-17 12:23:15 +01:00 committed by Lewis Russell
parent c9b129a02a
commit 573a71469d
11 changed files with 538 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
runtime/doc/news.txt
View File

@ -183,7 +183,13 @@ CHANGED FEATURES *news-changed*
These existing features changed their behavior.
• N/A
• 'scrollbind' now works properly with buffers that contain virutal lines.
Scrollbind works by aligning to a target top line of each window in a tab
page. Previously this was done by calculating the difference between the old
top line and the target top line, and scrolling by that amount. Now the
top lines are calculated using screen line numbers which take virtual lines
into account.
==============================================================================
REMOVED FEATURES *news-removed*

15
src/nvim/decoration.c
View File

@ -887,7 +887,8 @@ bool decor_redraw_eol(win_T *wp, DecorState *state, int *eol_attr, int eol_col)
static const uint32_t lines_filter[4] = {[kMTMetaLines] = kMTFilterSelect };
int decor_virt_lines(win_T *wp, linenr_T lnum, VirtLines *lines)
/// @param apply_folds Only count virtual lines that are not in folds.
int decor_virt_lines(win_T *wp, int start_row, int end_row, VirtLines *lines, bool apply_folds)
{
buf_T *buf = wp->w_buffer;
if (!buf_meta_total(buf, kMTMetaLines)) {
@ -896,15 +897,14 @@ int decor_virt_lines(win_T *wp, linenr_T lnum, VirtLines *lines)
return 0;
}
assert(lnum > 0);
int row = lnum - 1;
MarkTreeIter itr[1] = { 0 };
if (!marktree_itr_get_filter(buf->b_marktree, MAX(row - 1, 0), 0, row + 1, 0,
if (!marktree_itr_get_filter(buf->b_marktree, MAX(start_row - 1, 0), 0, end_row, 0,
lines_filter, itr)) {
return 0;
}
assert(start_row >= 0);
int virt_lines = 0;
while (true) {
MTKey mark = marktree_itr_current(itr);
@ -915,7 +915,8 @@ int decor_virt_lines(win_T *wp, linenr_T lnum, VirtLines *lines)
bool above = vt->flags & kVTLinesAbove;
int mrow = mark.pos.row;
int draw_row = mrow + (above ? 0 : 1);
if (draw_row == row && !hasFolding(wp, mrow + 1, NULL, NULL)) {
if (draw_row >= start_row && draw_row < end_row
&& (!apply_folds || !hasFolding(wp, mrow + 1, NULL, NULL))) {
virt_lines += (int)kv_size(vt->data.virt_lines);
if (lines) {
kv_splice(*lines, vt->data.virt_lines);
@ -926,7 +927,7 @@ int decor_virt_lines(win_T *wp, linenr_T lnum, VirtLines *lines)
}
}
if (!marktree_itr_next_filter(buf->b_marktree, itr, row + 1, 0, lines_filter)) {
if (!marktree_itr_next_filter(buf->b_marktree, itr, end_row, 0, lines_filter)) {
break;
}
}

6
src/nvim/diff.c
View File

@ -2143,7 +2143,11 @@ int diff_check_with_linestatus(win_T *wp, linenr_T lnum, int *linestatus)
return 0;
}
if (!dp->is_linematched && diff_linematch(dp)) {
// Don't run linematch when lnum is offscreen.
// Useful for scrollbind calculations which need to count all the filler lines
// above the screen.
if (lnum >= wp->w_topline && lnum < wp->w_botline
&& !dp->is_linematched && diff_linematch(dp)) {
run_linematch_algorithm(dp);
}

2
src/nvim/drawline.c
View File

@ -1156,7 +1156,7 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, int col_rows, s
area_highlighting = true;
}
VirtLines virt_lines = KV_INITIAL_VALUE;
wlv.n_virt_lines = decor_virt_lines(wp, lnum, &virt_lines);
wlv.n_virt_lines = decor_virt_lines(wp, lnum - 1, lnum, &virt_lines, true);
wlv.filler_lines += wlv.n_virt_lines;
if (lnum == wp->w_topline) {
wlv.filler_lines = wp->w_topfill;

2
src/nvim/ex_cmds.c
View File

@ -2695,7 +2695,7 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum
*so_ptr = 999; // force cursor to be vertically centered in the window
}
update_topline(curwin);
curwin->w_scbind_pos = curwin->w_topline;
curwin->w_scbind_pos = plines_m_win_fill(curwin, 1, curwin->w_topline);
*so_ptr = n;
redraw_curbuf_later(UPD_NOT_VALID); // redraw this buffer later
}

21
src/nvim/ex_docmd.c
View File

@ -81,6 +81,7 @@
#include "nvim/os/os_defs.h"
#include "nvim/os/shell.h"
#include "nvim/path.h"
#include "nvim/plines.h"
#include "nvim/popupmenu.h"
#include "nvim/pos_defs.h"
#include "nvim/profile.h"
@ -5580,39 +5581,43 @@ static void ex_swapname(exarg_T *eap)
/// (1998-11-02 16:21:01 R. Edward Ralston <eralston@computer.org>)
static void ex_syncbind(exarg_T *eap)
{
linenr_T topline;
linenr_T vtopline; // Target topline (including fill)
linenr_T old_linenr = curwin->w_cursor.lnum;
setpcmark();
// determine max topline
// determine max (virtual) topline
if (curwin->w_p_scb) {
topline = curwin->w_topline;
vtopline = get_vtopline(curwin);
FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
if (wp->w_p_scb && wp->w_buffer) {
topline = MIN(topline, wp->w_buffer->b_ml.ml_line_count - get_scrolloff_value(curwin));
linenr_T y = plines_m_win_fill(wp, 1, wp->w_buffer->b_ml.ml_line_count)
- get_scrolloff_value(curwin);
vtopline = MIN(vtopline, y);
}
}
topline = MAX(topline, 1);
vtopline = MAX(vtopline, 1);
} else {
topline = 1;
vtopline = 1;
}
// Set all scrollbind windows to the same topline.
FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
if (wp->w_p_scb) {
int y = topline - wp->w_topline;
int y = vtopline - get_vtopline(wp);
if (y > 0) {
scrollup(wp, y, true);
} else {
scrolldown(wp, -y, true);
}
wp->w_scbind_pos = topline;
wp->w_scbind_pos = vtopline;
redraw_later(wp, UPD_VALID);
cursor_correct(wp);
wp->w_redr_status = true;
}
}
if (curwin->w_p_scb) {
did_syncbind = true;
checkpcmark();

53
src/nvim/normal.c
View File

@ -2092,17 +2092,23 @@ static void display_showcmd(void)
grid_line_flush();
}
int get_vtopline(win_T *wp)
{
return plines_m_win_fill(wp, 1, wp->w_topline) - wp->w_topfill;
}
/// When "check" is false, prepare for commands that scroll the window.
/// When "check" is true, take care of scroll-binding after the window has
/// scrolled. Called from normal_cmd() and edit().
void do_check_scrollbind(bool check)
{
static win_T *old_curwin = NULL;
static linenr_T old_topline = 0;
static int old_topfill = 0;
static linenr_T old_vtopline = 0;
static buf_T *old_buf = NULL;
static colnr_T old_leftcol = 0;
int vtopline = get_vtopline(curwin);
if (check && curwin->w_p_scb) {
// If a ":syncbind" command was just used, don't scroll, only reset
// the values.
@ -2115,10 +2121,9 @@ void do_check_scrollbind(bool check)
if ((curwin->w_buffer == old_buf
|| curwin->w_p_diff
)
&& (curwin->w_topline != old_topline
|| curwin->w_topfill != old_topfill
&& (vtopline != old_vtopline
|| curwin->w_leftcol != old_leftcol)) {
check_scrollbind(curwin->w_topline - old_topline, curwin->w_leftcol - old_leftcol);
check_scrollbind(vtopline - old_vtopline, curwin->w_leftcol - old_leftcol);
}
} else if (vim_strchr(p_sbo, 'j')) { // jump flag set in 'scrollopt'
// When switching between windows, make sure that the relative
@ -2129,14 +2134,13 @@ void do_check_scrollbind(bool check)
// resync is performed, some of the other 'scrollbind' windows may
// need to jump so that the current window's relative position is
// visible on-screen.
check_scrollbind(curwin->w_topline - (linenr_T)curwin->w_scbind_pos, 0);
check_scrollbind(vtopline - curwin->w_scbind_pos, 0);
}
curwin->w_scbind_pos = curwin->w_topline;
curwin->w_scbind_pos = vtopline;
}
old_curwin = curwin;
old_topline = curwin->w_topline;
old_topfill = curwin->w_topfill;
old_vtopline = vtopline;
old_buf = curwin->w_buffer;
old_leftcol = curwin->w_leftcol;
}
@ -2144,20 +2148,18 @@ void do_check_scrollbind(bool check)
/// Synchronize any windows that have "scrollbind" set, based on the
/// number of rows by which the current window has changed
/// (1998-11-02 16:21:01 R. Edward Ralston <eralston@computer.org>)
void check_scrollbind(linenr_T topline_diff, int leftcol_diff)
void check_scrollbind(linenr_T vtopline_diff, int leftcol_diff)
{
win_T *old_curwin = curwin;
buf_T *old_curbuf = curbuf;
int old_VIsual_select = VIsual_select;
int old_VIsual_active = VIsual_active;
colnr_T tgt_leftcol = curwin->w_leftcol;
linenr_T topline;
linenr_T y;
// check 'scrollopt' string for vertical and horizontal scroll options
bool want_ver = (vim_strchr(p_sbo, 'v') && topline_diff != 0);
want_ver |= old_curwin->w_p_diff;
bool want_hor = (vim_strchr(p_sbo, 'h') && (leftcol_diff || topline_diff != 0));
bool want_ver = old_curwin->w_p_diff
|| (vim_strchr(p_sbo, 'v') && vtopline_diff != 0);
bool want_hor = (vim_strchr(p_sbo, 'h') && (leftcol_diff || vtopline_diff != 0));
// loop through the scrollbound windows and scroll accordingly
VIsual_select = VIsual_active = 0;
@ -2174,16 +2176,19 @@ void check_scrollbind(linenr_T topline_diff, int leftcol_diff)
if (old_curwin->w_p_diff && curwin->w_p_diff) {
diff_set_topline(old_curwin, curwin);
} else {
curwin->w_scbind_pos += topline_diff;
topline = (linenr_T)curwin->w_scbind_pos;
if (topline > curbuf->b_ml.ml_line_count) {
topline = curbuf->b_ml.ml_line_count;
}
if (topline < 1) {
topline = 1;
}
curwin->w_scbind_pos += vtopline_diff;
int curr_vtopline = get_vtopline(curwin);
y = topline - curwin->w_topline;
// Perf: reuse curr_vtopline to reduce the time in plines_m_win_fill().
// Equivalent to:
// int max_vtopline = plines_m_win_fill(curwin, 1, curbuf->b_ml.ml_line_count);
int max_vtopline = curr_vtopline + curwin->w_topfill
+ plines_m_win_fill(curwin, curwin->w_topline + 1,
curbuf->b_ml.ml_line_count);
int new_vtopline = MAX(MIN((linenr_T)curwin->w_scbind_pos, max_vtopline), 1);
int y = new_vtopline - curr_vtopline;
if (y > 0) {
scrollup(curwin, y, false);
} else {

3
src/nvim/option.c
View File

@ -88,6 +88,7 @@
#include "nvim/os/os.h"
#include "nvim/os/os_defs.h"
#include "nvim/path.h"
#include "nvim/plines.h"
#include "nvim/popupmenu.h"
#include "nvim/pos_defs.h"
#include "nvim/regexp.h"
@ -2474,7 +2475,7 @@ static const char *did_set_scrollbind(optset_T *args)
return NULL;
}
do_check_scrollbind(false);
win->w_scbind_pos = win->w_topline;
win->w_scbind_pos = get_vtopline(win);
return NULL;
}

21
src/nvim/plines.c
View File

@ -712,7 +712,7 @@ bool win_may_fill(win_T *wp)
/// @return Number of filler lines above lnum
int win_get_fill(win_T *wp, linenr_T lnum)
{
int virt_lines = decor_virt_lines(wp, lnum, NULL);
int virt_lines = decor_virt_lines(wp, lnum - 1, lnum, NULL, true);
// be quick when there are no filler lines
if (diffopt_filler()) {
@ -906,6 +906,25 @@ int plines_m_win(win_T *wp, linenr_T first, linenr_T last, int max)
return MIN(max, count);
}
/// Return number of window lines a physical line range will occupy.
/// Only considers real and filler lines.
///
/// Mainly used for calculating scrolling offsets.
int plines_m_win_fill(win_T *wp, linenr_T first, linenr_T last)
{
int count = last - first + 1 + decor_virt_lines(wp, first - 1, last, NULL, false);
if (diffopt_filler()) {
for (int lnum = first; lnum <= last; lnum++) {
// Note: this also considers folds.
int n = diff_check(wp, lnum);
count += MAX(n, 0);
}
}
return MAX(count, 0);
}
/// Get the number of screen lines a range of text will take in window "wp".
///
/// @param[in] start_lnum Starting line number, 1-based inclusive.

11
test/functional/testnvim.lua
View File

@ -835,9 +835,18 @@ function M.exec_capture(code)
return M.api.nvim_exec2(code, { output = true }).output
end
--- @param code string
--- @param code string|function
--- @return any
function M.exec_lua(code, ...)
if type(code) == 'function' then
return M.api.nvim_exec_lua(
[[
local code = ...
return loadstring(code)(select(2, ...))
]],
{ string.dump(code), ... }
)
end
return M.api.nvim_exec_lua(code, { ... })
end

442
test/functional/ui/scrollbind_spec.lua Normal file
View File

@ -0,0 +1,442 @@
local t = require('test.testutil')
local n = require('test.functional.testnvim')()
local clear = n.clear
local Screen = require('test.functional.ui.screen')
before_each(clear)
describe('Scrollbind', function()
local screen --- @type test.functional.ui.screen
before_each(function()
screen = Screen.new(40, 12)
screen:attach()
end)
it('works with one buffer with virtual lines', function()
n.exec_lua(function()
local lines = {} --- @type string[]
for i = 1, 20 do
lines[i] = tostring(i * 2 - 1)
end
local ns = vim.api.nvim_create_namespace('test')
vim.api.nvim_buf_set_lines(0, 0, -1, false, lines)
vim.bo.buftype = 'nofile'
for i in ipairs(lines) do
vim.api.nvim_buf_set_extmark(0, ns, i - 1, 0, {
virt_lines = { { { tostring(2 * i) .. ' v' } } },
})
end
vim.wo.scrollbind = true
vim.cmd.vsplit()
vim.wo.scrollbind = true
end)
n.feed('<C-d>')
t.eq(5, n.api.nvim_get_option_value('scroll', {}))
screen:expect({
grid = [[
6 v │6 v |
7 │7 |
8 v │8 v |
9 │9 |
10 v │10 v |
^11 │11 |
12 v │12 v |
13 │13 |
14 v │14 v |
15 │15 |
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('<C-u>')
local line1_grid = [[
^1 │1 |
2 v │2 v |
3 │3 |
4 v │4 v |
5 │5 |
6 v │6 v |
7 │7 |
8 v │8 v |
9 │9 |
10 v │10 v |
{3:[Scratch] }{2:[Scratch] }|
|
]]
screen:expect({ grid = line1_grid })
n.api.nvim_set_option_value('scroll', 6, {})
n.feed('<C-d>')
screen:expect({
grid = [[
7 │7 |
8 v │8 v |
9 │9 |
10 v │10 v |
11 │11 |
12 v │12 v |
^13 │13 |
14 v │14 v |
15 │15 |
16 v │16 v |
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('<C-u>')
screen:expect({ grid = line1_grid })
end)
it('works with two buffers with virtual lines on one side', function()
n.exec_lua(function()
local lines = {} --- @type string[]
for i = 1, 20 do
lines[i] = tostring(i)
end
local ns = vim.api.nvim_create_namespace('test')
vim.api.nvim_buf_set_lines(0, 0, -1, false, lines)
vim.bo.buftype = 'nofile'
vim.wo.scrollbind = true
vim.cmd.vnew()
lines = {} --- @type string[]
for i = 1, 20 do
lines[i] = tostring(i + (i > 3 and 4 or 0))
end
vim.api.nvim_buf_set_lines(0, 0, -1, false, lines)
vim.bo.buftype = 'nofile'
vim.api.nvim_buf_set_extmark(0, ns, 2, 0, {
virt_lines = {
{ { '4 v' } },
{ { '5 v' } },
{ { '6 v' } },
{ { '7 v' } },
},
})
vim.wo.scrollbind = true
end)
n.feed('<C-d>')
t.eq(5, n.api.nvim_get_option_value('scroll', {}))
screen:expect({
grid = [[
6 v │6 |
7 v │7 |
8 │8 |
9 │9 |
^10 │10 |
11 │11 |
12 │12 |
13 │13 |
14 │14 |
15 │15 |
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('<C-u>')
local line1_grid = [[
^1 │1 |
2 │2 |
3 │3 |
4 v │4 |
5 v │5 |
6 v │6 |
7 v │7 |
8 │8 |
9 │9 |
10 │10 |
{3:[Scratch] }{2:[Scratch] }|
|
]]
screen:expect({ grid = line1_grid })
n.api.nvim_set_option_value('scroll', 6, {})
n.feed('<C-d>')
screen:expect({
grid = [[
7 v │7 |
8 │8 |
9 │9 |
10 │10 |
^11 │11 |
12 │12 |
13 │13 |
14 │14 |
15 │15 |
16 │16 |
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('<C-u>')
screen:expect({ grid = line1_grid })
-- Note: not the same as n.feed('4<C-e>')
n.feed('<C-e>')
n.feed('<C-e>')
n.feed('<C-e>')
n.feed('<C-e>')
screen:expect({
grid = [[
5 v │5 |
6 v │6 |
7 v │7 |
^8 │8 |
9 │9 |
10 │10 |
11 │11 |
12 │12 |
13 │13 |
14 │14 |
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('<C-e>')
screen:expect({
grid = [[
6 v │6 |
7 v │7 |
^8 │8 |
9 │9 |
10 │10 |
11 │11 |
12 │12 |
13 │13 |
14 │14 |
15 │15 |
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('<C-y>')
n.feed('<C-y>')
screen:expect({
grid = [[
4 v │4 |
5 v │5 |
6 v │6 |
7 v │7 |
^8 │8 |
9 │9 |
10 │10 |
11 │11 |
12 │12 |
13 │13 |
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
end)
it('works with buffers of different lengths', function()
n.exec_lua(function()
vim.api.nvim_buf_set_lines(0, 0, -1, false, { '1', '2', '3' })
vim.bo.buftype = 'nofile'
vim.wo.scrollbind = true
vim.cmd.vnew()
local lines = {} --- @type string[]
for i = 1, 50 do
lines[i] = tostring(i)
end
vim.api.nvim_buf_set_lines(0, 0, -1, false, lines)
vim.bo.buftype = 'nofile'
vim.wo.scrollbind = true
end)
n.feed('10<C-e>')
screen:expect({
grid = [[
^11 │3 |
12 │{1:~ }|
13 │{1:~ }|
14 │{1:~ }|
15 │{1:~ }|
16 │{1:~ }|
17 │{1:~ }|
18 │{1:~ }|
19 │{1:~ }|
20 │{1:~ }|
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('<C-y>')
screen:expect({
grid = [[
10 │3 |
^11 │{1:~ }|
12 │{1:~ }|
13 │{1:~ }|
14 │{1:~ }|
15 │{1:~ }|
16 │{1:~ }|
17 │{1:~ }|
18 │{1:~ }|
19 │{1:~ }|
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
end)
it('works with buffers of different lengths and virtual lines', function()
n.exec_lua(function()
vim.api.nvim_buf_set_lines(0, 0, -1, false, { '1', '5', '6' })
local ns = vim.api.nvim_create_namespace('test')
vim.api.nvim_buf_set_extmark(0, ns, 0, 0, {
virt_lines = {
{ { '2 v' } },
{ { '3 v' } },
{ { '4 v' } },
},
})
vim.bo.buftype = 'nofile'
vim.wo.scrollbind = true
vim.cmd.vnew()
local lines = {} --- @type string[]
for i = 1, 50 do
lines[i] = tostring(i)
end
vim.api.nvim_buf_set_lines(0, 0, -1, false, lines)
vim.bo.buftype = 'nofile'
vim.wo.scrollbind = true
end)
n.feed('<C-e>')
n.feed('<C-e>')
screen:expect({
grid = [[
^3 │3 v |
4 │4 v |
5 │5 |
6 │6 |
7 │{1:~ }|
8 │{1:~ }|
9 │{1:~ }|
10 │{1:~ }|
11 │{1:~ }|
12 │{1:~ }|
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('8<C-e>')
screen:expect({
grid = [[
^11 │6 |
12 │{1:~ }|
13 │{1:~ }|
14 │{1:~ }|
15 │{1:~ }|
16 │{1:~ }|
17 │{1:~ }|
18 │{1:~ }|
19 │{1:~ }|
20 │{1:~ }|
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('<C-y>')
n.feed('<C-y>')
n.feed('<C-y>')
n.feed('<C-y>')
n.feed('<C-y>')
t.eq(n.exec_lua [[return vim.fn.line('w0', 1001)]], 6)
t.eq(n.exec_lua [[return vim.fn.line('w0', 1000)]], 3)
screen:expect({
grid = [[
6 │6 |
7 │{1:~ }|
8 │{1:~ }|
9 │{1:~ }|
10 │{1:~ }|
^11 │{1:~ }|
12 │{1:~ }|
13 │{1:~ }|
14 │{1:~ }|
15 │{1:~ }|
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
n.feed('<C-y>')
n.feed('<C-y>')
n.feed('<C-y>')
screen:expect({
grid = [[
3 │3 v |
4 │4 v |
5 │5 |
6 │6 |
7 │{1:~ }|
8 │{1:~ }|
9 │{1:~ }|
10 │{1:~ }|
^11 │{1:~ }|
12 │{1:~ }|
{3:[Scratch] }{2:[Scratch] }|
|
]],
})
end)
end)