Allow whitespace around section headers #9
Conversation
|
|
||
| identifier ::= [a-zA-Z_.?-] ([a-zA-Z0-9_.?-])*; | ||
| identifier ::= identifier-head (identifier-tail)*; |
There was a problem hiding this comment.
since you don't use identifier-head or identifier-tail, what's the value of it over:
identifier ::= [a-zA-Z_.?-] [a-zA-Z0-9_.?-]*;
?
There was a problem hiding this comment.
what do you mean I don't use it? I use it right in the line you commented on
There was a problem hiding this comment.
it's much more readable that way and consistent with keyword- chars. it makes it clear to the reader that there are different set of characters involved.
There was a problem hiding this comment.
I mean - this is the only place where you use it. Since it's not shared with other bits, I'm not sure if there's a value in keeping it as a separate entity.
I believe that it's clear that there are different characters involved since instead of [...]+ we give [...] [...]*.
| variable ::= '$' identifier; | ||
| keyword ::= [^=|#{}\[\]()]+; | ||
| keyword ::= keyword-head (keyword-tail* keyword-last)? |
There was a problem hiding this comment.
I'm concerned that the following will look cryptic:
keyword ::= [^=|#/{}\[\]()0-9] ([^=|#/{}\[\]() ]* [^=|#/{}\[\]()])?;
Then again, the grammar is intended at parsers not humans, so maybe that's okay. I would leave it in the clearer form at least for now since we don't use this grammar yet to generate parsers which in turn would allow us to fix bugs in the grammar itself. Thus readable-by-human is still pretty important to me.
There was a problem hiding this comment.
ok, I disagree but not gonna block on this. :)
|
@zbraniecki what are you thoughts on the current behavior of the l20n.js parser wrt. the chars allowed in keywords? |
|
umm, not sure what are you asking about. The l20n.js parser handles characters allowed in keywords? |
|
The grammar specifies that all possible characters except a shorthand of special ones like I'm asking if you have cycles to fix this in the parser or would you rater have me change the grammar for now and limit keywords to ASCII chars. Allowing more chars in the future will be backwards-compatible. |
|
I think I'd prefer to limit the chars in the spec for now. I'm concerned about non-ascii character being more confusing than helpful at this point. |
|
I removed the additional symbols for allowed characters and also restricted |
|
@zbraniecki can you take another look? |
|
@zbraniecki ping |
|
looks great! Sorry for holding you for so long. |
Fixes #5. @zbraniecki -- this implements the changes we talked about last week to characters allowed in keywords, including white-space. Note that currently the l20n.js parser doesn't allow non-Latin characters in keywords. Should we update the spec here for now?