tintoy / msbuild-project-tools-server

Language server for MSBuild intellisense (including PackageReference completion).
MIT License
59 stars 16 forks source link

Completions don't work properly when you just start typing #52

Open DoctorKrolic opened 1 year ago

DoctorKrolic commented 1 year ago

This is better to show than explain: Code_zmLLpQIKAE

For some reason the first letter isn't a part of completion session.

However, if you first manually trigger completions and then start typing, everything is working fine: Code_CKiqla371r

tintoy commented 1 year ago

That looks odd; are you typing < to trigger the completion? I wouldn't expect just typing a property or item name to trigger completion, BTW.

I also vaguely remember a while back the VS Code completion behaviour changing and that tripped us up until we changed our implementation to compensate. I'll see if I can dig up the issue.

tintoy commented 1 year ago

This isn't that issue, but it is some of the history of how we (conceptually) handled completions:

https://github.com/tintoy/msbuild-project-tools-vscode/issues/15

tintoy commented 1 year ago

Also (slightly) related in terms of unintuitive semantics:

https://github.com/tintoy/msbuild-project-tools-vscode/issues/21

tintoy commented 1 year ago

The trigger characters (see lines 100 and 158-164):

https://github.com/tintoy/msbuild-project-tools-server/blob/master/src/LanguageServer.Engine/Handlers/CompletionHandler.cs

And (see line 100):

https://github.com/tintoy/msbuild-project-tools-server/blob/master/src/LanguageServer.Engine/CompletionProviders/PropertyElementCompletionProvider.cs

And (see line 97):

https://github.com/tintoy/msbuild-project-tools-server/blob/master/src/LanguageServer.Engine/CompletionProviders/CompletionProvider.cs

tintoy commented 1 year ago

And here is the original issue, I think:

https://github.com/tintoy/msbuild-project-tools-vscode/issues/67

(this is going back a couple of years, though)

tintoy commented 1 year ago

BTW, logging is quite extensive if you turn on verbose logging; might help work out where behaviour is deviating from expectations.

DoctorKrolic commented 1 year ago

That looks odd; are you typing < to trigger the completion? I wouldn't expect just typing a property or item name to trigger completion, BTW.

Yeah, but since this has been in release versions from probably the very beginning, it would not be a pleasant user experience, if we remove this way of using completions entirely. So I guess this need a fix now.

Also triggering completions via < just doesn't work at all: Code_mRT2F4ZBG5

As you can see, the only things that work if I type < are built-in lexical-based completions from VS code, that just suggest you words you've recently typed. Also I should clarify, that all these gifs are from latest release version, not the dev build. I am not at my work station for about 2 weeks, so don't have access to my usual dev environment for now. But I do remember that I faced this behaviour several times when testing changes for my other PRs, so all this should apply to latest builds as well

tintoy commented 1 year ago

Hmm, I wonder if it's an interaction between the language server's completion behaviour and how your general completion behaviour is configured in your local VSCode? Or it might just be that we have different understandings of how completions are supposed to work. I use the extension fairly frequently and I've never had issues with completions using <, so I suspect it may be one of those 2 but I'll test it out later this morning and see what I can see.

tintoy commented 1 year ago

Ok, so I think I can see what's happened here.

image

This feature did not exist when the language server was written so it does not take into account that there may not be a trigger character. The problem is that a completion triggered by regular typing (i.e. quick-suggestions: other) does not seem to have any sort of differentiator (in the LSP message payload from VS Code to the language service) compared to a completion triggered explicitly (e.g. Ctrl-Space). But in only one of those cases should the character at the insertion point be replaced.

I'll have to have a think about how we might be able to handle this.

tintoy commented 1 year ago

Also possibly related:

https://github.com/microsoft/vscode/issues/185286