-
Notifications
You must be signed in to change notification settings - Fork 229
Update C# code when Razor components are renamed #12534
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
Update C# code when Razor components are renamed #12534
Conversation
This allows components to be fully supported when an edit originates from Roslyn, even though end tags aren't mapped
a9fae9c to
cec69d7
Compare
| // So that tooling can't observe or influence intermediate state, we don't give them the direct RazorCodeDocument | ||
| // that is our working state. Ideally it shouldn't be mutable at all of course | ||
| var outputDocument = codeDocument.ToHostOutput(); |
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.
This, and the methods, will be superseded by #12533
…amingComponents # Conflicts: # src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/DocumentMapping/AbstractEditMappingService.cs # src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/LspExtensions_WorkspaceEdit.cs # src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Rename/RemoteRenameService.cs
|
Apologies for the noise in the last few commits, managed to cause myself conflicts and test breaks with other PRs I had. I think it should all be good now :) |
...zor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/CohostRoslynRenameTest.cs
Outdated
Show resolved
Hide resolved
| if (node is BaseMarkupStartTagSyntax startTagSyntax && | ||
| startTagSyntax.GetEndTag() is { } endTag) | ||
| { | ||
| // We are changing a start tag, and so we have a matching end tag. We have to translate the edit over there too |
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.
Would this be any easier if we mapped the end tags, or does it not make a difference?
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.
If we mapped end tags, then this little bit of code would be unnecessary, yeah.
BUT we'd either need new code-gen to map too, and it would be "fake" (ie, not relevant at runtime) or we'd have to map the same span twice. Writing this code was easier than doing that and seeing what broke, and sometimes I feel bad if we just map everything to C# and let Roslyn do all the hard work. There is a balance somewhere.
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.
sometimes I feel bad if we just map everything to C# and let Roslyn do all the hard work.
You should fight that feeling. ;-)
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.
Why can't I have a semantic model to play with, like the big kids? 😔
Fixes #9829
Part of #8541
When renaming a component in a Razor file we now call Roslyn so that the component name or namespace is renamed in all C# code in the solution. With these changes, once dotnet/roslyn#81450 merges, we'll do the same when renaming a component type name from a C# file, including updating start and end component tags.