Skip to content

Fix regex when json values includes quotes #40

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

Merged
merged 2 commits into from
Jul 26, 2022

Conversation

erikn69
Copy link
Contributor

@erikn69 erikn69 commented Jun 23, 2022

There is a bug when you has '" "', or when you try to save json values on db
Examples:

SELECT '{"json_index":"json_value"}' AS test;
SELECT '"json_string"' AS test;
SELECT "'string_value'" AS test;
SELECT '`string_value`' AS test;

Result:
image
Result with this PR:
image


There is also another bug

SELECT '\'string_value' AS test;

image

Maybe solution

{
    name: 'string',
    regex: /(['].*?(?<!\\)[']|["].*?(?<!\\)["]|[`].*?(?<!\\)[`])/g
}

@erikn69 erikn69 force-pushed the patch-6 branch 4 times, most recently from ddd24ed to 9747b2b Compare June 23, 2022 19:09
@scriptcoded
Copy link
Owner

Thank you for the PR! The linter seems to complain about the test you added (https://github.com/scriptcoded/sql-highlight/pull/40/files#diff-5bb8db779819ddef5956a5d9d5949c05ef7445237656ca37bf2f02720271440bR21). If you can fix that issue I'd be happy.

As for the other bug you mentioned, it's something I'm aware. Some language features are hard to capture using only regular expressions, and I have briefly thought about rewriting this library using something like Ohm for parsing. However, based on this change I think we should be able to add support for escaping quotes. It's not pretty, but I think it'll do the job:

(['](?:\\'|.)*?[']|["](?:\\"|.)*?["]|[`](?:\\`|.)*?[`])

https://regex101.com/r/eKffvh/1

@erikn69 erikn69 force-pushed the patch-6 branch 2 times, most recently from b54c258 to 2ef4ef8 Compare July 25, 2022 13:55
@erikn69
Copy link
Contributor Author

erikn69 commented Jul 25, 2022

Done

@scriptcoded scriptcoded merged commit b2c2a6c into scriptcoded:master Jul 26, 2022
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.

2 participants