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(VIM-2432): handle scrolloff with <C-D> and <C-U> #391

Merged
merged 1 commit into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/main/java/com/maddyhome/idea/vim/group/MotionGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ else if (caretColumn > rightVisualColumn - sideScrollOffset) {
return SearchHelper.findParagraphRange(editor, caret, count, isOuter);
}

// Get the visual line that will be in the same screen relative location as the current caret line, after the screen
// has been scrolled
private static int getScrollScreenTargetCaretVisualLine(final @NotNull Editor editor, int rawCount, boolean down) {
final Rectangle visibleArea = getVisibleArea(editor);
final int caretVisualLine = editor.getCaretModel().getVisualPosition().line;
Expand All @@ -271,7 +273,7 @@ private static int getScrollScreenTargetCaretVisualLine(final @NotNull Editor ed
targetCaretVisualLine = down ? caretVisualLine + scrollOption : caretVisualLine - scrollOption;
}

return targetCaretVisualLine;
return EditorHelper.normalizeVisualLine(editor, targetCaretVisualLine);
}

public @Range(from = 0, to = Integer.MAX_VALUE) int moveCaretToNthCharacter(@NotNull Editor editor, int count) {
Expand Down Expand Up @@ -1167,6 +1169,8 @@ public boolean scrollScreen(final @NotNull Editor editor, final @NotNull Caret c

final Rectangle visibleArea = getVisibleArea(editor);

// We want to scroll the screen and keep the caret in the same screen-relative position. Calculate which line will
// be at the current caret line and work the offsets out from that
int targetCaretVisualLine = getScrollScreenTargetCaretVisualLine(editor, rawCount, down);

// Scroll at most one screen height
Expand All @@ -1190,12 +1194,11 @@ public boolean scrollScreen(final @NotNull Editor editor, final @NotNull Caret c
}
}
else {

scrollVisualLineToCaretLocation(editor, targetCaretVisualLine);

final int scrollOffset = getNormalizedScrollOffset(editor);
final int visualTop = getVisualLineAtTopOfScreen(editor) + scrollOffset;
final int visualBottom = getVisualLineAtBottomOfScreen(editor) - scrollOffset;
final int visualTop = getVisualLineAtTopOfScreen(editor) + (down ? scrollOffset : 0);
final int visualBottom = getVisualLineAtBottomOfScreen(editor) - (down ? 0 : scrollOffset);

targetCaretVisualLine = max(visualTop, min(visualBottom, targetCaretVisualLine));
}
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/com/maddyhome/idea/vim/helper/EditorHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,8 @@ else if (!Character.isWhitespace(chars.charAt(offset))) {
}

/**
* Scrolls the editor to put the given visual line at the current caret location, relative to the screen.
* Scrolls the editor to put the given visual line at the current caret location, relative to the screen, as long as
* this doesn't add virtual space to the bottom of the file.
* <p>
* Due to block inlays, the caret location is maintained as a scroll offset, rather than the number of lines from the
* top of the screen. This means the line offset can change if the number of inlays above the caret changes during
Expand Down Expand Up @@ -668,7 +669,11 @@ else if (bottomInlayHeight > visibleArea.height - caretScreenOffset + editor.get
inlayOffset = -bottomInlayHeight;
}

scrollVertically(editor, yVisualLine - caretScreenOffset - inlayOffset);
// Scroll the given visual line to the caret location, but do not scroll down passed the end of file, or the current
// virtual space at the bottom of the screen
final int lastVisualLine = EditorHelper.getVisualLineCount(editor) - 1;
final int yBottomLineOffset = max(getOffsetToScrollVisualLineToBottomOfScreen(editor, lastVisualLine), visibleArea.y);
scrollVertically(editor, min(yVisualLine - caretScreenOffset - inlayOffset, yBottomLineOffset));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,28 @@ class ScrollHalfPageDownActionTest : VimTestCase() {
}

@TestWithoutNeovim(SkipNeovimReason.SCROLL)
fun`test scroll downwards in bottom half of last page moves to the last line`() {
fun`test scroll downwards in bottom half of last page moves caret to the last line without scrolling`() {
configureByPages(5)
setPositionAndScroll(146, 165)
setPositionAndScroll(140, 165)
typeText(parseKeys("<C-D>"))
assertPosition(175, 0)
assertVisibleArea(141, 175)
}

@TestWithoutNeovim(SkipNeovimReason.SCROLL)
fun`test scroll downwards in bottom half of last page moves caret to the last line with scrolloff`() {
OptionsManager.scrolloff.set(10)
configureByPages(5)
setPositionAndScroll(140, 164)
typeText(parseKeys("<C-D>"))
assertPosition(175, 0)
assertVisibleArea(141, 175)
}

@TestWithoutNeovim(SkipNeovimReason.SCROLL)
fun`test scroll downwards at end of file with existing virtual space moves caret without scrolling window`() {
configureByPages(5)
setPositionAndScroll(146, 165) // 146 at top line means bottom line is 181 (out of 175)
typeText(parseKeys("<C-D>"))
assertPosition(175, 0)
assertVisibleArea(146, 175)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ class ScrollHalfPageUpActionTest : VimTestCase() {
assertVisibleArea(0, 34)
}

@TestWithoutNeovim(SkipNeovimReason.SCROLL)
fun`test scroll upwards in first half of first page moves to first line with scrolloff`() {
OptionsManager.scrolloff.set(10)
configureByPages(5)
setPositionAndScroll(5, 15)
typeText(parseKeys("<C-U>"))
assertPosition(0, 0)
assertVisibleArea(0, 34)
}

@TestWithoutNeovim(SkipNeovimReason.SCROLL)
fun`test scroll count lines upwards`() {
configureByPages(5)
Expand Down