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

Update whitespace parsing #10380

Closed
wants to merge 4 commits into from

Conversation

chsienki
Copy link
Member

Today we parse the razor syntax tree differently if we're in design time or runtime mode. Specifically, we assign trailing whitespace to markup in design time, and to csharp in runtime. This was originally implemented when the Razor editor used projection buffers, to ensure that users got the IntelliSense they were expecting.

As part of fuse using runtime documents, we need to update the way we assign whitespace for run time too. This PR removes the distinction and prefers the design time handling. This is arguably more correct, as whitespace, while mostly agnostic in HTML, does matter in certain cases (such as <pre>), so makes sense that we should represent it in the HTML document.

The vast majority of the baseline changes are because the newline at the end of a @using statement is no longer part of the using declaration itself, and basically every test has a using declaration.

Fixes #10358

@chsienki chsienki added area-compiler Umbrella for all compiler issues New Feature: Fuse labels May 17, 2024
@chsienki chsienki requested review from a team as code owners May 17, 2024 17:53
@davidwengier
Copy link
Member

the newline at the end of a @using statement is no longer part of the using declaration itself

Sounds like maybe this fixes #10375 then?

@jjonescz jjonescz force-pushed the fuse/fix_whitespace_assignment branch from 252d807 to a76bbd7 Compare May 27, 2024 11:55
@@ -24,6 +24,7 @@ public class TestFiles_IntegrationTests_CodeGenerationIntegrationTest_AddTagHelp
#pragma warning disable 1998
public async override global::System.Threading.Tasks.Task ExecuteAsync()
{
WriteLiteral("\r\n");
Copy link
Member

@jjonescz jjonescz May 27, 2024

Choose a reason for hiding this comment

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

More newlines are emitted now where they previously weren't. I think that's quite serious braking change - it can result in rendering of new spaces in web pages.

Copy link
Member

@jjonescz jjonescz May 27, 2024

Choose a reason for hiding this comment

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

At least it breaks aspnetcore tests (see internal run here), they would need to be updated.

@jjonescz jjonescz marked this pull request as ready for review May 27, 2024 12:21
{
Assert.Equal("Shared/Component1.razor", mappedSemicolon.Path);
Assert.Throws<ArgumentOutOfRangeException>(() => originalText.Lines.GetTextSpan(mappedSemicolon.Span));
}
Copy link
Member

@jjonescz jjonescz May 27, 2024

Choose a reason for hiding this comment

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

the newline at the end of a @using statement is no longer part of the using declaration itself

Sounds like maybe this fixes #10375 then?

Added this test to check. It doesn't look like this PR fixes the issue. I think it's because the #line mapping simply maps the whole using directive, it cannot map just a part of it per its spec.

@jjonescz jjonescz force-pushed the fuse/fix_whitespace_assignment branch from 6b81588 to e82bac2 Compare May 27, 2024 12:44
@chsienki
Copy link
Member Author

Closing in favor of #10459

@chsienki chsienki closed this Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-compiler Umbrella for all compiler issues New Feature: Fuse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Whitespace changes behavior of folding @code ranges in fuse
3 participants