Added grouping to ambiguous alternatives, allow leading newlines inside arrays and spaces after newlines before array elements and newlines and spaces before comments in arrays #378
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RFC 4234 Section 3.5 advises the use of grouping notation rather than "bare" alternation when alternatives consist of multiple rules or literals, e.g. "( int float ) / ( bool char )" instead of "int float / bool char". I might've gotten the grouping wrong, which just serves to illustrate the importance of using grouping notation.
The README.md has this at the end of the first TOML example: ```toml hosts = [ "alpha", "omega" ] ``` And later says: >...arrays also ignore newlines.... But the ABNF doesn't allow for a newline to precede the first element. I wasn't sure if the grammar should allow more than one newline preceding the first element, but given that an arbitrary number of whitespaces are allowed and the readme says that newlines are ignored in general I figured allowing an arbitrary number of newlines best matches your intent. It also won't break anyone that happens to have multiple leading newlines in their arrays.
The array-values production already allows for an arbitrary amount of newlines before the closing brace. It doesn't, however, allow any whitespace after the newlines that don't come just before the closing brace so: ```toml [5, 6 \t\t] ``` is valid, but ```toml [5, 6 \t\t ] ``` is invalid. Probably not a big deal.
|
I guess this automatically added my new commits to this pull request. The documentation states that newlines are ignored in arrays and gives examples of arrays with newlines before the first element, however the ABNF doesn't allow for newlines before the first element. I've added a production for multiple whitespaces or newlines, "ws-newline", and changed the "ws" after the array opening brace to "ws-newline". |
joelself
changed the title
Back to the README example:
```toml
hosts = [
"alpha",
"omega"
]
```
The ABNF allows spaces after an element, but before any newlines, the examples in the README have spaces before elements after a newline which isn't allowed in the grammar. I've added a new production "ws-newlines" which is just like "ws-newline" except it requires at least one newline like the production it replaces("newlines"). With "ws-newlines" it makes it legal to to put whitespace before an element after a newline.
|
Added another new production to allow whitespace before an array element after a newline. The examples in the README do this, but it isn't allowed by the grammar. |
joelself
changed the title
joelself
changed the title
Going over your hard example I found I couldn't parse this:
```toml
multi_line_array = [
"]",
# ] Oh yes I did
]
```
It turns out that the grammar doesn't allow newlines and whitespaces after the array-sep. I've modified the production again to accommodate newlines and whitespace after the array separator.
|
Going over your hard example I found I couldn't parse this: multi_line_array = [
"]",
# ] Oh yes I did
]It turns out that the grammar doesn't allow newlines and whitespaces after the array-sep before the comment. I've modified the production again to accommodate newlines and whitespace after the array separator before the comment. |
joelself
changed the title
after the last element without a leading newline.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RFC 4234 Section 3.5 advises the use of grouping notation rather than "bare" alternation when alternatives consist of multiple rules or literals, e.g. "( int float ) / ( bool char )" instead of "int float / bool char".
I might've gotten the grouping wrong, which just serves to illustrate the importance of using grouping notation.