sublimelsp / LSP

Client implementation of the Language Server Protocol for Sublime Text
https://lsp.sublimetext.io/
MIT License
1.65k stars 182 forks source link

Completion textEdit range not being honored by filter #976

Open razzeee opened 4 years ago

razzeee commented 4 years ago

Hey there,

it seems like this lsp client is not correctly implementing all completion cases. Probably due to bad docs on the protocol side of things.

Here's an explanation from the maintainer of the protocol, on how it should work: https://github.com/microsoft/language-server-protocol/issues/898#issuecomment-593968008

The problem in our case is with the elm language server see https://github.com/elm-tooling/elm-language-server/issues/257 for tracking on our side.

We also had that problem with CoC for Vim, which was fixed recently: https://github.com/neoclide/coc.nvim/commit/af2b7f55843bbb8e3d6a1fc9c41e7f7a54d3b27d

Cheers

rwols commented 4 years ago

I understand the problem, but the fix is non-trivial. Some language servers like clangd return high-quality fuzzy matches. These work remarkably well with ST's internal fuzzy prefix matching. And I don't want to inhibit those completions.

Possible solutions include:

  1. Only check prefix match after a trigger character (. in this case)
  2. Use difflib (perhaps get_close_matches)
  3. Communicate with SublimeHQ to bring the internal fuzzy string matching algorithm to the Python API.
razzeee commented 4 years ago

I don't think 1 would fix this? Maybe I'm not getting what your saying, just in case, some clarification. The matching is also broken after the dot and not only there.

sublime_text_2020-04-27_12-49-28

rwols commented 4 years ago

In your example above, you expect to only see Decode.map as completion, right? Your text edit would look something like

{
  'newText': 'Decode.map',
  'range':
  {
    'start': {'line': 26, 'character': 0},
    'end': {'line': 26, 'character': 7}  # remove "Decode."
  }
}

What I mean by (1) is that we extract the substring Decode. from the text buffer. We then check if newText starts with Decode. If it does, present it, otherwise discard it.

The above method is really bad for fuzzy completions though. Imagine that the user types this:

decode.

Then Decode.map would also be discarded. Yet, it's a useful completion because it also uppercases the D.

This is why I'm not eager to do the "starts with" check for word completions as it would inhibit quality completions from other language servers. But an OK solution would be to only do this after a trigger character, as probably then the user correctly typed the word.

I have another example of a language server that would probably break if we were to always filter non-fuzzily on prefixes, namely metals. For some completions it inserts an override keyword way before the prefix. We have a convenient unit test for this case:

https://github.com/sublimelsp/LSP/blob/d936b0443bc23bea7af6e0903975a381863cdd4e/tests/test_completion.py#L244-L267

Here, the user would type

def myF

and it'll be completed to

override def myFunction(): Unit = ???

This is desirable behavior.

TLDR: Filter only when the previous char is a trigger char, otherwise other language server's useful completions will break, or try (2) or (3)