-
Notifications
You must be signed in to change notification settings - Fork 697
Add cohost option to Razor #8189
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple things we need to think about eventually
- Do we want to group Razor server logs wit hthe C# logs? Or should we split them out instead?
- Razor report issue - does the command need to be updated to pull different logs / describe the cohosting option value?
- For VSCode, the experience is using the same language server instance as C# right? Just with language specific handlers and dynamic registration of those features?
- There's a lot of random feature related code in the client side, e.g. https://github.com/dotnet/vscode-csharp/blob/main/src/razor/src/completion/completionHandler.ts - how much of this applies to cohosting?
@@ -430,6 +431,9 @@ class RazorOptionsImpl implements RazorOptions { | |||
public get razorServerPath() { | |||
return readOption<string>('razor.languageServer.directory', ''); | |||
} | |||
public get cohostingEnabled() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this option requires a reload? If so we should add it to the needs reload options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all Razor options currently need a reload, and none of them are configured to prompt for reload :)
Very good questions! This is definitely just a start on cohosting, and dotnet/razor#11742 is only adding semantic tokens support specifically because it allows us to punt on some of these things and do them in separate PRs.
Logs from Razor currently come from here and it would be pretty trivial to use a custom message and put them back in the Razor output window.
Probably.
Yes. No rzls, no
The bits that apply are the ones that maintain Html virtual documents, and forward requests on to them. That's why I started with semantic tokens, because it doesn't. Those endpoints will have to move though, to the roslyn server, and some of our services will either need to be able to take both roslyn and razor servers as parameters, or more likely we can use a common interface. There is more work on the Razor side before that is needed though. |
razorComponentPath = path.dirname(extPath); | ||
}); | ||
|
||
// If cohosting is enabled we get the source generator from the razor component path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't they be the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a real build, yes they'd be the same, but for local development not necessarily. If you're only working on cohosting, then there is no need to specify a local path for rzls at all, since it won't run. Plus this makes more sense in future when the .razor folder will presumably go away entirely.
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.
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.
Goes with dotnet/roslyn#78167 and dotnet/razor#11748 to enable the start of Razor cohosting in VS Code.
Mainly just adding a new option, and using it to turn off a bunch of things, but also a slight tweak to configuration names which I'll call out.