Try to utilize LS completion textEdit if possible #295

nanoant · 2018-03-20T11:54:42Z

Sublime Text does not support explicit replacement with completion at given range, instead it replaces current completion prefix with the completion text. Knowing the completion prefix boundaries at plugin
side, we can trim textEdit range and text given by language server to the boundaries of the prefix.

Fixes #294

E.g. for PowerShell grammar and PowerShellEditorServices LS:

$tru<Ctrl+Space> (prefix "tru") -> now "true" instead of "$true"
$env:U<Ctrl+Space> (prefix "U") -> now "USER" instead of "$env:USER"

rwols · 2018-03-20T19:13:40Z

plugin/completion.py

+                        if last_row == start_row == end_row and start_col <= last_col:
+                            # sublime does not support explicit replacement with completion
+                            # at given range, but we try to trim the textEdit range and text
+                            # to the start location of the completion


Have you tried using the LspApplyDocumentEditCommand? (it's in edit.py)

The problem is that insert_text is prepared for each completion before the choice is made by the user.

After choice is made Sublime simply replaces prefix with whatever was the completion text returned by plugin in previous step, at this point there is no way to run plugin callback such as LspApplyDocumentEditCommand.

On the other hand language server does not know what is the prefix Sublime tries to complete, it does receive only the location you want to complete and knows document content, hence when you do $truCtrlSpace language server does not know if the prefix was tru or $tru.

In my Powershell example Sublime prefix is tru however language server is returning $true, and after completion we end up with $$true (double dollar sign) instead $true.

My PR uses extra information from textEdit response (if present) to make sure the completion starts exactly where the prefix begins, so Sublime does the proper replacement.

It is not perfect solution, it does assumption that everything happens on single line, and may not work as intended in extreme cases e.g. if language server returns %true for $truCtrlSpace, even it does not make sense, it will be still completed to $true instead of %true, but we can't do anything about this with the way Sublime currently handles completions.

Good points.

nanoant · 2018-03-21T10:11:46Z

FYI fixed editText -> textEdit in my PR commit message and reworked a bit the description.

tomv564 · 2018-06-01T20:17:38Z

plugin/completion.py

@@ -243,6 +243,24 @@ def format_completion(self, item: dict) -> 'Tuple[str, str]':
                hint = completion_item_kind_names[kind]
        # label is an alternative for insertText if insertText not provided
        insert_text = item.get("insertText") or label
+        # try to handle textEdit if present
+        text_edit = item.get("textEdit")
+        if text_edit:


Can we hide this behaviour behind a global LSP setting for now? I'd like to avoid hitting this code path automatically for all language servers that happen to send textEdit.

Additionally, can we extract the logic into a function returning an Optional[str], which is assigned to insert_text if not None?

tomv564 · 2018-06-01T20:21:44Z

plugin/completion.py

+                    start_row, start_col = range_start.get("line"), range_start.get("character")
+                    end_row, end_col = range_end.get("line"), range_end.get("character")
+                    if all([start_row is not None, start_col is not None,
+                            end_row is not None, end_col is not None]):


The range is probably specified to be required, in which case I like to avoid the None checks (I'm fine with errors from non-compliant servers).
There is existing logic for parsing ranges in protocol.py

tomv564

Thanks for explaining the changes, sorry about the super late review!

nanoant · 2018-06-02T13:03:38Z

@tomv564 No pbm for late review, I'll adapt my changes to your comments beginning of the next week. Cheers.

Sublime Text does not support explicit replacement with completion at given range, instead it replaces current completion prefix with the completion text. Knowing the completion prefix boundaries at plugin side, we can trim textEdit range and text given by language server to the boundaries of the prefix. Fixes sublimelsp#294 E.g. for PowerShell grammar and PowerShellEditorServices LS: $tru<Ctrl+Space> (prefix "tru") -> now "true" instead of "$true" $env:U<Ctrl+Space> (prefix "U") -> now "USER" instead of "$env:USER"

nanoant · 2018-06-04T08:57:23Z

@tomv564 I have applied your remarks to my patch, except your request to make textEdit optional. I would argue to make it default -- justification quoting https://microsoft.github.io/language-server-protocol/specification:

The insertText is subject to interpretation by the client side. Some tools might not take the string literally. For example VS Code when code complete is requested in this example con and a completion item with an insertText of console is provided it will only insert sole. Therefore it is recommended to use textEdit instead since it avoids additional client side interpretation.

I think CompletionHelper.text_edit_text checks (single line edit and position matching) should be safe enough to make it default. However I am open for a discussion, especially if you can think of any example that can break text_edit_text.

NOTE: I have also changed '\$' into '\\$' since linter was complaining about potential escape code problem.

tomv564

Thanks for pointing out the spec, sounds like defaulting to textEdit is the preferred solution. Looks like tight code, too. Can you look at the comment I left about your escape fix?

tomv564 · 2018-06-04T17:34:16Z

plugin/completion.py

        if len(insert_text) > 0 and insert_text[0] == '$':  # sublime needs leading '$' escaped.
-            insert_text = '\$' + insert_text[1:]
+            insert_text = '\\$' + insert_text[1:]


I'm a bit worried about this change as it was a working fix for completions from PHP language servers. Can you tell me what is problematic, or should we take this change in a separate PR?

It is just about escaping in Python -- '...' still respects backslash escape codes, \$ is not proper escape code so Python leaves it as is, however Linters complain about this invalid escape code, hence backslash should be escaped, or raw Python string should be used.

nanoant · 2018-06-05T09:14:22Z

@tomv564 Thanks a lot!

rwols reviewed Mar 20, 2018

View reviewed changes

nanoant force-pushed the patch/completion-textEdit-handling branch from ed368e8 to a05eef6 Compare March 21, 2018 10:09

nanoant changed the title ~~Try to handle completion editText if possible~~ Try to utilize LS completion textEdit if possible Mar 21, 2018

nanoant force-pushed the patch/completion-textEdit-handling branch 2 times, most recently from 019380a to a9e5f85 Compare April 9, 2018 10:47

nanoant force-pushed the patch/completion-textEdit-handling branch from a9e5f85 to aca4141 Compare April 30, 2018 10:29

tomv564 reviewed Jun 1, 2018

View reviewed changes

tomv564 requested changes Jun 1, 2018

View reviewed changes

nanoant force-pushed the patch/completion-textEdit-handling branch from aca4141 to b94f8dc Compare June 4, 2018 08:49

nanoant force-pushed the patch/completion-textEdit-handling branch from b94f8dc to 1993f04 Compare June 4, 2018 08:50

tomv564 requested changes Jun 4, 2018

View reviewed changes

tomv564 approved these changes Jun 5, 2018

View reviewed changes

tomv564 merged commit ed7d7a2 into sublimelsp:master Jun 5, 2018

nanoant mentioned this pull request Jul 4, 2018

Unintended whitespace removal #368

Closed

ayoub-benali mentioned this pull request Mar 21, 2019

bug in auto completion when edit behind the completion position #536

Closed

nanoant mentioned this pull request Mar 23, 2019

Extend on_query_completions return with optional explicit replacement range sublimehq/sublime_text#2754

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try to utilize LS completion textEdit if possible #295

Try to utilize LS completion textEdit if possible #295

nanoant commented Mar 20, 2018 •

edited

rwols Mar 20, 2018

nanoant Mar 20, 2018 •

edited

rwols Mar 21, 2018

nanoant commented Mar 21, 2018

tomv564 Jun 1, 2018 •

edited

tomv564 Jun 1, 2018

tomv564 left a comment

nanoant commented Jun 2, 2018

nanoant commented Jun 4, 2018 •

edited

tomv564 left a comment •

edited

tomv564 Jun 4, 2018

nanoant Jun 5, 2018

nanoant commented Jun 5, 2018

Try to utilize LS completion textEdit if possible #295

Try to utilize LS completion textEdit if possible #295

Conversation

nanoant commented Mar 20, 2018 • edited

rwols Mar 20, 2018

Choose a reason for hiding this comment

nanoant Mar 20, 2018 • edited

Choose a reason for hiding this comment

rwols Mar 21, 2018

Choose a reason for hiding this comment

nanoant commented Mar 21, 2018

tomv564 Jun 1, 2018 • edited

Choose a reason for hiding this comment

tomv564 Jun 1, 2018

Choose a reason for hiding this comment

tomv564 left a comment

Choose a reason for hiding this comment

nanoant commented Jun 2, 2018

nanoant commented Jun 4, 2018 • edited

tomv564 left a comment • edited

Choose a reason for hiding this comment

tomv564 Jun 4, 2018

Choose a reason for hiding this comment

nanoant Jun 5, 2018

Choose a reason for hiding this comment

nanoant commented Jun 5, 2018

nanoant commented Mar 20, 2018 •

edited

nanoant Mar 20, 2018 •

edited

tomv564 Jun 1, 2018 •

edited

nanoant commented Jun 4, 2018 •

edited

tomv564 left a comment •

edited