Skip to content

Conversation

@alexandrebodin
Copy link

As we discussed on this PR graphql/graphql-spec#323 (comment)

This looks like this now

screen shot 2017-06-14 at 21 34 45

@alexandrebodin alexandrebodin force-pushed the add-optional-mods-terminal branch from d8d2db8 to 7b1a3d9 Compare June 14, 2017 19:38
}

terminal = value:$(([^ \n"/`] [^ \n"\`,\]\}]*)) {
terminal = value:$(([^ ?\n"/`] [^ ?\n"\`,\]\}]*)) quantifier:('?')? {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's necessary to change here as well. ? as part of a terminal should be allowed. I think it is okay to require quoting the terminal to apply a ? to remove ambiguity.

}

quotedTerminal = '`' value:$(([^`\n] / ('\\`'))+)? closer:'`' {
quotedTerminal = '`' value:$(([^`\n] / ('\\`'))+)? closer:'`' quantifier:('?')? {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For completeness, this should include all of the possible quantifiers. See nonTerminal for an example of that

@leebyron
Copy link
Owner

Could you include a reference to this in the specmd documentation? That sort of serves as its own test.

Copy link
Owner

@leebyron leebyron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tossing this back into your review queue for the addition of docs

@leebyron leebyron closed this in dedafb6 Sep 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants