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

Introduce ScrollGroup #583

Merged
merged 17 commits into from
Jan 6, 2023
Merged

Conversation

citizenmatt
Copy link
Member

@citizenmatt citizenmatt commented Jan 5, 2023

This PR extracts the scrolling functionality from MotionGroup and VimMotionGroup and introduces the VimScrollGroup interface and the ScrollGroup service. The methods on VimMotionGroup were checked for external usages before moving. The code has also been converted into Kotlin.

Furthermore, the scrollCaretIntoView functions in the ScrollViewHelper.kt file are now licensed under the Vim license. These functions control how the window is scrolled to place the caret at least 'scrolloff' lines away from the top/bottom of the window, or place the caret to the middle of the window. The current implementation is based on reading Vim source, but is a different implementation that does not support soft wrapped lines. Scrolling is a complex area in Vim, and to accurately mimic Vim's behaviour with soft wraps, this implementation will need to be rewritten to match Vim's implementation.

PS. I migrated MotionGroup to Kotlin too, because who doesn't like a bit of Kotlin?

@@ -145,10 +145,10 @@ class IjVimCaret(val caret: Caret) : VimCaretBase() {
override fun hashCode(): Int = this.caret.hashCode()
}

val VimCaret.ij: Caret
inline val VimCaret.ij: Caret
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, any advantages from making this field inlined?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly not :) I didn't measure anything, but it felt like an overhead to call a function that simply does a cast and property access, especially when we've got that same pattern repeated in the code. It just means there's no difference to putting the pattern in the calling code or using this helper function, which is a lot nicer.

I've always wondered if it's worth putting IjVimEditor into Editors user data, to prevent allocating too frequently, too.

@AlexPl292
Copy link
Member

Awesome as always. Thank you!

@AlexPl292 AlexPl292 merged commit 3be6acf into JetBrains:master Jan 6, 2023
@citizenmatt citizenmatt deleted the refactor/scroll-group branch January 6, 2023 08: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
Development

Successfully merging this pull request may close these issues.

2 participants