Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grammar on README is incorrect #5

Closed
jeremeamia opened this issue Jul 6, 2013 · 4 comments
Closed

Grammar on README is incorrect #5

jeremeamia opened this issue Jul 6, 2013 · 4 comments

Comments

@jeremeamia
Copy link

There seems to be a couple of typos in your grammer on the README. Also, based on your compliance tests, there seems to be a few things missing. How does this look?

expression : expression ' or ' expression
           | expression '.' expression
           | expression '[' (number | wildcard) ']'
           | expression '.' wildcard
           | expression '.' number
           | identifier
identifier : [a-zA-Z_][a-zA-Z0-9]+
number : -?[1-9]+
wildcard : '*'

Also, according to your grammar, identifier has to be 2 characters or more. Wouldn't [a-zA-Z_][a-zA-Z0-9]* be better?

Are number or wildcard allowed at the root of the expression? If so, then the grammar could be changed to:

expression : expression ' or ' expression
           | expression '.' expression
           | expression '[' (number | wildcard) ']'
           | identifier
           | number
           | wildcard
identifier : [a-zA-Z_][a-zA-Z0-9]*
number : -?[1-9]+
wildcard : '*'
@jeremeamia
Copy link
Author

Or do we need or_expression : expression ' or ' expression | expression?

@jamesls
Copy link
Member

jamesls commented Jul 8, 2013

Thanks for the heads up, the README grammar is fairly out of date. With the recent or changes in #3, the root wildcard changes in #4, and the possibly update for identifiers and escaping chars in #2, the new syntax in ABNF form is going to be:

    expression        = sub-expression / index-expression / or-expression / identifier / "*"
    sub-expression    = expression "." expression
    or-expression     = expression "||" expression
    index-expression  = expression bracket-specifier / bracket-specifier
    bracket-specifier = "[" (number / "*") "]"
    number            = [-]digit
    digit             = "1" / "2" / "3" / "4" / "5" / "6" / "7" / "8" / "9" / "0"
    identifier        = *char
    char              = unescaped /
                        escape (
                            %x20-2F /    ; Space,!,",#,$,%,&,',(,),*,+,comma,-,.,/
                            %x3A-40 /    ; :,;,<,=,>,?,@
                            %x5B    /    ; Left bracket: [
                            %x5C    /    ; Back slash: \
                            %x5D    /    ; Right bracket: ]
                            %x5E    /    ; Caret: ^
                            %x60    /    ; Backtick: `
                            %x7B-7E /    ; {,|,},~
                            b       /    ; backspace
                            n       /    ; new line
                            f       /    ; form feed
                            r       /    ; carriage return
                            t       )    ; tab
    escape            = %x5C   ; Back slash: \
    unescaped         = %x30-39 / %x41-5A / %x5F / %x61-7A / %x7F-10FFFF

I'm going to wait until the PRs have been merged and #2 has been resolved until I update the README, but it's going to be fairly close to the above grammar (with possible changes to the char rule). I'll go ahead and put this in pending for the time being.

@jeremeamia
Copy link
Author

Cool. That looks fairly comprehensive. I've been looking through your implementation as well. One thing I wasn't sure of is whether or you support having something like "foo.bar||baz.fizz"? Your new grammar indicates that this is allowed.

@jamesls
Copy link
Member

jamesls commented Jul 8, 2013

Yes, it is allowed, that was one of the added benefits of switching from or to ||. I've added ba4d283 to the compliance tests to verify this.

@jamesls jamesls closed this as completed Oct 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants