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.

Sign up for GitHub

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

Jump to bottom

handle regex special characters #250

Merged
merged 1 commit into from
Oct 22, 2016
Merged

handle regex special characters #250

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion debug.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ function enable(namespaces) {

for (var i = 0; i < len; i++) {
if (!split[i]) continue; // ignore empty strings
namespaces = split[i].replace(/\*/g, '.*?');
namespaces = split[i].replace(/[\\^$+?.()|[\]{}]/g, '\\$&').replace(/\*/g, '.*?');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo, we should offload this work

https://github.com/sindresorhus/escape-string-regexp ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at that, but * would be escaped by that (or any such) library -- you'll note I'm not escaping it here so the existing wildcard replace will work as-is. It's possible to work around this issue, but for a 1-liner it seemed like overkill.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should add: it especially complicates the issue since this needs to work in both node and the browser. If this were a node-only library it wouldn't be such a big deal.

If the functionality needed were a big deal and possibly subject to further development, bug fixes, etc, it might be worth it -- but again, in comparison to a 1-liner, it seems like unnecessary complication.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah gotcha

if (namespaces[0] === '-') {
exports.skips.push(new RegExp('^' + namespaces.substr(1) + '$'));
} else {
Expand Down