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

Fixed flaw in :key_value rule #17

Closed
wants to merge 2 commits into from
Closed

Fixed flaw in :key_value rule #17

wants to merge 2 commits into from

Conversation

88Alex
Copy link

@88Alex 88Alex commented Jun 27, 2013

I edited the rule(:key_value) so spaces are not required before and after the =.
This way, key=3 would pass.

I edited the rule(:key_value) so spaces are not required before and after the =.
This way, "key=3" would pass.
@bootstraponline
Copy link

👍 toml-lang/toml#180

I modified the Parslet rules to make everything ignore spaces. This way, `myArray=[1,2,3,4,5]` passes.
@samsonjs
Copy link
Contributor

I ran this code and it doesn't seem to work. I added a test case at https://github.com/samsonjs/toml/commit/865f07f839f857fd0a59a817a532623e0a4e3147.

You'll have to at least make keys not parse =, as right now key=what? = "foo" is valid and parses a key named key=what?. I'm not sure what else might be involved in making this work.

@samsonjs
Copy link
Contributor

Not parsing = as part of the key works, but it's unclear whether this behaves according to the spec or not. Here's a branch that fixes & tests any number of spaces around keys & values: https://github.com/samsonjs/toml/tree/key-values

@88Alex
Copy link
Author

88Alex commented Jul 20, 2013

With my modifications to the rule, key=what? = "foo" generates a parse error.

@dirk
Copy link
Collaborator

dirk commented Oct 19, 2013

Quoting TOML v0.2.0 spec:

Keys start with the first non-whitespace character and end with the last non-whitespace character before the equals sign.

Therefore key=3 should be invalid (and ugly).

@dirk dirk closed this Oct 19, 2013
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.

4 participants