Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move DocumentMappingService down to Workspaces #10068

Merged
merged 7 commits into from
Mar 12, 2024
Merged

Move DocumentMappingService down to Workspaces #10068

merged 7 commits into from
Mar 12, 2024

Conversation

davidwengier
Copy link
Contributor

@davidwengier davidwengier commented Mar 12, 2024 •
edited
Loading

Part of #9519

In order to use the document mapping service in OOP we need it to be in Workspaces. In order to move the document mapping service to Workspaces we also need a couple of DocumentContext things down there too.

The changes in this PR are entirely mechanical, but GitHub doesn't know that AbstractRazorDocumentMappingService is a move of RazorDocumentMappingService, because the latter still exists. In future there will be another implementation of it in Remote.Razor too.

@davidwengier davidwengier requested a review from a team as a code owner March 12, 2024 11:13
DustinCampbell
DustinCampbell reviewed Mar 12, 2024
View reviewed changes
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IDocumentContextFactory.cs Outdated
@@ -4,7 +4,7 @@
using System;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.AspNetCore.Razor.LanguageServer;
namespace Microsoft.CodeAnalysis.Razor.Workspaces.ProjectSystem;
Copy link
Member

@DustinCampbell DustinCampbell Mar 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Consider changing this namespace to Microsoft.CodeAnalysis.Razor.ProjectSystem to match other types in this folder. I've been doing that with all of them as I've made changes. Same feedback for the other types moved to this namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like I also violated that on a previous PR so I'll fix them all up. I'll also change Microsoft.CodeAnalysis.Razor.Workspaces.DocumentMapping to Microsoft.CodeAnalysis.Razor.DocumentMapping to match, and update everything. At least there is an easy code action for these sorts of updates :)

Copy link
Member

Choose a reason for hiding this comment

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

At least there is an easy code action for these sorts of updates

For sure! Move to Namespace is a huge time saver. I feel less bad recommending changes like these knowing that's there.

@davidwengier davidwengier enabled auto-merge March 12, 2024 22:48
DustinCampbell
DustinCampbell reviewed Mar 12, 2024
View reviewed changes
...c/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/Razor/AddUsingsCodeActionResolver.cs
@@ -17,6 +17,7 @@
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Newtonsoft.Json.Linq;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
Copy link
Member

Choose a reason for hiding this comment

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

nit: sort usings

@davidwengier davidwengier merged commit be0a190 into dotnet:main Mar 12, 2024
12 checks passed
@davidwengier davidwengier deleted the DocumentMappingServiceDown branch March 12, 2024 22:58
DustinCampbell
DustinCampbell reviewed Mar 12, 2024
View reviewed changes
...t.AspNetCore.Razor.LanguageServer/CodeActions/Razor/ExtractToCodeBehindCodeActionResolver.cs
@@ -20,6 +20,7 @@
using Microsoft.CodeAnalysis.Razor;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Newtonsoft.Json.Linq;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
Copy link
Member

Choose a reason for hiding this comment

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

nit: sort usings

@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Mar 12, 2024
DustinCampbell
DustinCampbell reviewed Mar 12, 2024
View reviewed changes
...re.Razor.LanguageServer/DocumentPresentation/AbstractTextDocumentPresentationEndpointBase.cs
@@ -14,6 +14,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Microsoft.CodeAnalysis.Razor.Workspaces.Protocol;
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
Copy link
Member

Choose a reason for hiding this comment

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

nit: sort usings

DustinCampbell
DustinCampbell reviewed Mar 12, 2024
View reviewed changes
.../src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/FormattingContentValidationPass.cs
@@ -11,6 +11,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Microsoft.CodeAnalysis.Razor.Workspaces.Protocol;
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
Copy link
Member

Choose a reason for hiding this comment

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

nit: sort usings

DustinCampbell
DustinCampbell reviewed Mar 12, 2024
View reviewed changes
...c/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/FormattingDiagnosticValidationPass.cs
@@ -13,6 +13,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Microsoft.CodeAnalysis.Razor.Workspaces.Protocol;
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
Copy link
Member

Choose a reason for hiding this comment

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

nit: sort usings

DustinCampbell
DustinCampbell reviewed Mar 12, 2024
View reviewed changes
...r/src/Microsoft.AspNetCore.Razor.LanguageServer/InlineCompletion/InlineCompletionEndPoint.cs
@@ -19,6 +19,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Microsoft.CodeAnalysis.Razor.Workspaces.Protocol;
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
Copy link
Member

Choose a reason for hiding this comment

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

nit: sort usings

DustinCampbell
DustinCampbell reviewed Mar 12, 2024
View reviewed changes
...rosoft.VisualStudio.LanguageServerClient.Razor/Cohost/RazorCohostRequestContextExtensions.cs
using Microsoft.AspNetCore.Razor.LanguageServer.Cohost;
using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
using Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
Copy link
Member

Choose a reason for hiding this comment

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

nit: sort usings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These usings are already sorted 😛

For the rest, I opened #10080

@RikkiGibson RikkiGibson modified the milestones: Next, 17.10 P3 Mar 25, 2024
@dotnet-bot dotnet-bot mentioned this pull request Apr 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DustinCampbell DustinCampbell DustinCampbell approved these changes

@ryzngard ryzngard ryzngard approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
17.10 P3
Development

Successfully merging this pull request may close these issues.
4 participants
@davidwengier @DustinCampbell @ryzngard @RikkiGibson