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

Fixes gj/gk behaviour on wrapped lines; closes #4120 #4127

Merged
merged 4 commits into from
Oct 5, 2019

Conversation

hetmankp
Copy link
Contributor

@hetmankp hetmankp commented Oct 3, 2019

Closes #4120. This more or less reverts the functionality introduced by commit
4b2e079 and raised in issue #3890.

Unfortunately it is not currently possible to fully implement display
line movements over wrapped lines that will always preserve cursor
position. One of two things is necessary to change this, either the
VSCode API needs to be extended to return information about wrapped
lines (but this was rejected in issue microsoft/vscode#23045).
Alternatively VSCodeVim would need to reïimplement all motions using
vscode.commands.executeCommand() commands rather than manipulating
vscode.window.activeTextEditor.selections directly, as the former
preserves column position while the latter does not. However it's not
clear to me if all VIM beahviour could be reïmplemented this way without
first trying this out and sounds like a bit of a ground up rewrite.

It seems for now we're stuck with gj/gk not preserving column position
when jumping across short lines.

This more or less reverts the functionality introduced by commit
4b2e079 and raised in issue VSCodeVim#3890.

Unfortunately it is not currently possible to fully implement display
line movements over wrapped lines that will always preserve cursor
position. One of two things is necessary to change this, either the
VSCode API needs to be extended to return information about wrapped
lines (but this was rejected in issue microsoft/vscode#23045).
Alternatively VSCodeVim would need to reïimplement all motions using
vscode.commands.executeCommand() commands rather than manipulating
vscode.window.activeTextEditor.selections directly, as the former
preserves column position while the latter does not. However it's not
clear to me if all VIM beahviour could be reïmplemented this way without
first trying this out and sounds like a bit of a ground up rewrite.

It seems for now we're stuck with gj/gk not preserving column position
when jumping across short lines.
@J-Fields
Copy link
Member

J-Fields commented Oct 4, 2019

Alternatively VSCodeVim would need to reïimplement all motions using
vscode.commands.executeCommand() commands rather than manipulating
vscode.window.activeTextEditor.selections directly

Why's this? Aren't basically all motions other than gj agnostic to whether lines are wrapped?

@hetmankp
Copy link
Contributor Author

hetmankp commented Oct 4, 2019

Alternatively VSCodeVim would need to reïimplement all motions using
vscode.commands.executeCommand() commands rather than manipulating
vscode.window.activeTextEditor.selections directly

Why's this? Aren't basically all motions other than gj agnostic to whether lines are wrapped?

@J-Fields, this is specific to maintaining the column position. What I gathered from microsoft/vscode#23045 is that VSCode already maintains column position for us. This works OK as long as we manipulate the cursor via vscode.commands.executeCommand() but the desired column is reset every time you modify the vscode.window.activeTextEditor.selection & .selections attributes directly.

Unfortunately, the only code with an awareness of the display column position when wrapping is turned on, is the internals of VSCode and it seems the developers are uninterested in exposing this. So the only option would be to implement all motions using executeCommand(), to prevent the desired column being reset.

Without trying this I'm not sure if there would be any corner cases where VIM behaviour could not be expressed using existing VSCode commands, however the developers did indicate that they would be open to adding additional commands as needed on a case by case basis.

These tests had been previously removed as supporting this behaviour is
not yet possible due to technical reasons, however they do represent
correct VIM behaviour so we keep them until implementation becomes
architecturally possible.
@J-Fields
Copy link
Member

J-Fields commented Oct 5, 2019

Got it, thanks! Shame there isn't an API for this.

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.

Cursor movement in wrapped lines changed from 1.10.2 to 1.11.0
2 participants