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

Respect default behaviour of cursorSurroundingLines #4481

Merged
merged 1 commit into from
Jan 9, 2020

Conversation

cvaldev
Copy link
Contributor

@cvaldev cvaldev commented Jan 8, 2020

What this PR does / why we need it:

It addresses the issue of revealing a range on a mouse click which causes us to not respect the default behaviour of cursorSurroundingLines. The default behaviour of cursorSurroundingLines ("editor.cursorSurroundingLinesStyle": "default") is to ignore the scroll offset (https://github.com/microsoft/vscode/blob/master/src/vs/editor/browser/viewParts/lines/viewLines.ts#L574), thus not scrolling on a click event.

VSCode handles revealing a range on a click, I believe it is addressed with https://github.com/microsoft/vscode/blob/71b60d0d2201e5ba0a52d9bd2b914da2cee11ab5/src/vs/editor/browser/viewParts/lines/viewLines.ts#L649, so there is no need for us to also attempt to reveal a range on a mouse selection event. In fact, this creates noticeable issues now that scroll offset is supported by vscode.

Which issue(s) this PR fixes
Fixes #4465

Special notes for your reviewer:

From my own manual testing, this appears to not break anything.

I'm unsure if this needed to be true in the past, or if it was an oversight as it didn't cause any obvious issues up until scroll off was added to vscode. If you'd prefer, I could add a check for cursorSurroundingLinesStyle === "default" instead, but I honestly think revealing a range on a mouse selection event is simply not necessary anymore as vanilla vscode takes care of it.

Copy link
Member

@J-Fields J-Fields left a comment

Choose a reason for hiding this comment

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

Sweet - thanks for tracking this one down!

@J-Fields J-Fields merged commit 2a5dc45 into VSCodeVim:master Jan 9, 2020
@xconverge
Copy link
Member

Note this is a terrifying part of the codebase, there is a set of test steps in a gist in the comment for the function at the top that should be double checked

@J-Fields
Copy link
Member

J-Fields commented Jan 9, 2020

Yep - that all still works @xconverge

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.

Mouse scrolls editor when Cursor Surrounding Lines is set
3 participants