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

Clean up extension methods in MS.CA.Razor.Workspaces #10670

Merged
merged 47 commits into from
Jul 24, 2024

Conversation

DustinCampbell
Copy link
Member

As I started looking at co-hosting support for Go to Definition, I noticed that our extension methods for working with LSP types had started to grow out of control. This is even more awkward now, since we've got two sets of LSP types in Razor for co-hosting: Microsoft.VisualStudio.LanguageService.Protocol and Roslyn.LanguageService.Protocol.

My commit history starts out helpful, but by the end, the commits are quite a bit bigger as they focus specifically on LSP extensions and updating relevant calling code.

@DustinCampbell DustinCampbell requested a review from a team as a code owner July 24, 2024 00:36
Copy link
Contributor

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

GitHub borked on this PR, so I don't know that I got all of it, but I saw the important parts.

My only feedback is bikeshedding the meaning of words like "Collapsed" and "Empty", and whether we can just remove logging. I've been converting one of the document mapping methods from TextEdit to TextChange, and thats what I've been doing as I've been creating/modifying these sorts of extension methods.

Love the change overall though, and very excited to resolve the conflicts in my current branch 😁

Copy link
Contributor

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

Love it

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.

3 participants