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

Improve string escape usability in Proposal A #50

Open
glyn opened this issue Jun 8, 2020 · 1 comment
Open

Improve string escape usability in Proposal A #50

glyn opened this issue Jun 8, 2020 · 1 comment
Labels
Proposal A Anything discussing the advance of the Proposal A for a formal specification of JSONPath

Comments

@glyn
Copy link
Collaborator

glyn commented Jun 8, 2020

Currently, Proposal A uses the following (PEG.js) syntax for child names that can appear in brackets with single quotes:

SingleQuotedString
  = x:"\\'" xs:SingleQuotedString { return "'" + xs; }
  / x:"\\\\" xs:SingleQuotedString { return "\\" + xs; }
  / x:[^'] xs:SingleQuotedString { return x + xs; }
  / ''

Using an online evaluator for PEG.js and feeding in selector.peg, observe that the selector:

$['\'', '\\', '\n', '\\n']

parses to:

[
   [
      "children",
      [
         [
            "name",
            "'"
         ],
         [
            "name",
            "\"
         ],
         [
            "name",
            "\n"
         ],
         [
            "name",
            "\n"
         ]
      ]
   ]
]

Firstly, notice the redundancy: both '\n' and '\\n' are parsed as "\n". But, more importantly, notice that '\n' is treated as valid.

A user who forgets which characters need to be escaped might think that '\n' represents a string consisting of a newline character. Allowing this to parse is potentially unhelpful.

If we remove the redundancy, we can fail unsupported escape sequences (and allow for other escape sequences to be added in future).

For example, the alternative syntax:

SingleQuotedString
  = x:"\\'" xs:SingleQuotedString { return "'" + xs; }
  / x:"\\\\" xs:SingleQuotedString { return "\\" + xs; }
  / x:[^'\\] xs:SingleQuotedString { return x + xs; }
  / ''

fails to parse '\n' with the message Expected "'", "\\'", "\\\\", or [^'\\] but "\\" found, but loses no expressive power.

@cburgmer cburgmer added the Proposal A Anything discussing the advance of the Proposal A for a formal specification of JSONPath label Jun 8, 2020
@cburgmer
Copy link
Owner

cburgmer commented Jun 8, 2020

Nice catch. Looks good to me!

Will be interesting to see how the other implementations handle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal A Anything discussing the advance of the Proposal A for a formal specification of JSONPath
Projects
None yet
Development

No branches or pull requests

2 participants