LSP icon indicating copy to clipboard operation
LSP copied to clipboard

Behaviour mismatch between Sublime-LSP and erlang_ls

Open nwalker opened this issue 3 years ago • 4 comments

Describe the bug erlang_ls expects leading triggerCharacter to be preserved on completion, which is not always true in Sublime-LSP. I'm not even sure it is bug, it look like unclear definition of protocol.

erlang_ls reports this completionProvider capacity

'completionProvider':` {'resolveProvider': True, 'triggerCharacters': [':', '#', '?', '.', '-', '"']} 

When asked for completions of erlang module directive(something that starts with -, like -define(TEST, true).) erlang_ls replies with

:: <<< erlang-ls-debug 16: [
{'insertText': 'behaviour(${1:Behaviour}).', 'kind': 15, 'label': '-behaviour().', 'insertTextFormat': 2},
 {'insertText': 'callback ${1:name}(${2:Args}) -> ${3:return()}.', 'kind': 15, 'label': '-callback name(Args) -> return().', 'insertTextFormat': 2},
 {'insertText': 'compile(${1:}).', 'kind': 15, 'label': '-compile().', 'insertTextFormat': 2},
 {'insertText': 'define(${1:MACRO}, ${2:Value}).', 'kind': 15, 'label': '-define().', 'insertTextFormat': 2},
.....

It clearly expects that client will keep leading -. After completion confirm prefix gets removed including -, which results in obviously invalid syntax. This is default ST behavior.

Issue was described earlier in https://github.com/erlang-ls/erlang_ls/issues/1051, where @plux says

erlang_ls have defined "-" as a trigger character and thus it should be kept by the client.

But I was unable to find any confirmation for "should be kept" part(and it may be my fault).

I checked vscode, it keeps leading -. I asked about other editors and got the only reply that eglot in emacs also keeps leading -, but I can't treat it as a viable proof.

What do you think of this? Should I file clarification request in main LSP repo? Or should I just implement workaround in Sublime-LSP?

To Reproduce

  1. set up erlang_ls(may be non-trivial, especially on Windows).
  2. in empty file something.erl type -d and trigger autocomplete.
  3. select -define() and press TAB.

Preferred behavior In erlang source with connected erlang_ls -def completes to -define(MACRO, Value)..

Environment (please complete the following information):

  • OS: Ubuntu 18.04
  • Sublime Text version: 4113
  • LSP version: 1.9.0
  • Language servers used: https://github.com/erlang-ls/erlang_ls

nwalker avatar Aug 25 '21 22:08 nwalker

To avoid ambiguities, language servers should use textEdit instead of the deprecated insertText.

rwols avatar Aug 26 '21 11:08 rwols

I'm sorry to bring bad news, but it is was undeprecated for some reasons, so this is still kind of an issue. https://github.com/microsoft/vscode-languageserver-node/issues/492 https://github.com/microsoft/vscode-languageserver-node/commit/3a23efc2c43045900f37aa4b47e851aa04bd6c1a https://github.com/microsoft/language-server-protocol/commit/800c75b1ce901845b33f739f4331c324b1b96f75

For clarity - deprecation discussion and commits. https://github.com/microsoft/language-server-protocol/issues/264 https://github.com/microsoft/vscode-languageserver-node/commit/2662b4e6cb5e60829a01b63a940d8874032dcc7e https://github.com/microsoft/language-server-protocol/commit/e0c6d2da6fa64b32d3133cbe89ff925ba9566833

Anyway, I'll look into implementing TextEdits, it may result in more consistent behavior.

nwalker avatar Aug 26 '21 14:08 nwalker

There are major problems for insertText with respect to determining what the editor considers the prefix string to be.

If you type this:

foo.bar.b

and a server returns an insertText of “foo.bar.baz”, then it’ll depend on the logic of the prefix matcher of a specific editor what happens. Sane logic is to say that the prefix should end at the first word-separator character, or whitespace.

rwols avatar Aug 26 '21 14:08 rwols

I have one more idea that would probably make it work for this client. I see you have the - prefixed in the label property of the completion items. And I also notice there is no filterText. What this results in, is that the (ST) trigger will be the (LSP) label. However, triggers behave badly with non-word characters. If you supply a filterText with only alphanumeric characters (e.g. the behaviour in -behaviour()., then things might just work.

rwols avatar Aug 26 '21 20:08 rwols