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 b on long lines #775

Merged
merged 3 commits into from Jun 30, 2022
Merged

Conversation

zbw8388
Copy link
Collaborator

@zbw8388 zbw8388 commented Nov 21, 2021

Solves #773 and partially addresses #573.

Somehow a sequence of cursorMove commands is causing a bug in vscode. Removing them and directly setting the cursor location solves this issue. We don't need cursorMove vscode commands in this case anyways as less than one line move does not seem to get registered in jumplist.

Since the extension is getting highlights from redraw notifications, which is limited by window size, and neovim did not make the starting column of the current viewport in buffer available in the drawing notifications, the visual mode still displays incorrectly in vscode. However, it seems like the actual editing itself is working properly. There is no good fix to this other than implementing the display part of visual mode in the extension.

@justinmk
Copy link
Collaborator

The deleted code sure looks attractive :)

Possible to write some sort of test? At least to exercise the codepath.

Rebasing will fix CI.

@theol0403
Copy link
Member

I've been using this for a while and it seems to work well. In addition, I think the codepath is already excersized by tests because it is a core part of the plugin. I think it also fixes a bug where lots of j/k up and down on the end of the line won't return to the same col after a bit.

Copy link
Member

@theol0403 theol0403 left a comment

Choose a reason for hiding this comment

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

There might be a slight performance decrease when holding j/k, but I'm having a lot of trouble being sure. I think it's acceptable.

@theol0403
Copy link
Member

Additionally, I think this fixes a problem where some jumps don't get registered in jumplist, because they come from a sequence of cursormove commands.

@theol0403 theol0403 merged commit f783274 into vscode-neovim:master Jun 30, 2022
@zbw8388 zbw8388 deleted the fix-b-on-long-line branch June 30, 2022 16:54
Jendker added a commit to Jendker/vscode-neovim that referenced this pull request Feb 19, 2023
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.

None yet

3 participants