Closed
Conversation
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.
Contributor
Author
|
For the first point (inconsistency bugs), see the following asciicast which demos how to trigger them. |
Contributor
Author
|
Also, I think Ideally, when we have |
Codecov Report
@@ Coverage Diff @@
## master #11764 +/- ##
=======================================
Coverage 81.90% 81.91%
=======================================
Files 164 164
Lines 191923 191986 +63
Branches 43556 43569 +13
=======================================
+ Hits 157201 157269 +68
+ Misses 21974 21962 -12
- Partials 12748 12755 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Contributor
|
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.
Thanks. The 'smoothscroll' implementation is tricky, there is quite a
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 ///
|
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 28, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 28, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 28, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 28, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
zeertzjq
pushed a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 28, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 28, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 29, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 29, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 29, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 29, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
Apr 30, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
May 1, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
May 2, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
luukvbaal
added a commit
to luukvbaal/neovim
that referenced
this pull request
May 2, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
clason
pushed a commit
to clason/neovim
that referenced
this pull request
May 10, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
tom-anders
pushed a commit
to tom-anders/neovim
that referenced
this pull request
May 21, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
folke
pushed a commit
to folke/neovim
that referenced
this pull request
May 22, 2023
…othscroll'
Problem: Cursor positioning and display problems with 'smoothscroll' and
using "zt", "zb" or "zz".
Solution: Adjust computations and conditions. (Yee Cheng Chin,
closes vim/vim#11764)
vim/vim@db4d88c
Co-authored-by: Bram Moolenaar <Bram@vim.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
zzwhenever 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()orredraw_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.