-
Notifications
You must be signed in to change notification settings - Fork 445
searcher.js: Escape characters when building regexps #1397
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
Conversation
Otherwise characters with regexp meaning like `.` won't behave as expected.
|
🚀 Preview deployment available at: https://6621dea1.rdoc-6cd.pages.dev (commit: 481b5d5) |
|
Sorry can you give me an example of the issue that you're trying to fix? |
|
For now this doesn't fix much of anything, because nothing really match these special characters. But my initial plan was to allow to search by fully qualified name, e.g. Hence I figured I'd just submit that small fix independently. If you want to check it works, go on the current version of RDoc, and search for On this branch, it doesn't crash. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok I can verify that it avoids the JS error. Thanks!
|
quote mdn:
and that's why search broke in safari 17.6:
i don't think only and exclusively browsers from 2025 should be required for a documentation page. would you accept an alternative implementation even if it means "bundling" a polyfill? i think even this should work for the purpose: if (!RegExp.escape) {
RegExp.escape = function(string) {
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
};
} |
Why the snark? Of course it's an oversight, and of course a fix is welcome. It never occurred to me I'll be looking for a polyfill. |
Otherwise characters with regexp meaning
like..won't behave as expectedActually
.when inside[]doesn't have a special meaning, however\and a few other break the search.