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

Fix various scrolling issues #255

Merged
merged 4 commits into from Nov 12, 2020
Merged

Conversation

citizenmatt
Copy link
Member

  • Fix VIM-2158: scrolling jumps around when scrolloff is greater than half the screen height, but less than the total screen height
  • Reposition the cursor when scrolloff changes. E.g. :set so=999 will reposition the cursor in the middle of the screen immediately, rather than waiting for the next motion
  • Limit the height of inlays shown above/below a line when scrolling up/down. Normally, IdeaVim will try to show the associated block inlay (e.g. Rider's Code Vision links for usages, source control, etc.) above a line, when moving that line to the top of the screen, and vice versa - inlays attached below a line when moving that line to the bottom. However, multiline rendered doc comments are treated as block inlays rather than lines, and can take up a lot of space. Trying to show all of the inlay can cause the visible area to bounce around and can even force the cursor off screen. IdeaVim will now limit the block inlay height to 3 standard line heights. If the inlay is taller than this, it will be cut off.
  • Workaround an issue with lots of block inlays/doc comments causing the actual number of text lines to be very small. This forces the scrolling algorithm to treat a small movement as a large scroll and so position the cursor in the middle of the screen. This can cause scrolling to "stick", with the cursor unable to scroll to the proper location. A fudge factor has been added to try to catch when there are significantly less text lines than expected, and avoid the scroll to centre loop.
  • Tests

Top tip: if you're looking for a good example file to test rendered doc comments, then String.java is a good place to go.

@AlexPl292
Copy link
Member

Hey, I've just tried to use this PR with String and I'm afraid that the scroll is broken a bit 😞. I don't know, are these issues are the part of this PR, or they are completely independent?

2020-11-11 11 12 08

@AlexPl292
Copy link
Member

Matt, I really appreciate your help. I just see how difficult it is and I'm happy that someone can handle it properly.

Offtopic

Matt Ellis trying to fix all scroll issues

image

@citizenmatt
Copy link
Member Author

Ah. This issue isn't due to the PR - you'll see the same behaviour in a stable version.

The problem is that so=5 will try to keep five lines of context at the bottom of the screen when moving down with j. But the 5th line has a ridiculously large doc comment above it. Scrolling to show the actual 5th line includes this massive inlay, and that pushes the cursor off the top of the screen. And then subsequent j movements are trying to catch up.

This PR does have a mitigation for a similar problem - if we were moving up (k) instead, or moving down and the 5th line had an inlay attached to the bottom of the line, then IdeaVim makes sure only a max of 3 * line height of the doc comment is shown (try moving the class String declaration off the top of the screen and moving up with k). But this fix doesn't help in this situation, and it also doesn't help when the line with the massive inlay is line 3 in a 5 line offset.

This needs a separate PR to fix, although I don't know what the fix is yet. First guess is to normalise the scroll offset height to be expected line height of offset + total acceptable inlay height, limiting it so that the offset will only include say 3 line heights of inlay. Of course, it's all complicated by the fact that we scroll by positioning a line at the top/bottom of the screen, rather than specifying a pixel offset...

@AlexPl292
Copy link
Member

Okay, I see. Looks like IdeaVim gets troubles every time IJ devs decide to draw something else than text in the editor :)

@AlexPl292 AlexPl292 merged commit c38b18e into JetBrains:master Nov 12, 2020
@citizenmatt citizenmatt deleted the VIM-2158 branch November 12, 2020 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants