-
Notifications
You must be signed in to change notification settings - Fork 196
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
Indent @section blocks #6030
Indent @section blocks #6030
Conversation
...est/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting/CodeDirectiveFormattingTest.cs
Show resolved
Hide resolved
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.
Had some questions on how we're visiting
// If this is the direct child of a @section directive then we add a Razor indent to essentially | ||
// set the baseline one level in. | ||
// Note: "Direct child" in a little poetic license, as the structure is actually: | ||
// MarkupElement -> MarkupBlock -> CSharpCodeBlock (¯\_(ツ)_/¯) -> RazorDirectiveBody -> RazorDirective |
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.
lmao @javiercn here's an example of what we have to do in order to make sense of the Razor syntax tree for something as simple as an @section
directive. aka related to https://github.com/dotnet/aspnetcore/issues/34957
src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/FormattingVisitor.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/FormattingVisitor.cs
Outdated
Show resolved
Hide resolved
owner.Parent is MarkupBlockSyntax block && | ||
owner == block.Children[block.Children.Count - 1] && | ||
// MarkupBlock -> CSharpCodeBlock -> RazorDirectiveBody -> RazorDirective | ||
block.Parent.Parent.Parent is RazorDirectiveSyntax directive && |
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.
Lol damn, I would have figured this would have null-ref'd at the top level document for normal HTML but if that were the case our tests would be on the floor. /cc @javiercn for another example of sadness w/ the SyntaxTree
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 was surprised at this too, but all of the formatting code is null-annotated, so I'm trusting the compiler that there are no warnings here. Happy to add some question marks if you don't trust the compiler though :)
Does this fixes dotnet/aspnetcore#6028 ? |
Sadly, no, this wouldn't change that. In general the Razor editor doesn't currently format JavaScript content. |
Fixes #6002