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

Better string support (double quotes or escaped singles) #18

Open
2 tasks
ErisDS opened this issue Jul 17, 2018 · 1 comment
Open
2 tasks

Better string support (double quotes or escaped singles) #18

ErisDS opened this issue Jul 17, 2018 · 1 comment
Labels
bug Something isn't working enhancement New feature or request

Comments

@ErisDS
Copy link
Member

ErisDS commented Jul 17, 2018

NQL works mostly on literals - strings that don't need to be quoted because they are obviously strings.

E.g. tag:photo, we don't need to do tag:'photo', it's totally redundant!

However, if a string contains a character that has another purpose in NQL, we have to use quotes (or escaping) to make it clear the whole thing is intended as a string.

E.g. in a date string published_at:<2017-09-01 12:45:12, both the space and the colons mean that we cannot use a literal, and we need to use quotes e.g. published_at:<'2017-09-01 12:45:12'.

NOTE: should colons be allowed inside of literals?! Or should they be allowed as escaped-chars!??!

This works absolutely wonderfully if the filter lives in a URL encoded URL string, or in JSON, but poses a problem in javascript code:

Example from the prev/next helper in Ghost:

  apiOptions = {
            include: 'author,tags',
            order: 'published_at ' + order,
            limit: 1,
            filter: "slug:-" + slug + "+published_at:" + op + "'" + publishedAt + "'", // jscs:ignore
        }

Normally we would use single quotes around the code, but we can't because this would clash with NQL.

In other languages, you'd switch to using double quotes, but NQL doesn't support this.

If we switch to using template literals:

  apiOptions = {
            filter: `slug:-${slug}+published_at:${op}'${publishedAt}'`, // jscs:ignore
}

The single quotes end up escaped like \' inside the string, which NQL also doesn't seem to understand - or it goes wrong somewhere on the way to NQL - need to test this better to understand what's happening - it definitely should work

TODO:

  • At least support escaped single quotes (this is a bug)
  • Consider supporting double quotes (this is an enhancement)
@ErisDS ErisDS added enhancement New feature or request bug Something isn't working labels Jul 17, 2018
@daniellockyer daniellockyer transferred this issue from TryGhost/NQL-Lang Mar 1, 2022
ErisDS added a commit that referenced this issue Mar 8, 2022
refs #18

- Added a bunch of additional tests, all passing, that show strings are lexed correctly
- I was not able to reproduce any issues with quotes not being treated correctly
- Escaped quotes are handled as expected
@ErisDS
Copy link
Member Author

ErisDS commented Mar 8, 2022

As far as I can tell, everything is working correctly the only thing we don't support is allowing an unescaped double quote in a single-quoted string, this is weird.

Ideally we'd support either quote style, and not require the other quote to be escaped within it.

Beyond that, it's possible to escape an apostrophe inside a single-quoted string, so we can represent 'john o'nolan'.

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

No branches or pull requests

1 participant