Skip to content

Conversation

@dibarbet
Copy link
Member

@dibarbet dibarbet commented Sep 1, 2021

This PR does a few things

  1. Moves the ILanguageClient implementation down to EditorFeatures
  2. Removes the VisualStudioWorkspace import from ILanguageClient implementations in favor of a service that retrieves the HostWorkspaceServices (allows VSMac to provide their own set). Resolved by More options refactoring #56089 to use the global options service in all cases where we previously accessed the workspace.
  3. To resolve a circular dependency where the LSP layer depends on Editor features (for things like VS icons, inline rename, etc, see Remove dependency on Editor and EditorFeatures from Microsoft.CodeAnalysis.LanguageServer.Protocol project #55142), I moved any LSP handler implementation that required EditorFeatures into the EditorFeatures project itself. This makes the LSP layer totally clean and clearly indicates which handlers need to be fixed and moved to the LSP project.

TODO verify on helix

@ghost ghost added the Area-IDE label Sep 1, 2021
@tmat
Copy link
Member

tmat commented Sep 1, 2021

internal interface IDiagnosticModeService : IWorkspaceService

I'm deleting dependency on workspace here #56052


Refers to: src/Features/Core/Portable/Diagnostics/IDiagnosticModeService.cs:14 in 85406c0. [](commit_id = 85406c0, deletion_comment = False)

@dibarbet dibarbet force-pushed the lsp_editor_features branch from 85406c0 to 8f1b62c Compare September 7, 2021 22:35
@dibarbet dibarbet marked this pull request as ready for review September 7, 2021 22:55
@dibarbet dibarbet requested a review from a team as a code owner September 7, 2021 22:55
@dibarbet dibarbet force-pushed the lsp_editor_features branch from 571b3d2 to c9b2877 Compare September 8, 2021 01:03
@dibarbet dibarbet changed the base branch from main to release/dev17.1-preview1 September 8, 2021 18:56
@dibarbet dibarbet force-pushed the lsp_editor_features branch from 5380c11 to 9668b4b Compare September 8, 2021 19:33
Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

:shipit: but a few requests for TODO comments or equivalent. There's also some types that got moved to EditorFeatures that still have "VisualStudio" prefixes for no clear reason and can probably be renamed.

@dibarbet dibarbet force-pushed the lsp_editor_features branch from e800069 to e519d85 Compare September 9, 2021 17:54
@dibarbet dibarbet enabled auto-merge September 10, 2021 00:32
@dibarbet dibarbet merged commit c60c9cc into dotnet:release/dev17.1-preview1 Sep 10, 2021
@dibarbet dibarbet deleted the lsp_editor_features branch September 10, 2021 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants