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

VIM-1558 Support block inlays in scrolling and motion #173

Merged
merged 3 commits into from
Jan 28, 2019

Conversation

citizenmatt
Copy link
Member

This PR changes the way scrolling and motion work to take intra-line block inlays, such as Rider's Code Vision. It changes the implementation from being fundamentally line based to being more based on scroll offsets, since the inlays mean that the height of the screen is not constant. When inlays are not displayed, it behaves as though it were still line based.

When inlays are displayed, there are some changes in behaviour:

  • Scrolling and motion commands mostly try to keep the top line aligned with the top of the screen. When the top line has inlays attached above it, the line is moved so that the inlay is visible.
    • When scrolling down, the bottom line's trailing inlays are always visible, and the top line (with leading inlays) is also kept aligned.
    • M does not follow this behaviour and simply places the current line in the middle of the screen
    • CTRL-D and CTRL-U also doesn't follow this rule, instead trying to keep the caret in the same relative screen position (scroll offset, not line number).
  • Repeated use of CTRL-D or CTRL-U scrolls half a screen height, which due to inlays isn't a consistent line number. Using [count] CTRL-D will scroll a consistent number of lines.
    • The current caret location is maintained as a scroll offset, but is moved if the new caret line has leading or trailing block inlays.
    • If [count] CTRL-D is going to be over a screen of scrolling, the scroll distance is capped at a screen height. Inlays are taken into consideration, so the top line is always the bottom line of the previous page + 1.

This PR is based on #170, and includes those changes.

Fixes VIM-1558.

Commands tested

The following commands have been manually tested, with and without inlays and with/without scrolloff, scrolljump and scroll set:

  • Scroll window up/down - CTRL-Y/CTRL-E+ [count] CTRL-Y/[count] CTRL-E
  • Scroll backwards/forwards - CTRL-B/CTRL-F + [count] CTRL-B/[count] CTRL-F
  • Scroll half page down/up - CTRL-D/CTRL-U + [count] CTRL-D/[count] CTRL-U
  • Scroll line to top of window z<CR>/zt + [count] z<CR>/[count] zt
  • Scroll line to centre of window zz/z. + [count] zz/[count] z.
  • Scroll line to bottom of window - z-/zb + [count] z-/[count] zb
  • Move caret to first/middle/last screen line - H/M/L + [count] H/[count] M/[count] L
  • Scroll to next/prev page first/last line - z+/z^
  • Scroll caret into view:
    • Mouse click
    • Caret moves (e.g. off end of page)
    • Search

How to test

  • gradle runIde
  • Invoke the "Vim (internal) add test inlays". This is an internal only action and doesn't have a menu - use Find Action. It will add a random assortment of block inlays of different sizes, some above and some below the line. It can be called multiple times to add more inlays. To clear the inlays, simply close and reopen the file.

Alternatively, to test in Rider:

  • Edit gradle.properties and change the first line to:
    ideaVersion RD-2018.3
    
  • gradle runIde
  • Open any C# project

Known issues

If the scrolloff option is set large enough, and there are block inlays, navigation becomes unpredictable. It works correctly if there are no block inlays. So the technique to set a large scroll offset to force the caret to remain in the centre of the screen is currently incompatible with block inlays.

This is because the check to keep the scroll offset to half the screen height is still based on line height and does not include inlay heights, which are different based on the expected location within the file. I'll look at this for a separate PR.

@vlasovskikh vlasovskikh self-assigned this Jan 25, 2019
@MarkusAmshove
Copy link

I've tested this PR with Rider and can confirm that it resolves the issues that I had (unpredictable movement, command like ciw acting on a different line).

The only scrolloptions I use apart from the default configuration is scrolloff=5

@vlasovskikh vlasovskikh merged commit 531a9c2 into JetBrains:master Jan 28, 2019
@vlasovskikh
Copy link
Contributor

@citizenmatt Thanks for your PR! I've reviewed your code and tested the plugin using your internal action for inserting random block inlays. I've made a few modifications after the review: I haven't found any major problems, the commits are there in master in case you're interested. It was mostly about compatibility and code style. Thanks again for your fix! I'll release it as a EAP build for beta testing and, after some time, as an official build.

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.

3 participants