search

jmespath / jmespath.py

JMESPath is a query language for JSON.
http://jmespath.org
MIT License
2.19k stars 181 forks source link

Add proper string escaping #8

Closed jamesls closed 11 years ago

jamesls commented 11 years ago

Grammar has been updated with supported escape sequences (See #2 for details). These changes are isolated to the lexer module.

cc @mtdowling

toastdriven commented 11 years ago

Looks good. I worry about a degenerate-style case (like \\\), but I suspect that's already handled & should be an error anyhow.

mtdowling commented 11 years ago

The alternative to escaping a bunch of characters is to add optional double quotes to identifier. This would mean you'd only need to worry about escaping double quotes and would allow any character to be present within double quotes.

jamesls commented 11 years ago

I think it's important to point out that escaping special characters and enclosing identifiers in quotes are not mutually exclusive, I actually think we should eventually support both. I can see times when escaping special chars is not ideal (when you have a lot of chars to escape in an identifier). I can see when using double quotes for identifiers is not ideal (if you're storing a jmespath expression in a JSON object like we do for the compliance tests, if you're storing a jmespath expression as a string in a language that requires strings to be only double quoted, on the CLI, etc).

Is there any downside to supporting both double quotes and escaping special chars? The only downside I see is that it makes the implementation more complicated, but I don't see any downsides from a jmespath user's perspective.

jamesls commented 11 years ago

I updated the PR to support quoting identifiers (just double quotes for now).

After writing compliance tests for these, I don't feel as strongly one way or another. In fact, because the \ char is used as an escape sequence for most languages and formats (such as JSON), you have to double escape \. See this test for an example of how crazy this can get.

jamesls commented 11 years ago

I went ahead and removed the escape sequences, e.g. foo\.bar and just kept the double quotes in for now. The commit is still there for reference if we decide to include this in the future.