This repository has been archived by the owner. It is now read-only.

Support tuples for type directive tokens #1613

Merged
merged 1 commit into from Aug 14, 2017

Conversation

Projects
None yet
4 participants
@ajaybhargavb
Copy link
Member

ajaybhargavb commented Aug 12, 2017

#1592

@NTaylorMullen @rynowak

I've also verified this change in design time in VS.

if (Optional(CSharpSymbolType.LeftParenthesis))
{
while (!Optional(CSharpSymbolType.RightParenthesis) && !EndOfFile)
{

This comment has been minimized.

@NTaylorMullen

NTaylorMullen Aug 12, 2017

Member

( string val, int val2) is a valid tuple. Therefore, need Optional(CSharpSymbolType.WhiteSpace) here (means you can
also remove the trailing one at the end of this while.

This comment has been minimized.

@ajaybhargavb
@@ -811,6 +811,30 @@ public void DirectiveDescriptor_AllowsNullableTypes(string expectedType)
Factory.Span(SpanKindInternal.Code, expectedType, markup: false).AsDirectiveToken(descriptor.Tokens[0])));
}

[Theory]
[InlineData("(int aa, string bb)?")]

This comment has been minimized.

@NTaylorMullen

NTaylorMullen Aug 12, 2017

Member

Add a spaced out tuple case:

(    int     aa     ,    string     bb    )

This comment has been minimized.

@ajaybhargavb
@ajaybhargavb

This comment has been minimized.

Copy link
Member

ajaybhargavb commented Aug 12, 2017

🆙 📅

Optional(CSharpSymbolType.WhiteSpace);
Optional(CSharpSymbolType.Comma);
}

This comment has been minimized.

@NTaylorMullen

NTaylorMullen Aug 12, 2017

Member

Oops, missed this. Can have optional whitespace before the questionmark too. That's a little more tricky though. You only want to consume the whitespace if the next symbol is a question mark. Should do the same thing with our other nullable support below.

This comment has been minimized.

@ajaybhargavb

ajaybhargavb Aug 12, 2017

Member

Huh?! Didn't realize that is valid.

This comment has been minimized.

@ajaybhargavb

ajaybhargavb Aug 12, 2017

Member

Looks like even int ?aa = null; is valid.

This comment has been minimized.

@rynowak

rynowak Aug 12, 2017

Member

Yep, this is why languages don't usually treat whitespace as a token.

@NTaylorMullen
Copy link
Member

NTaylorMullen left a comment

Once you've added the question mark whitespace bit then feel free to merge.

@ajaybhargavb ajaybhargavb force-pushed the ajbaaska/tuples branch from ce9294f to a80c196 Aug 14, 2017

@ajaybhargavb ajaybhargavb force-pushed the ajbaaska/tuples branch from a80c196 to 8d2a9e5 Aug 14, 2017

@ajaybhargavb ajaybhargavb merged commit 8d2a9e5 into dev Aug 14, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ajaybhargavb ajaybhargavb deleted the ajbaaska/tuples branch Aug 14, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.