Skip to content

Conversation

@davidwengier
Copy link
Member

This is a highly mechanical PR, but its also big, so commit-at-a-time is probably best. This moves our extension methods that deal with LSP types, Roslyn types, and Razor types, to the Workspaces layer so that cohosting can use them in OOP. And also general cleanliness, because we had them scattered around and I never knew where to put new ones :)

Part of #9519

Purely mechanical, just copied and pasted files in Explorer :)
These were previously a transitive dependency in the language server, via the ContainedLanguage DLL. We already ship them in VS Code and VS.
Everything that needs these types already references LanguageServer and Workspaces
And in a couple of places, the existing ToRange method instead of the project-specific AsRange method which did the same thing :)
@davidwengier davidwengier requested a review from a team as a code owner March 7, 2024 07:02
@davidwengier
Copy link
Member Author

Integration test run is https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=9203177&view=results but it won't show up in GitHub because it's before a rebase, so the commit hash is different.

@davidwengier
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@davidwengier
Copy link
Member Author

Integration tests passed, except the MEF validation test, which is being fixed in #10044

Copy link
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mechanical changes look good. However, I'd prefer that we don't further couple the LanguageServer project to VS, since I'm actively trying to break that dependency because it's already making a mess of the co-hosting effort. Instead, could you move all of the protocol types that were moved to LanguageServer to a "Protocol" folder in Workspaces? That way, we can get rid of the project without coupling further.

@davidwengier
Copy link
Member Author

No argument from me. I will have to move TextDocumentAndVersion down as well, and will adjust some namespaces.

Copy link
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good!

@davidwengier davidwengier merged commit 8a402ba into main Mar 8, 2024
@davidwengier davidwengier deleted the MoveExtensionMethodsDown branch March 8, 2024 00:09
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Mar 8, 2024
@RikkiGibson RikkiGibson modified the milestones: Next, 17.10 P3 Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants