Add support for leading vertical bar in union types #907
Conversation
| @@ -83,6 +83,8 @@ union Feed = Story | Article | Advert | |||
|
|
|||
| union AnnotatedUnion @onUnion = A | B | |||
|
|
|||
| union AnnotatedUnionTwo @onUnion = A | B | |||
There was a problem hiding this comment.
Did you intend to have | A | B?
There was a problem hiding this comment.
Nope the printer will only print without it or with it I think it's better to leave it out and add it with prettier for ex
| }; | ||
| expect(printJson(doc)).to.equal(printJson(expected)); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Can you please also add test cases for | Wo | Rld |, union Hello = Wo | Rld |, Wo || Rld and union Hello = |?
|
@robzhu I added severals news tests |
|
@alexandrebodin, thank you for the high quality PR and fast turnaround! |
| @@ -943,6 +943,9 @@ function parseUnionTypeDefinition(lexer: Lexer<*>): UnionTypeDefinitionNode { | |||
| */ | |||
| function parseUnionMembers(lexer: Lexer<*>): Array<NamedTypeNode> { | |||
| const members = []; | |||
| if (peek(lexer, TokenKind.PIPE)) { | |||
| skip(lexer, TokenKind.PIPE); | |||
| } | |||
There was a problem hiding this comment.
The wrapping peek is unnecessary. skip is effectively a call to peek.
| @@ -943,6 +943,9 @@ function parseUnionTypeDefinition(lexer: Lexer<*>): UnionTypeDefinitionNode { | |||
| */ | |||
| function parseUnionMembers(lexer: Lexer<*>): Array<NamedTypeNode> { | |||
There was a problem hiding this comment.
Every method in the parser has parse language atop it. It would be nice to include this here as well
|
This is not the only place in the IDL where bar-separated tokens exists. Should we remain consistent and update the others as well? Or would that not be fitting? |
|
@leebyron I could update the other places if you can point them to me. |
|
Let me take a quick look |
This adds a bit of cleanup after #907 for post-commit review: * Removes unnecessary `peek()` * Adds leading pipe support to directive locations * Removes some redundant tests * Orders tests so similar tests remain together
This adds a bit of cleanup after #907 for post-commit review: * Removes unnecessary `peek()` * Adds leading pipe support to directive locations * Removes some redundant tests * Orders tests so similar tests remain together
Implementation of the leading vertical bar in the language parser.
Issue #897