[vim/vim] Fix smoothscroll to work with zz/zt/zb (PR #11764)
Yee Cheng Chin
This commit does two things:
First, existing zt/zb/zz implementation is buggy when 'smoothscroll' are 'wrap' are set. They can lead to inconsistency bugs with regarding to repainting and cursor states.
For zz, if the user uses zz whenever the top line is smooth-scrolled, the resulting draw will be wrong. For zt, if the line above the current line is super long (meaning it is so long that the wrapped line covers more than the entire screen), it will lead to the cursor being in a bad state. For zb, the exact conditions are harder to reproduce, but basically it triggers when the top line is smooth scrolled, and second line is more than one physical line tall.
Most of the reasons why they are broken is because reset_skipcol() or redraw_later() aren't called in the right place, so this change will make sure to call them appropriately.
Second, zb/zz currently will not utilize 'smoothscroll' in a satisfactory fashion. If 'smoothscroll'/'wrap' are set, we should be able to accurate put the current line at the bottom/middle of the screen by smooth-scrolling a long wrapped line at the top of the screen. This change will make those two commands do that.
Also, add tests to regression test these features.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/11764
Commit Summary
- 0a63ed6 Fix smoothscroll to work with zz/zt/zb
File Changes
(11 files)
- M src/macros.h (2)
- M src/move.c (156)
- M src/testdir/dumps/Test_smooth_long_10.dump (6)
- M src/testdir/dumps/Test_smooth_long_11.dump (2)
- M src/testdir/dumps/Test_smooth_long_12.dump (10)
- A src/testdir/dumps/Test_smooth_long_13.dump (6)
- A src/testdir/dumps/Test_smooth_long_14.dump (6)
- A src/testdir/dumps/Test_smooth_long_15.dump (6)
- M src/testdir/dumps/Test_smooth_long_8.dump (2)
- M src/testdir/dumps/Test_smooth_long_9.dump (2)
- M src/testdir/test_scroll_opt.vim (12)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yee Cheng Chin
For the first point (inconsistency bugs), see the following asciicast which demos how to trigger them.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yee Cheng Chin
Also, I think scrolloff is currently not working perfectly when smoothscroll is set. The logic is not taking smoothscroll into account and it also doesn't handle super long lines (lines that cannot be shown in its entirely on the screen). I decided not to tackle them for now as I don't think they lead to any serious bugs, just that it would scroll a little more than necessary.
Ideally, when we have smoothscroll, scrolloff can be more precise in always just showing that many physical lines for zt and zb (actually, for zb, we don't even need smoothscroll since we can show partial lines depending on the display setting).
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
codecov[bot]
Codecov Report
Merging #11764 (0a63ed6) into master (7b8db11) will increase coverage by
0.45%.
The diff coverage is82.19%.
@@ Coverage Diff @@ ## master #11764 +/- ## ========================================== + Coverage 81.90% 82.36% +0.45% ========================================== Files 164 154 -10 Lines 191923 181643 -10280 Branches 43556 41153 -2403 ========================================== - Hits 157201 149608 -7593 + Misses 21974 19558 -2416 + Partials 12748 12477 -271
| Flag | Coverage Δ | |
|---|---|---|
| huge-clang-none | 82.62% <82.19%> (+<0.01%) |
⬆️ |
| huge-gcc-none | ? |
|
| huge-gcc-testgui | 52.65% <70.42%> (+<0.01%) |
⬆️ |
| huge-gcc-unittests | 0.29% <0.00%> (-0.01%) |
⬇️ |
| linux | 82.36% <82.19%> (-0.06%) |
⬇️ |
| mingw-x64-HUGE | ? |
|
| mingw-x86-HUGE | ? |
|
| windows | ? |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/move.c | 90.65% <82.19%> (-0.83%) |
⬇️ |
| src/regexp_bt.c | 78.50% <0.00%> (-7.47%) |
⬇️ |
| src/json.c | 77.84% <0.00%> (-5.47%) |
⬇️ |
| src/regexp_nfa.c | 85.19% <0.00%> (-4.57%) |
⬇️ |
| src/libvterm/src/parser.c | 55.18% <0.00%> (-4.15%) |
⬇️ |
| src/edit.c | 82.84% <0.00%> (-3.48%) |
⬇️ |
| src/gui.c | 69.57% <0.00%> (-3.15%) |
⬇️ |
| src/normal.c | 87.89% <0.00%> (-2.99%) |
⬇️ |
| src/clipboard.c | 71.68% <0.00%> (-2.91%) |
⬇️ |
| src/misc2.c | 86.66% <0.00%> (-2.81%) |
⬇️ |
| ... and 125 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
> This commit does two things:
>
>
> First, existing zt/zb/zz implementation is buggy when 'smoothscroll' are
> 'wrap' are set. They can lead to inconsistency bugs with regarding to
> repainting and cursor states.
>
> For zz, if the user uses `zz` whenever the top line is smooth-scrolled,
> the resulting draw will be wrong. For zt, if the line above the current
> line is super long (meaning it is so long that the wrapped line covers
> more than the entire screen), it will lead to the cursor being in a bad
> state. For zb, the exact conditions are harder to reproduce, but
> basically it triggers when the top line is smooth scrolled, and second
> line is more than one physical line tall.
>
> Most of the reasons why they are broken is because `reset_skipcol()` or
> `redraw_later()` aren't called in the right place, so this change will
> make sure to call them appropriately.
>
>
> Second, zb/zz currently will not utilize 'smoothscroll' in a
> satisfactory fashion. If 'smoothscroll'/'wrap' are set, we should be
> able to accurate put the current line at the bottom/middle of the screen
> by smooth-scrolling a long wrapped line at the top of the screen. This
> change will make those two commands do that.
>
>
> Also, add tests to regression test these features.
bit of interference with other options and features. We'll just have to
find and fix the problems over time.
--
Violators can be fined, arrested or jailed for making ugly faces at a dog.
[real standing law in Oklahoma, United States of America]
/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.