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

Don't emit #line default and #line hidden between consecutive using directives #9991

Merged
merged 11 commits into from
Mar 6, 2024

Conversation

davidwengier
Copy link
Contributor

Fixes #9946
Fixes #8671

Roslyn won't offer to generate using directives into hidden regions unless an instance of an ISpanMappingService is provided. In VS Code, and in future in cohosting, this is not the case. The fix I am proposing here is to simply allow the using directive block to not be in a hidden region. Most using directives are already fully mapped anyway (with more coming with #9949) and even for those that aren't mapped, since there is no debuggable code in the using directive block, there seems to me to be no downside to having default mappings. The change ensures that whatever comes after the using directives (attributes, class declaration, something else in future) is correctly hidden.

The only tooling changes here are tests. We could change the property that Roslyn checks because it is now redundant, but turns out that is hardcoded in Roslyn anyway. The tests are a little funny, because they only validate user scenarios that already work today, but interestingly wouldn't have worked in our test code before, as our test code does not have a span mapping service.

NOTE: This will conflict with the above mentioned PR, but not in any interesting ways.

Reviewing commit-at-a-time might make sense, if only to make things easier because of the giant commit to update all of the test baselines.

Copy link
Contributor

@alexgav alexgav left a comment

Choose a reason for hiding this comment

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

:shipit:

@davidwengier
Copy link
Contributor Author

@dotnet/razor-compiler for a second review

@jjonescz jjonescz added the area-compiler Umbrella for all compiler issues label Mar 4, 2024
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

Done review pass. Mostly LGTM, one minor coment.

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
Projects
None yet
5 participants