Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Fix excessive grid_scrolls on ext_messages+small window #13909

Closed
wants to merge 1 commit into from

Conversation

glacambre
Copy link
Member

No description provided.

This commit fixes excessive redraws that would happen when ext_messages
is used with a scrolloffset larger than the window.

The reason we need to use `plines <= so * 2` is that the window needs to
accomodate both the scroll offset (so * 2) and the line the cursor is
on, so there isn't enough room even if plines is equal to (so * 2).

Closes neovim#12810.
@glacambre glacambre changed the title Fix excessive grid_scrolls on ext_messages+small window [RDY] Fix excessive grid_scrolls on ext_messages+small window Feb 10, 2021
@glacambre glacambre changed the title [RDY] Fix excessive grid_scrolls on ext_messages+small window [RFC] Fix excessive grid_scrolls on ext_messages+small window Feb 10, 2021
@@ -892,7 +892,7 @@ void curs_columns(
extra += 2;
}

if (extra == 3 || plines < so * 2) {
if (extra == 3 || plines <= so * 2) {

This comment was marked as resolved.

This comment was marked as resolved.

@glacambre
Copy link
Member Author

I strongly believe that new behavior is more correct than the previous one and thus that the oldtest need to be updated, could somebody else confirm?

os.remove('Xtest')
end)

it('ext_messages doesnt trigger unnecessary grid_scrolls', function()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really connected to ext_messages at all? don't we see the same change of behaviour without ext_messages but with three grid lines (so still two for the window) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had tried to reproduce it with the TUI and failed, but I just tried again and succeeded, so there's indeed nothing ext_messages-specific here.

In fact, Vim seems to have this bug too, so would you prefer that I fix it upstream and then port the patch to Neovim?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make sense to report the issue upstream yes, but if the fix is the same you can just add the vim-patch tag to the commit message.

@glacambre
Copy link
Member Author

My Vim patch was included in #13908 , so this PR can be closed. Thanks a lot for the reviews!

@glacambre glacambre closed this Feb 11, 2021
@glacambre glacambre deleted the excessive_scrolling branch February 11, 2021 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants