atom / atom-languageclient

Language Server Protocol support for Atom (the basis of Atom-IDE)
https://ide.atom.io/
MIT License
388 stars 78 forks source link

Auto replace #264

Closed Aerijo closed 5 years ago

Aerijo commented 5 years ago

This changes the replacementPrefix logic to remember the original range from the server, and attempt to match it as close as possible.

I won't be merging this soon, because I need some time for myself and others to see how terrible this attempt is and how it can be refined. It also needs tests added for the new behaviour. Also need to handle insertText gracefully.

Design

When we request a new set of completions, we note

Each suggestion from the server can have a custom range (single line is enforced by spec), but we expect most to match [TP, OBP]. The ones that don't have a custom prefix added from the custom start (CS) to OBP. As we also invalidate the cache if BP < OBP, this ensures CS <= OBP <= BP and so that part of the prefix is static.

E.g., it might look like

v CS                  v CE
 " . / p a t h / f i "
            TP^  OBP^

The spec is unclear about how to treat the ranges when the cursor moves. I feel like the following is a natural choice:

@isundaylee I'm not going to try and fix the end part of the replacement in this PR like we discussed. It's already complicated enough as it is. The fix would only apply to Text suggestions too, as snippets may be using snippet syntax in that range.

Aerijo commented 5 years ago

OK, I've updated the autocomplete tests. The old ones focused heavily on the details of the internal methods, which meant any changes would easily break them. The new tests only check the results of converting various different CompletionItem's into Autocomplete+ Suggestions, without specifying how the actual conversion should work.

I'll merge when they pass, and then try it out with the various current servers before making a release and bumping their versions.