Formatting adding trailing newline clears last line

[carlpett]

• OS and language server
  Linux + Gopls
• How you installed LSP (Package Control or from git?)
  Package Control
• Minimal reproduction steps

package main

import (
	"fmt"
)

func main() {
	fmt.Println("Hello, world")
} // No newline!

Format

package main

import (
	"fmt"
)

func main() {
	fmt.Println("Hello, world")

• Log
  No diagnostic output.

Initially reported to gopls, but they pointed out that the gopls commandline does the right thing.

Is this a LSP issue or Sublime itself?
Let me know if I can provide any other helpful information!

[rwols]

Managed to reproduce this and get the payload so we can see what is happening.

[
    {
        "range": {
            "start": {
                "line": 8,
                "character": 0
            },
            "end": {
                "line": 9,
                "character": 0
            }
        },
        "newText": ""
    },
    {
        "range": {
            "start": {
                "line": 9,
                "character": 0
            },
            "end": {
                "line": 9,
                "character": 0
            }
        },
        "newText": "} // No newline!\n"
    }
]

I've added a debug print to see how the edits are applied.
This seems to trip up our "do the edits in reverse-order" trick:

LSP: applying change: (92,92) } // No newline!
LSP: applying change: (76,109) 

cc @haferburg

[rwols]

Let's bring in the spec :)

Text edits ranges must never overlap, that means no part of the original document must be manipulated by more than one edit. However, it is possible that multiple edits have the same start position: multiple inserts, or any number of inserts followed by a single remove or replace edit. If multiple inserts have the same position, the order in the array defines the order in which the inserted strings appear in the resulting text.

In this case it looks like this is breaking spec. We first remove the line } // No newline! and then insert at that position.

However we could also fix this by changing this line to

        return change[1][0], change[1][1], -index

[haferburg]

I'd say the two edits are adjacent, but not overlapping. So I don't think this is breaking the spec.

But I also don't see how this input would trip up the algorithm. If you first insert the "} // No newline!\n" at 9:0, then delete 8:0 to 9:0.

But it looks like this might be some corner case because it is editing the last line of the file. Maybe the Sublime API is weird in this regard?

[haferburg]

Wait a second, so the original file has no newline, but the deletion is still from 8:0 to 9:0, even though there is no line 9. How come?

[haferburg]

I would expect the deletion to be 8:0 to 8:16.

[rwols]

Just a note that coc.nvim uses this library

https://github.com/microsoft/vscode-languageserver-node

to defer its sorting logic of the text edits. I'm not an expert at TypeScript but I believe the relevant logic for it is here:

https://github.com/microsoft/vscode-languageserver-node/blob/9175d3894ed952b0ffd416e55c03c877b475b51e/types/src/main.ts#L2523-L2580

[tomv564]

Nice find @rwols, I like their logic but they can return the entire text of the edited document and refer to the unmodified original for each of the edits. Perhaps there's an approach that lets us return to this simplicity while avoiding the slowness of the initial approach we had?

[rwols]

Perhaps there's an approach that lets us return to this simplicity while avoiding the slowness of the initial approach we had?

We would have to implement our own "convert row-col to offset", that would be painful.

There's this doc comment about what happens to invalid points:

https://github.com/microsoft/vscode-languageserver-node/blob/9175d3894ed952b0ffd416e55c03c877b475b51e/types/src/main.ts#L25

To reiterate what's said there: the line number is clamped between 0 and #lines (inclusive).

It then depends on what the offsetAt function does with such a point, and what substring does when its arguments are (or may be) OOB. Does substring create newlines?

[rwols]

I want to mark this as a language server bug. We have thought about it for a while. We verified our unit tests. And we discovered an out-of-bounds position provided by the server. It depends on the implementation of the text editor client what will happen with OOB positions, and language servers shouldn't depend on implemention-defined behavior.

[tomv564]

If a line number is greater than the number of lines in a document, it defaults back to the number of lines in the document.

What if we apply this to the incoming positions? Will it fix the issue?

[rwols]

What if we apply this to the incoming positions? Will it fix the issue?

After screwing around in the console we could perhaps use this info to our advantage:

>>> view.rowcol(view.size())[0]
49
>>> view.rowcol(view.size() - 1)
(48, 69)