- Making the resolve completion use document for cohosting
- Serializing TextDocument property into Data member of completion items so that Roslyn will forward the request to us
- Basic sanity test (shows we are getting called now)
Preparation for ongoing work to hook up the Roslyn tokenizer and
https://github.com/dotnet/razor/issues/11182 I suppose.
There were three places that `UpdateProjectWorkspaceState` was called:
1. In `RazorProjectService`, just before calling
`UpdateProjectConfiguration`
2. In `ProjectWorkspaceStateGenerator`, where we will need to add a call
to `UpdateProjectConfiguration` in future, to wire up the tokenizer
3. In our LiveShare bits, in response to events from the above.
Previous attempts to plumb through more things for `RazorConfiguration`
resulted in RPS failures, that appeared to be simply more compilations
of closed files. This makes sense because we were adding another update,
which would have triggered another set of `ProjectChanged` events. I
thought it would make more sense to combine these two updates together,
so no matter which part of the project was being updated, there could be
a single `ProjectChanged` notification. This is that.
* Move completion resolve code to common layer
* Undo move of IProjectSnapshotManagerExtensions and MiscFilesHostProject per PR feedback
* Undo unnecessary changes in tests
* Refactor the code to remove usage of IProjectSnapshotManager
I couldn't originally test some code actions because they used the file
system, so I intended this PR to fix that by abstracting the file system
away. It is that, but I also found some more tests that we lacked
coverage for in cohosting, and since I find the cohosting tests to be
just so wonderful (not biased at all, obviously) I thought I'd be a bit
extra.
I deliberately didn't try to fix any of the code actions, but I think
they could probably all do with some nicer whitespace handling. Managed
to avoid the self-nerd-snipe this time though.
For a long while IDocumentSnapshot's FileKind, FilePath, and TargetPath properties have all been annotated as nullable. It turns out that the only reason for this was ImportDocumentSnapshot, which has been removed. So, we can now allow these properties to be correctly annotated as non-nullable,
This change removes several parameters from DocumentState.GenerateCodeDocumentAsync(...) and computes them internally. So, there's no longer a need to expose GetImportsAsync for GenerateCodeDocumetnAsync callers.
The comment explaining why this DocumentState method is internal seems to be out of date. It's currently only used within DocumentState, so this change removes the comment and makes it private.
This one might be a _little_ controversial in terms of how I wired the
`LanguageServerFeatureOptions` up through the `ProjectSnapshot`, but
it's somewhat similar to how it works in cohosting, with
`RemoteProjectSnapshot -> RemoteSolutionSnapshot ->
SolutionSnapshotManager -> LanguageServerFeatureOptions`.
Aside from that aspect of the specific implementation, which you're more
than welcome to critique and suggest an alternative to, the fundamental
change here is removing `LanguageServerFlags` and using
`LanguageServerFeatureOptions` directly. It seems that
`LanguageServerFlags` was intended to encapsulate the feature options
for the compiler, but since that time they are never actually used by
the compiler, and just ended up in a weird spot. They were "flags for
the language server", but were only ever initialized in VS. We we
essentially ended up with this unnecessary middle-man, that didn't
always exist. The `LanguageServerFeatureFlags` on the other hand are
always set in VS, VS Code, etc.
TL;DR: Now that all of the code for deciding whether to use runtime
compilation is entirely on the tooling side, it just makes sense to use
the tooling side options class directly.
These flags were explicitly not for the compiler, being not serialized, and hence don't really belong on RazorConfiguration.
They also were only ever set in the VS layer
Turns out the boolean parameter passed in here is read in exactly one
spot in our code, and that line of code is never hit in these tests
because they use mocks.
Yes, this is the equivalent of removing a `Thread.Sleep` that was put in
previously, but I'm taking it regardless :)
Alex Gavrilov (DEV PROD)
David Wengier
dotnet bot
dotnet-maestro[bot]
Bill Wagner
Dustin Campbell
Pierson Lee (PIE)
Andrew Hall
Jan Jones