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.

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

Add feature switch to use our new Razor LSP editor. #1481

Merged
merged 3 commits into from
Jan 14, 2020

Conversation

NTaylorMullen
Copy link
Contributor

  • Added a new Razor LSP content type to identify our new Razor editor experience.
  • Given that there are already content type associations for the .cshtml/.razor file paths I had to build a RazorEditorFactory that takes precedence over WTE's editor factories and if our feature flags are true to change the content type on created editors.
  • Added a README.md to the LanguageServerClient project to indicate how to use the new Razor LSP editor (no functionality currently).
  • For a feature flag I took a two pronged approach of allowing environment variables or VS' first class feature flag support. We haven't yet made the changes to Visual Studio necessary to surface our feature flag as a properly settable feature flag. Once that is done our feature flag will be toggable via the Enable Preview Features dialog.
  • Didn't feel there was a ton of value in adding tests for our EditorFactory since it acts on things like environment variables and internal VS feature flag functionalities.

dotnet/aspnetcore#17787

- Added a new Razor LSP content type to identify our new Razor editor experience.
- Given that there are already content type associations for the .cshtml/.razor file paths I had to build a RazorEditorFactory that takes precedence over WTE's editor factories and if our feature flags are `true` to change the content type on created editors.
- Added a `README.md` to the LanguageServerClient project to indicate how to use the new Razor LSP editor (no functionality currently).
- For a feature flag I took a two pronged approach of allowing environment variables or VS' first class feature flag support. We haven't yet made the changes to Visual Studio necessary to surface our feature flag as a properly settable feature flag. Once that is done our feature flag will be toggable via the `Enable Preview Features` dialog.
- Didn't feel there was a ton of value in adding tests for our EditorFactory since it acts on things like environment variables and internal VS feature flag functionalities.

dotnet/aspnetcore#17787
Copy link
Contributor

@ryanbrandenburg ryanbrandenburg left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I don't have any background in VS extensions so I might be missing stuff.

@@ -0,0 +1,12 @@
# Using the Razor LSP Editor
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor

@ajaybhargavb ajaybhargavb left a comment

Choose a reason for hiding this comment

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

As always, I highly appreciate the detailed description at the top 👍

- Included the LanguageServer core dlls as part of the VSIX install. To do this I had to expose the language server's output path via a target and then dynamically include its outputs in the VSIX output.
- Given how VSIX's are built I could not privately reference our language server to ensure that it's built when the VSIX is built. To work around this limitation I added a `_BuildLanguageServer` target that calls MSBuild directly on the language server as part of build.
- For now I'm dynamically booting the currently built language server that's deployed with the extension. I haven't given a lot of thought to what flavor of OS (x64 vs. x86) the server can run on; or if we need to ensure dotnet.exe is available.
- Found that the VS LSP client and the O# language server library that we use don't inter-operate as we'd expect. The server ends up telling the client that it doesn't care about text document change events (we do). I've created a separate PR in O# to fix this: OmniSharp/csharp-language-server-protocol#199
- Add OmniSharp 3rd party library to signing information for VSIX insertions.


dotnet/aspnetcore#17789
@NTaylorMullen NTaylorMullen merged commit 8956b56 into nimullen/17746 Jan 14, 2020
@NTaylorMullen NTaylorMullen deleted the nimullen/17787 branch January 14, 2020 22:04
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