-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Classify pattern variables #59735
Classify pattern variables #59735
Conversation
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.
@alrz Can you merge the latest main and run this test locally:
roslyn/src/EditorFeatures/CSharpTest/Classification/SyntacticClassifierTests.cs
Lines 5109 to 5125 in 220b3eb
[Theory] | |
[CombinatorialData] | |
[WorkItem(18956, "https://github.com/dotnet/roslyn/issues/18956")] | |
public async Task TestVarPattern(TestHost testHost) | |
{ | |
await TestInMethodAsync(@" | |
_ = 1 is var x; | |
", | |
testHost, | |
Identifier("_"), | |
Operators.Equals, | |
Number("1"), | |
Keyword("is"), | |
Keyword("var"), | |
Identifier("x"), | |
Punctuation.Semicolon); | |
} |
I think it will fail (Identifier("x")
should be Local("x")
), then you need to update it.
This test didn't exist when you first opened the PR.
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 don't see an issue here but @CyrusNajmabadi should double check
Thanks for the pointer. I'll take a look. |
Failures seem to be unrelated. @CyrusNajmabadi |
ping @CyrusNajmabadi (looks like one check is still blocking) |
We're having issues with ci. Likely won't be able to merge until Monday unless someone can fix that. |
@CyrusNajmabadi |
# Conflicts: # src/EditorFeatures/CSharpTest/Classification/TotalClassifierTests.cs
This comment was marked as off-topic.
This comment was marked as off-topic.
It's been quite a few Mondays since the last time, still not green. :( |
@alrz The current failure is @CyrusNajmabadi Can you merge please? |
Thanks! |
Fixes #59484