-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Fallback to PCRE2 if match whole word used with regexp #82072
Fallback to PCRE2 if match whole word used with regexp #82072
Conversation
Thanks for the PR! But if it is a non-regex whole word search, it won't be added, right? Maybe it should just be duplicated and put next to the |
Correct. If
That was my first intention, but if it is added unconditionally then it will be also applied to all non-regex whole word searches. If condition added - that there will be 2 similar conditions which looks confusing as for me. Therefore I moved Example with duplicate + condition. if (query.isWordMatch) {
const regexp = createRegExp(query.pattern, !!query.isRegExp, { wholeWord: query.isWordMatch });
const regexpStr = regexp.source.replace(/\\\//g, '/'); // RegExp.source arbitrarily returns escaped slashes. Search and destroy.
args.push('--regexp', regexpStr);
if (query.isRegExp) { // same as below
args.push('--auto-hybrid-regex');
}
} else if (query.isRegExp) { // same as above
let fixedRegexpQuery = fixRegexNewline(query.pattern);
fixedRegexpQuery = fixNewline(fixedRegexpQuery);
args.push('--regexp', fixedRegexpQuery);
args.push('--auto-hybrid-regex');
} [1] only exception is special case when search query is exactly |
But we convert all whole word searches to regex searches, even if they are not originally regex searches, that is my point. |
Yes. I'll happily update PR to simply duplicate line if it's OK to always include it. |
Ok, that's a good point. |
Yes, so it's just about preference. I don't think that it has any performance impact in any of two options (this PR or include for every regexp search). Thanks for reviewing! |
Fixes #82071
This PR ensures that
--auto-hybrid-regex
will be added to Regexp searches regardless of Match Whole Word state.Changed order of operations a bit to keep related checks and CLI arguments nearby.