autozimu / LanguageClient-neovim

Language Server Protocol (LSP) support for vim and neovim.
MIT License
3.55k stars 273 forks source link

Signs are not updating properly #1125

Closed martskins closed 3 years ago

martskins commented 3 years ago

I just noticed that signs are not always properly updated. The logic we're using to calculate the diff is incorrect in that it compares the previous signs with the new ones, but the previous signs were applied to an older version of the buffer, so there are cases were that diff is invalid.

It's hard to put in words, but essentially the line numbers of the signs don't match when comparing different versions of the documents, so diffing them could be inaccurate, which results in us removing signs incorrectly sometimes, or other times placing a sign more than once in the same line, which eventually leads to stale signs.

This gif illustrates shows the issue and you'll also see at the end of the gif that the placed signs list shows more than one sign placed on the same line:

http://g.recordit.co/DNSNfCBx5A.gif

I think to solve this we shouldn't really be applying diffs of signs but rather replacing all the signs. One thing we could do to make this a little cheaper is to only place the signs that overlap the current viewport and update on each cursor move. This would result in unnecessary updates too, but if we also store the previous viewport and compare the current one with the previous one I think we should be able to decide whether to update or not more accurately (if the viewport is the same after a cursor move, don't update signs).

I'm hoping to open a PR to solve this this weekend or early next week, but if someone else wants to give it a go I'm more than happy to help.

martskins commented 3 years ago

Opened #1126 in an attempt to fix this

martskins commented 3 years ago

Closing via #1126