-
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
Recommenders for data
and with
#43971
Conversation
What is hte 'data' keyword? |
It's the current keyword for records: https://github.com/dotnet/csharplang/blob/master/proposals/records-wip.md |
@CyrusNajmabadi for review. Thanks |
@@ -5061,7 +5061,7 @@ class C | |||
</Document>, | |||
showCompletionInArgumentLists:=showCompletionInArgumentLists) | |||
|
|||
state.SendTypeChars("w") | |||
state.SendTypeChars("z") |
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.
intresting :D
{ | ||
return | ||
context.IsGlobalStatementContext || | ||
context.TargetToken.IsUsingKeywordInUsingDirective() || |
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.
why this line?
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.
Good catch, I'll remove. This is a bad carry-over from the static
recommender I started from.
return CheckPreviousModifiers(context); | ||
} | ||
|
||
return false; |
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.
do we have test with things like ref data partial
struct (and permutations)?
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.
question still applies
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.
Sorry, I thought I addressed that comment. We have a test for partial $$
and one for ref $$
. I don't think that what follows the $$ matters, but I can add such tests if you think it's useful.
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.
data
is a contextual keyword only directly before class
at the moment, so if anything here relies on data
being a contextual keyword, that won't work.
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.
@agocke I've already accounted for that, since you won't have typed class
at the time you want to type data
. We had a similar problem with partial
or ref
I think.
validModifiers: SyntaxKindSet.AllTypeModifiers, | ||
validTypeDeclarations: SyntaxKindSet.ClassInterfaceStructTypeDeclarations, | ||
canBePartial: true, | ||
cancellationToken: cancellationToken)) |
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.
validModifiers: SyntaxKindSet.AllTypeModifiers, | |
validTypeDeclarations: SyntaxKindSet.ClassInterfaceStructTypeDeclarations, | |
canBePartial: true, | |
cancellationToken: cancellationToken)) | |
validModifiers: SyntaxKindSet.AllTypeModifiers, | |
validTypeDeclarations: SyntaxKindSet.ClassInterfaceStructTypeDeclarations, | |
canBePartial: true, | |
cancellationToken: cancellationToken)) |
|
||
#if !CODE_STYLE | ||
private const uint DotDotTokenValueAssertion = -(DataKeyword - SyntaxKind.DataKeyword); | ||
#endif |
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.
what's this?
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.
This is explained below. I forgot to rename the constant though. Will fix
{ | ||
var precedingModifiers = context.PrecedingModifiers; | ||
return !precedingModifiers.Contains(SyntaxKind.DataKeyword); | ||
} |
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.
this entire method is tiny. i would just inline the entire thing.
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 tried to keep a similar structure to others. I'll adjust in a follow-up PR (want to get this in for demo bits)
The
data
recommender is similar to that forpublic
.The
with
recommender is like the one foras
.