atom-community / atom-languageclient

Provide integration support for adding Language Server Protocol servers to Atom.
https://www.npmjs.com/package/atom-languageclient
MIT License
44 stars 13 forks source link

Use textEdit of completionItem unconditionally #207

Open machitgarha opened 2 years ago

machitgarha commented 2 years ago

Identify the Bug

php-ide-serenata#527

Description of the Change

The description is explained in the commit. Please read that beforehand.

Give priority to the server's CompletionItem.textEdit rather than the replacementPrefix extracted in the client. These cases shouldn't be different in most cases, but not some. For example, in the case of a trigger character, the server may decide to replace it along with the prefix, but the client ignores it, resulting in wrong output, because newText is created on the server and it is the server who actually knows where it must be placed.

Alternate Designs

There are many alternatives actually. But the current change is minimal and it only affects if the server set the CompletionItem.textEdit.

Possible Drawbacks

Possibly zero. If a package using this library get impacted by this, then the LSP server it uses is doing something wrong, so a hidden bug will be exposed. But this is a rare case, at least theoretically.

Verification Process

Tested with php-ide-serenata. The mentioned issue is fixed there (e.g. autocompletion of $this leads to $$this).

Release Notes

machitgarha commented 1 year ago

Yes, I realized it. As soon as I find time, I'll look into it. Thanks for your attention.