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

Viewport scrolling issue with C-d/C-u #983

Open
Tracked by #1261
theol0403 opened this issue Jul 31, 2022 · 23 comments
Open
Tracked by #1261

Viewport scrolling issue with C-d/C-u #983

theol0403 opened this issue Jul 31, 2022 · 23 comments
Labels
bug Something isn't working category: sync synchronization between vscode and nvim (text, windows, etc) priority

Comments

@theol0403
Copy link
Member

theol0403 commented Jul 31, 2022

I noticed that @zbw8388 made a branch on their fork that made me realize a bug I missed when merging #919 and #885 - when scrolling with C-u/C-d, the veiwport is wrong.

This is because neovim internally scrolls the content while using C-u, so that the cursor position stays the same relative to the window. However, vscode moves the cursor and does not scroll the content. This causes the viewport to be unsynced.

I can think of three solutions:

  • sync neovim scroll with vscode. This could be done with win_viewport, or winsaveview. This would also remove the need for a custom implementation of C-e/C-y/zt/zb/zz. I believe @asvetliakov said this could cause jitter, but we can try.
  • modify vscode scrolling strategy. There might be a vscode api option where moving the cursor long distances will scroll the content instead of moving the cursor
  • modify neovim scrolling strategy. Make neovim scroll how vscode does, which only scrolls the content if needed

I think the first option is ideal, if it doesn't rely on too much code.

@theol0403
Copy link
Member Author

theol0403 commented Jul 31, 2022

@zbw8388 branch: https://github.com/zbw8388/vscode-neovim/tree/neovim-scroll

You might find this commit useful, it changes the scroll command and reduces jitter: theol0403@b21f7fc

I made a branch based on @zbw8388 's work where it totally relies on neovim for scrolling behaviour. This seems to work fairly well but has some jitter. This deletes a lot of code.

https://github.com/theol0403/vscode-neovim/tree/experimental-scroll

@zbw8388 please feel free to keep working on this!

@ttytm
Copy link

ttytm commented Jul 31, 2022

As vscode is generally a bit troublesome with keyboard scrolling, I wrote a plugin recently that I think works fairly well. Maybe it also improves the experience for others who use vscode-neovim until there is a final implementation and solution.
https://github.com/tobealive/german-scroll.code

And so great to see the skyrocketing development in recent days. I think slowly but steady, you make it better than the VSCodeVim extension 😊

@zbw8388
Copy link
Collaborator

zbw8388 commented Jul 31, 2022

