Skip to content

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 :)

// 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 :)


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 and others added 3 commits April 24, 2025 07:14
Co-authored-by: Andrew Hall <andrha@microsoft.com>
@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
@RikkiGibson RikkiGibson modified the milestones: Next, 18.0 P1 Aug 20, 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.

4 participants