Skip to content

Support cohosting in VS Code #11748

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.

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

Merged
merged 26 commits into from
Apr 24, 2025
Merged

Support cohosting in VS Code #11748

merged 26 commits into from
Apr 24, 2025

Conversation

davidwengier
Copy link
Member

@davidwengier davidwengier commented Apr 17, 2025

Goes with dotnet/roslyn#78167 and dotnet/vscode-csharp#8189 to expose cohosting to VS Code.

Needs dotnet/roslyn#78167 merged and referenced before it will build.

Part of #11759

@davidwengier davidwengier force-pushed the dev/dawengie/CohostVSCode branch from 099becb to 063901b Compare April 22, 2025 01:22
davidwengier added a commit to dotnet/roslyn that referenced this pull request Apr 22, 2025
Goes with dotnet/vscode-csharp#8189 and
dotnet/razor#11748 to allow Razor cohosting in
VS Code, but also fixes some VSIX issues that broke cohosting in VS too.
Reviewing commit-at-a-time is probably recommended.
@davidwengier
Copy link
Member Author

@dotnet/razor-tooling PTAL

@davidwengier davidwengier removed the request for review from a team April 23, 2025 04:25

namespace Microsoft.VisualStudioCode.RazorExtension.Services;

// TODO:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: file a follow up

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a bullet point under 9 on #9519 :)

@@ -1,8 +1,13 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using System;
Copy link
Contributor

Choose a reason for hiding this comment

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

change to implicit usings?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know how to do emojies on mac so just imagine a questionmark emoji so that this is inquisitive and not a suggestion or aggrevation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can't really. The 2nd commit in this PR explicitly turns off implicit usings in this project, because now that it references the shared project, it has to be in lock step with the other thing that references it. ie, could do this, but have to do it in MS.VS.LanguageServices.Razor too. So if we feel strongly enough, not this PR :)

Copy link
Contributor

Choose a reason for hiding this comment

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

No strong opinion, just noted a change that looks weird and was hoping turning them off was why

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, and press the globe looking button in the bottom left of the keyboard :)

using Microsoft.VisualStudio.Threading;

namespace Microsoft.VisualStudio.Razor.Settings;

[Export(typeof(IClientSettingsReader))]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused that this actually works. I see that IClientSettingsReader has GetClientSettings() which this also has. Is that enough for MEF? It doesn't actually have to have the IClientSettingsReader in it's type ancestry?

Copy link
Member Author

Choose a reason for hiding this comment

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

I made IClientSettingsManager implement IClientSettingsReader just so I didn't have to go and update everywhere that used it, so yes, it is in its type ancestry.

@davidwengier davidwengier enabled auto-merge April 23, 2025 21:11
@davidwengier davidwengier merged commit f1568ee into main Apr 24, 2025
11 checks passed
@davidwengier davidwengier deleted the dev/dawengie/CohostVSCode branch April 24, 2025 00:29
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Apr 24, 2025
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.

2 participants