Hi @theol0403, thank you for getting on this! I created a branch in this repo (https://github.com/vscode-neovim/vscode-neovim/tree/experimental-scroll) so it would be easier to collaborate.

I also vote for the first option. I was playing around with the idea of letting neovim handle the content scroll, but I removed that functionality due to mouse-wheel scrolling. In vscode it is different from that of neovim: vscode (like many other apps) scrolls the content, while neovim scrolls the cursor. If we decided to scroll cursor during mouse wheel scroll, then I remembered that @asvetliakov mentioned it would break vscode jumplist. We need to decide how we draw the line between vscode and neovim.

From a technical standpoint, I am also thinking if we should associate the lock in document_change_manager with the new lock, as we need to update cursor and content scroll together or it causes jitter.

@theol0403
Copy link
Member Author

@zbw8388 thanks for working on this too!

I vote that we let neovim handle all scrolling. This will let us remove all custom scrolling implementations (zz, C-e, etc). I think that switching to neovim-like behaviour where the content is scrolled instead of the cursor is a good feature.

I'm not sure if the jumplist note is relevant, but we should see if we can work around that if it is an issue. It might be worth it to look into if we can use neovim jumplist again (and also support jumplist from outline view).

If we can remove jitter, and let neovim handle all scrolling, that would be ideal.

I'm not very experienced with synchronization, but it seems like you are quite good with that!

I'm a little lost as to how to fix the current issues in my branch. Your branch (with a few tweaks from mine) seems to work very well to fix the original issue, but mine seems to have a lot of bugs (jitter, incorrect scrolling, etc). I think implementing a lock like you said would be a good next step.

@zbw8388
Copy link
Collaborator

zbw8388 commented Aug 1, 2022

I'm not sure if the jumplist note is relevant, but we should see if we can work around that if it is an issue. It might be worth it to look into if we can use neovim jumplist again (and also support jumplist from outline view).

What was the reason for not using neovim in the current implementation?

I'm a little lost as to how to fix the current issues in my branch. Your branch (with a few tweaks from mine) seems to work very well to fix the original issue, but mine seems to have a lot of bugs (jitter, incorrect scrolling, etc). I think implementing a lock like you said would be a good next step.

Ah I dropped a comment in one of your commits: ee7f35d#r79878840. Basically editor.revealRange does not produce a thenable object (but I believe internally uses an asynchronous call), which makes vscode ask neovim to scroll to a stale position, and that scrolling will get registered by vscode again. If we use the thenable object, we ensure that the message to neovim always uses the latest scrolling position by locally "locking" the vscode scolling position setting.

I'm still a bit worried about the mouse wheel scrolling though. Current implementation one cannot scroll outside of the cursor and we need to agree on what to do on that.

@theol0403
Copy link
Member Author

What was the reason for not using neovim in the current implementation?

I think the main reason was that navigating using vscode methods (outline view, clicking, go to definition, etc) would not register a jump in neovim jumplist for some reason, so the code had to be patched to manually add to the jumplist:

#461
#452

However, I'm not sure why vscode movement (which results in setting the cursor in neovim) doesn't add to jumplist - maybe this can be fixed.

Either way, the current implementation uses the vscocde jumplist (which main disadvantage is that it doesn't have buffer-local jump history).

I see about the thenable thing, although is it possible that the excecuteCommand only resolves the promise when the command is sent, not completed? Either way I'm good to revert, it seemed to improve jitter but that could have made it architecturally worse.

I'm not totally sure what the problem with mouse wheel scrolling is, couldn't we just replicate neovim's behaviour, where the cursor stays in the center of the screen while scrolling?

@Armadillidiid
Copy link

Hi @theol0403, thank you for getting on this! I created a branch in this repo (https://github.com/vscode-neovim/vscode-neovim/tree/experimental-scroll) so it would be easier to collaborate.

I also vote for the first option. I was playing around with the idea of letting neovim handle the content scroll, but I removed that functionality due to mouse-wheel scrolling. In vscode it is different from that of neovim: vscode (like many other apps) scrolls the content, while neovim scrolls the cursor. If we decided to scroll cursor during mouse wheel scroll, then I remembered that @asvetliakov mentioned it would break vscode jumplist. We need to decide how we draw the line between vscode and neovim.

From a technical standpoint, I am also thinking if we should associate the lock in document_change_manager with the new lock, as we need to update cursor and content scroll together or it causes jitter.

As vscode is generally a bit troublesome with keyboard scrolling, I wrote a plugin recently that I think works fairly well. Maybe it also improves the experience for others who use vscode-neovim until there is a final implementation and solution. https://github.com/tobealive/german-scroll.code

And so great to see the skyrocketing development in recent days. I think slowly but steady, you make it better than the VSCodeVim extension blush

Is it possible to have something like vim-smoothie to make page up and page down C-d C-u imitate a mouse scroll?

@dragonlobster
Copy link

dragonlobster commented May 16, 2023

hi may or may not be related but i would like to center the window after C-u, so <C-u>zz i was wondering if it was possible? i tried this but it didn't work:

vim.api.nvim_set_keymap("n", "<C-u>", "<C-u>zz", { noremap=true })

@theol0403 theol0403 added bug Something isn't working category: sync synchronization between vscode and nvim (text, windows, etc) priority labels Jul 4, 2023
@lawrence-laz
Copy link

Super annoying with scrolloff enabled, that after C-u and C-d the cursor is not vertically centered, but I noticed that any further movement returns cursor back to center, so this is a good enough workaround I am currently running:

// keybindings.json
  {
    "command": "-vscode-neovim.send",
    "key": "ctrl+d"
  },
  {
    "command": "-vscode-neovim.send",
    "key": "ctrl+u"
  },
  {
    "command": "vscode-neovim.send",
    "key": "ctrl+d",
    "args": "<C-d>jk",
  },
  {
    "command": "vscode-neovim.send",
    "key": "ctrl+u",
    "args": "<C-u>jk",
  },

@abmantis
Copy link

Super annoying with scrolloff enabled, that after C-u and C-d the cursor is not vertically centered, but I noticed that any further movement returns cursor back to center, so this is a good enough workaround I am currently running:

// keybindings.json
  {
    "command": "-vscode-neovim.send",
    "key": "ctrl+d"
  },
  {
    "command": "-vscode-neovim.send",
    "key": "ctrl+u"
  },
  {
    "command": "vscode-neovim.send",
    "key": "ctrl+d",
    "args": "<C-d>jk",
  },
  {
    "command": "vscode-neovim.send",
    "key": "ctrl+u",
    "args": "<C-u>jk",
  },

I have tried that and it does not work for me. Also tried using zz instead of jk but it also does not center the cursor after moving. Any tips?

@lawrence-laz

This comment was marked as resolved.

@tom-pollak
Copy link

If you set scrolloff to 1000 it'll keep your cursor centered all the time? Not exactly what you want of course 🙂

I've found the neovim-vscode scrolling generally sucks anyway, so I set mine to 16j/k

@ZeChArtiahSaher
Copy link

ZeChArtiahSaher commented Oct 10, 2023

Given the amount of various quirks and desyncs that happen in the smallest of situations probably the entire viewport sync needs a proper rewrite

@xiyaowong
Copy link
Collaborator

xiyaowong commented Oct 10, 2023

@ZeChArtiahSaher The problem about viewport is not so serious now, is it? After merging PR #1459, most of the problems related to viewport that I usually encounter have been improved.

But if someone starts to optimize viewport in an all-round way, that would be great. However, the implementation scheme may need to be discussed again. @theol0403

@tom-pollak
Copy link

Just wanted to say thanks! When I booted up vscode this week jump to next problem and peeking references in the same file are now automagically working thanks to your fix, thanks a million :)

@ZeChArtiahSaher
Copy link

ZeChArtiahSaher commented Oct 11, 2023

Yeah like the stability and jitter is still present, I think it becomes esp obvious if you get a larger file, I see the merge. Currently on latest release but also I did try some of @theol0403's earlier forks some time in the past (half-year/year ago?) which kept cursor centered with surrounding lines/scrolloff 999, it worked but also was still exhibiting plenty jitter. Btw seems current release doesn't have scrolloff centering working (or never did? - or I broke something _)?

It's probably a general design problem; permissive or enforcing style of state drift between vscode and neovim. I think there should be some way to manage this in a more stable way, when such things happen.

@tom-pollak
Copy link

tom-pollak commented Nov 15, 2023

workaround for centered scrolling

Disable default implementation:

  {
    "key": "ctrl+u",
    "command": "-vscode-neovim.ctrl-u",
    "when": "editorTextFocus && neovim.ctrlKeysNormal.u && neovim.init && neovim.mode != 'insert' && editorLangId not in 'neovim.editorLangIdExclusions'"
  },
  {
    "key": "ctrl+d",
    "command": "-vscode-neovim.ctrl-d",
    "when": "editorTextFocus && neovim.ctrlKeysNormal.d && neovim.init && neovim.mode != 'insert' && editorLangId not in 'neovim.editorLangIdExclusions'"
  }

Using multicommand scroll and move the cursor to center:

settings.json

  "multiCommand.commands": [
    {
      "command": "multiCommand.pageUp",
      "sequence": [
        {"command": "editorScroll", "args": {"to": "up", "by": "wrappedLine", "value": 32 }},
        { "command": "cursorMove", "args": { "to": "viewPortCenter" } }
      ]
    },
    {
      "command": "multiCommand.pageDown",
      "sequence": [
        {"command": "editorScroll", "args": {"to": "down", "by": "wrappedLine", "value": 32 }},
        { "command": "cursorMove", "args": { "to": "viewPortCenter" } }
      ]
    }
  ],

keybindings.json

  {
    "key": "ctrl+u",
    "command": "multiCommand.pageUp",
    "when": "editorTextFocus"
  },
  {
    "key": "ctrl+d",
    "command": "multiCommand.pageDown",
    "when": "editorTextFocus"
  }

Alternatively you can use "by": "halfPage" but this seems more buggy

@ZeChArtiahSaher
Copy link

ZeChArtiahSaher commented Nov 17, 2023

@tom-pollak You are a legend! It works overall, only small gripe with the render frame stutter however much more usable than current pulls

@AndreiMoraru123
Copy link

@tom-pollak indeed best solution so far

@jemorriso
Copy link

For me multiCommand was greyed out (not sure if it's an extension I don't have) so I modified @tom-pollak's solution like so:

  {
    "key": "ctrl+u",
    "command": "runCommands",
    "args": {
      "commands": [
        {
          "command": "editorScroll",
          "args": {
            "to": "up",
            "by": "halfPage"
          }
        },
        {
          "command": "cursorMove",
          "args": { "to": "viewPortCenter" }
        }
      ]
    }
  },
  {
    "key": "ctrl+d",
    "command": "runCommands",
    "args": {
      "commands": [
        {
          "command": "editorScroll",
          "args": {
            "to": "down",
            "by": "halfPage"
          }
        },
        {
          "command": "cursorMove",
          "args": { "to": "viewPortCenter" }
        }
      ]
    }
  },

@TiLopes
Copy link

TiLopes commented Feb 27, 2024

I'm currently using @tom-pollak settings however ctrl+u does not go to the first line whereas ctr+d does. Is there any fix to this?

@Laurensdc
Copy link

@jemorriso's solution worked brilliantly without any extra plugins.
Didn't even have to unmap the default keybindings.

Expanded it for full page scrolls:

{
    "key": "ctrl+b",
    "command": "runCommands",
    "args": {
      "commands": [
        {
          "command": "editorScroll",
          "args": {
            "to": "up",
            "by": "page"
          }
        },
        {
          "command": "cursorMove",
          "args": {
            "to": "viewPortCenter"
          }
        }
      ]
    }
  },
  {
    "key": "ctrl+f",
    "command": "runCommands",
    "args": {
      "commands": [
        {
          "command": "editorScroll",
          "args": {
            "to": "down",
            "by": "page"
          }
        },
        {
          "command": "cursorMove",
          "args": {
            "to": "viewPortCenter"
          }
        }
      ]
    }
  },

@sidneyfrancois
Copy link

@jemorriso Thanks! Your solution just worked fine for me, for the first time the scroll feels so fast and cosistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working category: sync synchronization between vscode and nvim (text, windows, etc) priority
Projects
None yet
Development

No branches or pull requests