microsoft / language-server-protocol

Defines a common protocol for language servers.
https://microsoft.github.io/language-server-protocol/
Creative Commons Attribution 4.0 International
11.17k stars 788 forks source link

Specify exactly what "the client will normalize line ending characters" means #1853

Open DanTup opened 10 months ago

DanTup commented 10 months ago

I recently hit a weird but where our formatter was changing \n in a document to \r\n. Because we have a step to minimize edits (because otherwise things like breakpoints could be lost), the edit was reduced to "insert \r before the \n".

However, in VS Code this resulted in \n\n in the document instead of \r\n which meant a new blank line appeared between every line of code:

https://github.com/Dart-Code/Dart-Code/assets/1078012/4cbe6c12-36ae-4b86-bd0f-d846fe0936c1

I filed https://github.com/microsoft/vscode/issues/200157 but this is considered by design. IMO this is not obvious behaviour from the current spec, and for servers not to produce edits that mess up a users code, the rules need to be clear.

So, I think the comments on the WorkspaceEditClientCapabilities.normalizesLineEndings class field need to clearly describe the rules of normalisation so that clients and servers have the same expectations/behaviour.

@dbaeumer I'm happy to send a PR detailing VS Code's behaviour if you think it's reasonable to define this as the LSP behaviour.

dbaeumer commented 10 months ago

I think we should document this as the wanted behavior when normalizesLineEndings is true

DanTup commented 10 months ago

I think maybe there is some missing information here... How does the server know which line endings the client is planning to use (for example if a document has mixed line endings)?

Or are we saying that if a client has normalizesLineEndings=true, it's never safe for a server to send edits that try to insert \r before a \n because the client may normalise it before insertion?

Honestly I feel like this behaviour is really awkward for servers, because they can't just minimize the edits they want to make - they need to add special handling for changing \n to \r\n (a replacement rather than an insertion) 😔