sublimelsp / LSP

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

additionalTextEdits support in auto completion #529

Closed kadircet closed 5 years ago

kadircet commented 5 years ago

I believe this issue was also mentioned in #294 but it was optional so it didn't get addressed(if there were any other reasons, I couldn't see within the issue itself).

LSP supports additionalTextEdits field in completion items, and at least clangd makes use of it to provide user with include insertions. I was wondering if there was any particular reason for not supporting this. I believe it could be added in a similar way to resolve support.

rwols commented 5 years ago

It is a somewhat daunting task to start implementing this, because the ST API is simply not capable of arbitrary text replacements for a completion item. An initial (naive) thought on how to implement this would require a callback for which completion item the user selected from the ST completion widget, but currently we cannot attach a callback to that event. Thus we're limited to providing the ST completion widget one single textEdit (as its API demands).

rwols commented 5 years ago

I hope Jon and Will can one day find the time to re-think the completion widget a-la Atom/VSCode.

kadircet commented 5 years ago

I see that auto completion lacks the functionality to do arbitrary insertions but it seems like you've a way to perform actions on the view after a completion item has been selected (https://github.com/tomv564/LSP/blob/master/plugin/completion.py#L110).

And it seems like there are replace/insert/erase functionalities at arbitrary positions https://www.sublimetext.com/docs/3/api_reference.html#sublime.View

Why these two cannot be combined?

predragnikolic commented 5 years ago

I think this is doable.

Here is an example of what I think would work...

    def handle_resolve_response(self, response, view):
-        # replace inserted text if a snippet was returned.
-        if current_completion and response.get('insertTextFormat') == 2:  # snippet
-            insertText = response.get('insertText')
-            try:
-                sel = view.sel()
-                sel.clear()
-                sel.add(current_completion.region)
-                view.run_command("insert_snippet", {"contents": insertText})
-            except Exception as err:
-                exception_log("Error inserting snippet: " + insertText, err)

def handle_resolve_response(self, response):
+    view.run_command("example", {"response": response})

+ class ExampleCommand(sublime.TextCommand):
+    def run(self, edit, response):
+       # handle the response there

The advantage of calling our own TextCommand, instead of running insert_snippet command is that you get an edit token. With it you can insert, erase, replace.

rwols commented 5 years ago

I'm not sure how the cursor will respond to text replacements like that. The thing with the auto-complete widget is that it replaces text under the cursor, and so there is probably extra logic accounted for that. But, it is certainly good to try this approach anyway.

ayoub-benali commented 5 years ago

I would also be interested in additionalTextEdits support for Metals. @predragnikolic did you try your example ? I am willing to help

predragnikolic commented 5 years ago

@ayoub-benali no, I just shared my thought on how I would go about it... but I didn't try it. Currently I am looking for a job and I can't really focus on open-source right now :)

tomv564 commented 5 years ago

Give it a go! Ideally the existing " apply (workspace) edits" logic can be re-used for this!

ayoub-benali commented 5 years ago

I will definitely give it a try @tomv564 can you point me to the " apply (workspace) edits" code ?

ayoub-benali commented 5 years ago

but currently we cannot attach a callback to that event. Thus we're limited to providing the ST completion widget one single textEdit (as its API demands).

For what is worth I made a feature request in ST forum about this subject here ...

ayoub-benali commented 5 years ago

I found a way to detect the string that is added to the view right after a commit_completion command. This way we can compare what is inserted with one of the user choices (state that is saved beforehand) and then check if that choice had any additionalTextEdits by doing string comparaison.

I just have a small issue though:

The server can send such a string in CompletionItem.insertedText (which I save to do the comparison later)

override def visitFile(x$1: Path, x$2: BasicFileAttributes): FileVisitResult = ${0:???}

Once selected, added to the view and "interpreted" by sublime would become:

override def visitFile(x: Path, x: BasicFileAttributes): FileVisitResult = ???

The problem is that when we compare the two strings they are different because of the $ magic. Is there a way to calculate what would a string be once "interpreted" by sublime to get the same result? Or any other idea to fix this issue ? cc @tomv564 @rwols

ayoub-benali commented 5 years ago

Given that doing string comparison, as shown above, to detected user input won't work I think we are blocked by the sublime API :(

Found this feature request https://github.com/SublimeTextIssues/Core/issues/2077 maybe I will be picked by @wbond if we show more interest, it is labeled as minor ...

ayoub-benali commented 5 years ago

I think this one can be closed see https://github.com/tomv564/LSP/pull/602