fix(find): respect spaces when query has no whitespace #20364
Conversation
calixteman
left a comment
calixteman
left a comment
There was a problem hiding this comment.
Unfortunately, it won't work with a pdf containing "Hello. World" and a query equals to "o. w" (fyi it's ok in Acrobat).
I think a fix could be to add optional white spaces around group of punctuation signs which induces to not have [ ]* between consecutive punctuation signs.
It'd lead to update SPECIAL_CHARS_REG_EXP but you've to take care to the case of . and ?.
|
Handled the Suggested test case. Also Attaching the PDF with the test case: |
| const DIACRITICS_REG_EXP = /\p{M}+/gu; | ||
| const SPECIAL_CHARS_REG_EXP = | ||
| /([.*+?^${}()|[\]\\])|(\p{P})|(\s+)|(\p{M})|(\p{L})/gu; | ||
| /([*+^${}()|[\]\\])|((?:[.?]|\p{P})+)|(\s+)|(\p{M})|(\p{L})/gu; |
There was a problem hiding this comment.
Why do you want to not capture . or ? ?
There was a problem hiding this comment.
I’m still capturing them just in the punctuation-run group (p2) so consecutive punctuation is treated as one token. That lets us add spaces only around the run, never inside it.
| return `[ ]*\\${p1}[ ]*`; | ||
| // Escaped metacharacters like . * + ? ... | ||
| // Allow spaces around them ONLY if the user typed spaces. | ||
| return queryHasWhitespace ? `[ ]*\\${p1}[ ]*` : `\\${p1}`; |
There was a problem hiding this comment.
Why do you want to do that ?
| return `[ ]*${p2}[ ]*`; | ||
| // Punctuation: allow optional spaces ONLY if the user typed spaces. | ||
| // p2 is a *run* of punctuation; escape it as a whole. | ||
| const escapedRun = escapeForRegex(p2); |
There was a problem hiding this comment.
You just have to replace the . and the ? by . and ?
| // Punctuation: allow optional spaces ONLY if the user typed spaces. | ||
| // p2 is a *run* of punctuation; escape it as a whole. | ||
| const escapedRun = escapeForRegex(p2); | ||
| return queryHasWhitespace ? `[ ]*${escapedRun}[ ]*` : `${escapedRun}`; |
There was a problem hiding this comment.
Again I don't understand why you want to make a difference between hasWhitespace and !hasWhitespace.
|
@shreeMahadikGit do you plan to work on this patch or not ? |
|
Hii @calixteman, First of all, apologies for the late reply. I found a few more issues with the solution I implemented. As you can see in the attached screenshot, the search isn’t matching the correct characters. I’ll need to do some additional work on my solution to address this. I have refactored the code according to your comments and removed most of the unnecessary parts as well, but I’ve discovered a few more issues in the current implementation that need to be fixed. If you think someone else can address this more quickly, please feel free to reassign the issue. 
|
|
Thank you very much for your contribution, but I've addressed the issue myself: |
3 participants
Fixes this issue:
#20225