Skip to content

Add syntax classification for nullable directives#31579

Merged
jasonmalinowski merged 2 commits intodotnet:masterfrom
jhinder:nullable-directive-classifier
Dec 10, 2018
Merged

Add syntax classification for nullable directives#31579
jasonmalinowski merged 2 commits intodotnet:masterfrom
jhinder:nullable-directive-classifier

Conversation

@jhinder
Copy link
Contributor

@jhinder jhinder commented Dec 6, 2018

This PR adds syntax classification support for #nullable [enable|disable] directives.

Left: 16.0 Preview 1, right: VS 15.9 with this PR
nullable-directives

@jhinder jhinder requested a review from a team as a code owner December 6, 2018 05:54
@vatsalyaagrawal vatsalyaagrawal added Area-IDE Feature - Nullable Reference Types Nullable Reference Types Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Dec 6, 2018
@vatsalyaagrawal vatsalyaagrawal added this to the 16.0.P2 milestone Dec 6, 2018
@jasonmalinowski
Copy link
Member

Tagging @jinujoseph for M2 approval.

@jasonmalinowski
Copy link
Member

@jcouv @jaredpar Any concerns with this? Not sure if you had some bigger work coming.

@jasonmalinowski jasonmalinowski self-assigned this Dec 10, 2018
@jcouv
Copy link
Member

jcouv commented Dec 10, 2018

Nice. This change is desirable.
We are planning to add #nullable restore in preview 2. But it will still be a NullableDirectiveTrivia, so I don't think we'll need to revisit this code.

Thanks!

@jasonmalinowski
Copy link
Member

Great. Thanks @jhinder!

@jasonmalinowski jasonmalinowski merged commit ac7a0d0 into dotnet:master Dec 10, 2018
@jhinder jhinder deleted the nullable-directive-classifier branch December 11, 2018 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved to merge Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. Feature - Nullable Reference Types Nullable Reference Types

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants