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

Dash in a rule name results in a parsing error #44

Closed
wilrodriguez opened this issue Jan 11, 2023 · 6 comments
Closed

Dash in a rule name results in a parsing error #44

wilrodriguez opened this issue Jan 11, 2023 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@wilrodriguez
Copy link

Just discovered a fun little bug. Looks like dashes in rule names will generate a syntax error. Something like this:

Error on line 118, column 8.

    foo-bar Baz optional
       ^

You can use the following to replicate it:

ruleset Baz {
    qux str optional
}

schema {
    foo-bar Baz optional
}
@ryan95f ryan95f added the bug Something isn't working label Jan 11, 2023
@ryan95f
Copy link
Owner

ryan95f commented Jan 11, 2023

Thanks for this. I will create a PR to solve this.

@ryan95f ryan95f self-assigned this Jan 11, 2023
@wilrodriguez
Copy link
Author

This one is just a regex issue. Just need a - at the end of the pattern in grammar.lark on lines 29-31.

required_rule: /[a-zA-Z0-9_-]+/ type "required" NEW_LINES
             | /[a-zA-Z0-9_-]+/ type NEW_LINES
optional_rule: /[a-zA-Z0-9_-]+/ type "optional" NEW_LINES

@wilrodriguez
Copy link
Author

wilrodriguez commented Jan 11, 2023

Notably... should probably also allow other symbol characters like :%$@!^*(). Could probably even update it to \S since YAML is pretty much flexible enough to allow most characters as a key... and you can pretty much bet someone is going to need to have a key somewhere with some weird symbols. Though even \S is somewhat naive since you can have whitespace in keys, and in fact, it's quite common. YAML supports setting any printable unicode character, especially if you quote your keys. So... it might rather naive to assume we can are going to be able to predict what someone will need for a key. Perhaps we can match a certain subset for plain values or match everything contained within quotes if they need something extra.

@ryan95f
Copy link
Owner

ryan95f commented Jan 12, 2023

That is a very good point. I will expand my current PR #45 to include these suggestions.

@ryan95f
Copy link
Owner

ryan95f commented Jan 15, 2023

As part of release 0.2.1, rule names have been updated to allow for all unicode characters including dashes and spaces.

@wilrodriguez
Copy link
Author

Awesome! Just confirmed your change fixes this issue. I'll open a separate enhancement issue for broadening the key definition. I made a few attempts at implementing it myself but LARK never liked my syntax and I'm not really sure why.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants