Applying large text edits results in the viewport shifting to the right

[uhthomas]

System information

macOS Catalina 10.15.5 (19F101) Sublime Text Dev Channel, Build 4074

Install

Package Control

Reproduction

Assuming the LSP configuration

{
    "lsp_format_on_save": true,
    "lsp_code_actions_on_save": {
        "source.fixAll": true,
        "source.organizeImports": true
    },
    "show_symbol_action_links": true,
    "only_show_lsp_completions": true,
    "clients": {
        "gopls": {
            "enabled": true,
            "settings": {
                "gopls.usePlaceholders": true,
                "gopls.analyses": {
                  "fillreturns": true,
                  "nonewvars": true,
                  "undeclaredname": true,
                  "unusedparams": true
                },
                "gopls.completeUnimported": true,
                "gopls.staticcheck": true
            }
        }
    }
}

Then, given the snippet

package main

import "fmt"

func main() {
    fmt.Println("Hello, world!" ) // <--- place your cursor here
}

Place your cursor in front of the arrow and then save. The cursor will jump to column 1.

[rchl]

The server sends two changes to fix formatting. The first that removes the whole 6th line and the second that adds back formatted one.

Server response

``` [ { "newText": "", "range": { "end": { "character": 0, "line": 6 }, "start": { "character": 0, "line": 5 } } }, { "newText": "\tfmt.Println(\"Hello, world!\") // <--- place your cursor here\n", "range": { "end": { "character": 0, "line": 6 }, "start": { "character": 0, "line": 6 } } } ] ```

I think that fixing this would be quite tricky depending on how correct we want to be.

This case might seem simple on the surface but many won't be and the algorithm to anchor the cursor to the original place would likely be quite complex.

Ideally, we would reuse something that ST uses internally but I don't think there is any API exposed for that.

Maybe we could apply edits in a different way to fix it but again, I doubt it.

[uhthomas]

Hah, interesting. It's definitely more complex than I first imagined. I wonder how editors like vscode, atom, Goland, etc. handle this case? sublimelsp is the only project I've noticed this sort of behaviour.

[rwols]

We could try adding caret stability functionality but something general for all language servers is going to be complex. The first thing that comes to mind is to check whether there are edits on the caret's line, and if so, re-position the caret manually somehow.

[uhthomas]

It might be a good idea to iterate on this process? IMO anchoring the caret at a fixed point is almost certainly better than sending it to column 1. It might be worth taking a look at vscode and seeing how they handle it, as the project is open source and also has a general purpose LSP.

[rchl]

It's most likely handled at the core level of the editor. Maybe VSCode handles that case better. We are just using ST's APIs to remove and add regions as instructed by the server.

[rchl]

I was thinking that we could process the payload ourselves to create a minimal change. We could do that by applying changes in memory and then creating minimal change from the diff. That would help this case and also the ridiculous case of pyls where it always returns back the whole document.

I believe this might be an implementation of such algorithm: https://github.com/microsoft/vscode-eslint/blob/master/server/src/diff.ts (in JS).

[uhthomas]

That sounds like a great idea! Is there another open issue for pyls?

[rwols]

There's also https://docs.python.org/3/library/difflib.html#sequencematcher-objects

[rwols]

It should also be a server setting because for example clangd already has minimal text changes. It would be a waste of CPU cycles to do it again ourselves.

[rchl]

Yes, eslint also sends minimal changes.

BTW, eslint implementation should give LSP compatible change chunks so it has advantage over more generic diff libraries. Just needs to be rewritten in python.

[acheronfail]

FWIW I'm also seeing this while using rust-analyzer for LSP. Formatting is janking the viewport's scroll around.

[rwols]

See: https://github.com/sublimehq/sublime_text/issues/4144