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

Fix typo in Base filter #61682

Closed
wants to merge 1 commit into from
Closed

Conversation

Yuki2718
Copy link
Collaborator

Unescaped period in regex

@Alex-302
Copy link
Member

This is not a typo) Easier to search.

@Alex-302 Alex-302 closed this Aug 17, 2020
@Yuki2718
Copy link
Collaborator Author

This is not a typo) Easier to search.

You mean it's better for management/maintenance?

@Alex-302
Copy link
Member

invoke.js visually better distinguishable

@Yuki2718
Copy link
Collaborator Author

invoke.js visually better distinguishable

@Alex-302 If that is the sole reason, could you consider put an example as a comment, as you did in this commit dd28d97, and adopt the suggested change for better performance on uBO? \. should be more efficient than the wildcard . even for plain regex, but on uBO it makes real difference. If I understand correctly only invoke is the potentially tokenizable word in the rule for uBO, but adding . will make it untokenizable.

@gwarser Sorry to trouble you but please correct me if I'm wrong.

@gwarser
Copy link

gwarser commented Aug 23, 2020

This will not help - character sets will prevent tokenization - ...[a-z0-9]... uBO simply looks for [ and ( (this.reRegexTokenAbort = /[([]/;) and breaks.

@Yuki2718
Copy link
Collaborator Author

@gwarser Thanks for pointing my misunderstand out!
@Alex-302 Sorry, please forget about this!

@Yuki2718 Yuki2718 deleted the fix_typo branch October 19, 2020 13:53
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.

None yet

3 participants