Open DoctorKrolic opened 1 year ago
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.
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).
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 🙂
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>"
}
}
]
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:
We can't fix this one without upgrading to a recent version of the OmniSharp LSP library.
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).
@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.
Can you please open a PR, so we can discuss the actual code changes there?
I managed to get a consistent repro of https://github.com/tintoy/msbuild-project-tools-server/issues/52#issuecomment-1627449845
.csproj
with this content is the only MSBuild-related file in the project