sublimelsp / LSP

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

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

Closed ayoub-benali closed 5 years ago

ayoub-benali commented 5 years ago

When autocomplete has to edit before the completion position the trimming introduced in https://github.com/tomv564/LSP/pull/295 leads to a bug, see line

You can see an example bellow using Scala with Metals: broken-autocomplete

Here is the CompletionItem sent back by the server:

{
    'insertTextFormat': 2,
    'data': {
        'symbol': 'example/Foo#myFunction().',
        'target': 'file:/home/ayoub/workspace/testproject/?id=root'
    },
    'detail': 'override def myFunction(): Unit',
    'sortText': '00000',
    'filterText': 'override def myFunction',
    'preselect': True,
    'label': 'override def myFunction(): Unit',
    'kind': 2,
    'additionalTextEdits': [],
    'textEdit': {
        'newText': 'override def myFunction(): Unit = ${0:???}',
        'range': {
            'start': {
                'line': 7,
                'character': 2
            },
            'end': {
                'line': 7,
                'character': 18
            }
        }
    }
}

Expected behavior

Before:

def myF<ctrl+space>

After:

override def myFunction(): Unit = ???

Please note the bug doesn't happen when the prefix is fully specified: broken-autocomplete-2

Any ideas on how to fix this kind of issues ? I am willing to make a PR but first would like to get your thoughts

cc @nanoant

ayoub-benali commented 5 years ago

One idea could be that we:

  1. remove the trimming applied here
  2. after completion we add extra logic in CompletionSnippetHandler to remove any leftovers.

We know:

Therefore we can assume that the remaining can simply deleted.

Example

// first
def myF<ctrl+space>
// after user choice we get broken code
def override def myFunction1(): Unit = ???
// finaly delete the borken prefix with a diff between and range and last prefix
override def myFunction1(): Unit = ???

The whole challenge is to somehow know which choice the user make so we can decide which fix we need to apply if need to be

@rwols can you think of a case where this logic would brake things ?

nanoant commented 5 years ago

@ayoub-benali Unfortunately, I don't see good solution for this, other than making Sublime Text aware of completion text replacement ranges. The problem is that the completion prefix myFunctionCtrlSpace and its completion text override def myFunction1(): Unit = ??? starts before the prefix.

This is shortcoming of Sublime Text that does not provide explicit way to apply replacement with explicit range. It does only replace current prefix with proposed text, see https://github.com/tomv564/LSP/pull/295#discussion_r175924508.

Therefore I have added feature request to Sublime Text there: https://github.com/SublimeTextIssues/Core/issues/2754

ayoub-benali commented 5 years ago

fixed by https://github.com/tomv564/LSP/pull/602