-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Require spaces within line span directives and relax LangVer check #61686
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
Conversation
| WRN_ObsoleteMembersShouldNotBeRequired = 9042, | ||
| ERR_RefReturningPropertiesCannotBeRequired = 9043, | ||
|
|
||
| ERR_LineSpanDirectiveRequiresSpace = 9028, |
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.
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.
Not sure how I got the right number, but in the wrong place... ;-)
| <value>The #line directive end position must be greater than or equal to the start position</value> | ||
| </data> | ||
| <data name="ERR_LineSpanDirectiveRequiresSpace" xml:space="preserve"> | ||
| <value>The #line span directive requires space before the first parenthesis, before the character offset and before the file name</value> |
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.
| EOF(); | ||
| } | ||
|
|
||
| [Fact, WorkItem(61663, "https://github.com/dotnet/roslyn/issues/61663")] |
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.
WorkItem(61663, "#61663")
It feels like the [WorkItem] attribute belongs on the previous test rather than this one. #Closed
| } | ||
|
|
||
| [Fact, WorkItem(61663, "https://github.com/dotnet/roslyn/issues/61663")] | ||
| public void RequireSpace_MissingFilename() |
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.
| } | ||
|
|
||
| [Fact, WorkItem(61663, "https://github.com/dotnet/roslyn/issues/61663")] | ||
| public void RequireSpace_MissingFilename_WithoutCharacterOffset() |
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 (isActive) | ||
| { | ||
| lineKeyword = CheckFeatureAvailability(lineKeyword, MessageID.IDS_FeatureLineSpanDirective); |
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.
|
Can this resource be removed? In reply to: 1146180483 In reply to: 1146180483 Refers to: src/Compilers/CSharp/Portable/CSharpResources.resx:6673 in 78cbece. [](commit_id = 78cbece, deletion_comment = False) |
| For example, this would be valid: `#line(1,2)-(3,4)5"file.cs"`. | ||
|
|
||
| In Visual Studio 17.3, the compiler requires spaces before the first parenthesis, the character | ||
| offset and the file name. |
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.
Consider adding a comma: ", and the file name." #Resolved
|
@dotnet/roslyn-compiler for second review. Thanks |
|
|
||
| ## Required spaces in #line span directives | ||
|
|
||
| ***Introduced in .NET SDK 7.0.100, Visual Studio 2022 version 17.3.*** |
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.
.NET SDK 6.0.400?
|
@dotnet/roslyn-compiler for second review. Thanks |
|
@dotnet/roslyn-compiler for second review. Thanks |
1 similar comment
|
@dotnet/roslyn-compiler for second review. Thanks |
|
@jcouv Can confirm that razor generates valud |
|
Awesome. Thanks @chsienki |
Fixes #61663
Addresses the most urgent scenario in #61094
Corresponding spec update: dotnet/csharplang#6195
FYI @cston @chsienki