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
feat(viewport): sync neovim scroll with vscode #1335
base: master
Are you sure you want to change the base?
Conversation
fix scrolling commands by removing custom code and reverting to default nvim behavior
@theol0403 Hi, what other works need to do for this PR? The main issues I encounter in my daily use are related to viewport for now. |
@xiyaowong I agree this is one of the biggest areas of improvement (#1261). Unfortunately this PR is very much pre-alpha, I essentially copy/pasted the code from #993 and solved some merge conflicts. However, the way I evolved the cursor manager in 0.4.0 took a different sync approach than this PR uses so I will have to refactor this PR to be compatible with that. Once the PR is properly written, there will undoubtedly be a large bug fixing session. As I am about to return to an extremely busy year of class, I don't know when I will have time to work on this - you are welcome to take a stab if you want. 1st priority for me atm, other than starting the school year (am I also president of a robotics design team), is to create a CONTRIBUTING.md file with all the information I've learned, which will make it much easier to find new contributors. |
@theol0403 Today I looked into the synchronization issue with the viewport. A simple try: https://github.com/xiyaowong/vscode-neovim/tree/fix/viewport The main problem is that scrolling in VSCode is not based on lines, so the obtained visible range is not accurate and usually smaller than the actual range. This requires manually adding compensation when synchronizing the vim viewport, hence the vim viewport is generally larger. In this case, when using native vim commands for jumping or scrolling, the following situation occurs: vim cursor position changes -> sync VSCode cursor position -> VSCode viewport changes -> sync vim viewport -> vim cursor position changes This leads to jitter. |
@xiyaowong the solution must be to let neovim handle all scrolling and denounce vscode scrolling |
@theol0403 I’m not sure what you mean by "let nvim handle all scrolling." I think what you're referring to is bidirectional synchronization, which would be the ideal situation. We are using VSCode, and it should always be the primary role with nvim as a secondary one. Besides, how's it possible to make VSCode not handle scrolling? |
I mean that nvim is given priority for all events that change scroll position. When moving cursor, using scrolling commands, etc, nvim should control the vscode scroll (keep in mind that moving cursor does not create scroll event in vscode). However, in the case that the scroll is generated from vscode (mouse wheel, go to definition), it should be denounced for when there are no nvim updates. Basically, nvim should be primary role, just like cursor sync. |
@xiyaowong I see you're starting to work on viewport sync. Start with |
Hmm. I have finished reading the relevant pr codes and related discussions. I might do it my own way first. The main synchronization logic should not be complicated, but many edge scenes should be considered. |
Made some updates. In short, there are too many situations that cause cursor jitter. Maybe now is not the time for an update, and we can wait for vscode to provide related APIs before optimizing: microsoft/vscode#195406. |
@xiyaowong I quickly tested your PR, and it does seem to work pretty well... is the jitter really that bad/unavoidable? What is the fundamental issue? I don't think the linked vscode issue will solve, that's for the vscode window dimensions, not the editor (and it can be years to wait for vscode fixes). Either there is never a solution, or we have to solve the current limitations. |
Conceptually, I think no jitter is possible, if the cursor and viewport changes from nvim get applied atomically, and if the vscode scroll events get debounced before sending to nvim. |
We need to known the editor height, It is not accurate to adjust the viewport by adjusting the window height. Some other problems, but mainly the above one.
|
Eons after - will this ever work |
This continues the work of #993 and #885.
Will fix #1261.
Currently in a very rough draft. A lot of work is needed, including in reconsidering how change locks are handled. My visual selection PR went in a different direction than this, so I have to figure out what's best.
There are also a lot of edge cases to solve.