Showing completions after trigger chars can be unstable

[DoctorKrolic]

I managed to get a consistent repro of #52 (comment)

.csproj with this content is the only MSBuild-related file in the project

[tintoy]

Interesting! I'll see if we can create a test for this - we do have a bunch of similar ones and the test infrastructure to be explicit about starting project file and completion location.

[tintoy]

First step will be to verify that we are correctly parsing and interpreting the existing and modified document XML:

https://github.com/tintoy/msbuild-project-tools-server/blob/master/test/LanguageServer.Engine.Tests/XmlLocatorTests.cs (around line 210).

[tintoy]

The initial tests I've added indicate that the language service does correctly recognise all of the target locations for element insertion listed in #129.

I'll keep digging 🙂

[tintoy]

It looks like something has changed in VS Code's LSP implementation; the extension is definitely offering the correct completions but VS Code is not showing them.

Offering completions to replace element "" @ [7,36..7,38)
Completion was triggered by typing one or more characters; target range will be extended by 1 characters toward start of document (now: "[7,35..7,38)").
Offering 747 completion(s) for 7,37 -> [Element, Invalid]:/Project/PropertyGroup/ ([7,36..7,38))

followed by the completions:

[Trace - 5:34:26 PM] Received response 'textDocument/completion - (5)' in 55ms.
Result: [
    {
        "label": "<!-- -->",
        "kind": 1,
        "detail": "Comment",
        "documentation": "XML comment",
        "sortText": "1000<!-- -->",
        "insertTextFormat": 2,
        "textEdit": {
            "range": {
                "start": {
                    "line": 6,
                    "character": 34
                },
                "end": {
                    "line": 6,
                    "character": 37
                }
            },
            "newText": "<!-- $0 -->"
        }
    },
    {
        "label": "<OutputType>",
        "kind": 10,
        "detail": "Property",
        "documentation": "Type of output to generate (WinExe, Exe, or Library)",
        "sortText": "1000<OutputType>",
        "insertTextFormat": 2,
        "textEdit": {
            "range": {
                "start": {
                    "line": 6,
                    "character": 34
                },
                "end": {
                    "line": 6,
                    "character": 37
                }
            },
            "newText": "<OutputType>${1|Library,Exe|}</OutputType>"
        }
    },
    // ... snip ...
    {
        "label": "<RestoreOutputPath>",
        "kind": 10,
        "detail": "Property",
        "documentation": "I don't know anything about the 'RestoreOutputPath' property, but it's defined in this project (or a project that it imports); you can override its value by specifying it here.",
        "sortText": "1010<RestoreOutputPath>",
        "insertTextFormat": 2,
        "textEdit": {
            "range": {
                "start": {
                    "line": 6,
                    "character": 34
                },
                "end": {
                    "line": 6,
                    "character": 37
                }
            },
            "newText": "<RestoreOutputPath>$0</RestoreOutputPath>"
        }
    },
    {
        "label": "<NuspecOutputPath>",
        "kind": 10,
        "detail": "Property",
        "documentation": "I don't know anything about the 'NuspecOutputPath' property, but it's defined in this project (or a project that it imports); you can override its value by specifying it here.",
        "sortText": "1010<NuspecOutputPath>",
        "insertTextFormat": 2,
        "textEdit": {
            "range": {
                "start": {
                    "line": 6,
                    "character": 34
                },
                "end": {
                    "line": 6,
                    "character": 37
                }
            },
            "newText": "<NuspecOutputPath>$0</NuspecOutputPath>"
        }
    }
]

[tintoy]

Ok, so I have figured out what the problem is:

When you type a trigger character (e.g. <) to trigger completion, we have to tell VS Code to replace some existing text (e.g. <>) if the completion is chosen. The version of LSP our extension supports is so old now that VS Code has dropped support for the old-style newText-with-range completions; they now expect us to use replace instead:

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#insertReplaceEdit

We can't fix this one without upgrading to a recent version of the OmniSharp LSP library.

[tintoy]

Finally figured out a way to handle this correctly!

I've changed the completion provider behaviour so it no-longer extends the selection when trigger characters are typed to trigger completion (this was previously required because otherwise the completion would not correctly replace them).

[tintoy]

@DoctorKrolic can you give this branch a try and see if it works for you?

If so, I'll open a PR for review and publish a new version of the extension.

[DoctorKrolic]

Can you please open a PR, so we can discuss the actual code changes there?