-
Notifications
You must be signed in to change notification settings - Fork 95
fix: correct regexp slash escapes #149
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
fix: correct regexp slash escapes #149
Conversation
|
@michaelficarra can you take a look at this? |
|
@nzakas @JoshuaKGoldberg I've updated the PR to use the PEG to just parse the various components of the regexp. For now, the only parsing contexts are character classes, hex escapes, single-character escapes, and of course the top-level parsing context. Please review and also try it out to see if it works in your project. I also updated the tests to be more strict. |
a7d9efa to
0ace1a3
Compare
|
Rebased. |
|
✅ confirmed, that works for me! I posted the test case in eslint/eslint#16555 (comment):
Thanks for picking this up! |
|
@JoshuaKGoldberg published v1.7.0 |
Fixes #68 in support of eslint/eslint#16555.
This solution is a kind of hacky workaround based on #68's comments. It "encodes" all backslash-escaped forward slashes (
'\\' + '/') with a backslash and an equivalent unicode character ('\\' + '\\x2F').I think it would be much better to work within the PEG grammar in
grammar.pegjs. Unfortunately, there is no backtracking or other nontrivial regular expression techniques in PEG.js. Expressions liked:[^\/]+("capture any number of characters other than/, under the named") don't have a way I could find to, say, skip past a character like\.& { predicate }and! { predicate }with matchers like"/" d:.+ { ... } "/"or just"/" d:.+ { ... }also seemed promising. But then I couldn't figure out how to get it to match non-greedily: i.e. the full/foo\/bar/instead of just/foo\/.I don't often work with PEGs and am not confident I didn't miss some obvious technique. I hope someone who's more experienced in these matters will point something out that's more PEG-oriented. 🙂