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

Adding ignore case modifier "(?i)" to search causes results to not load. #395

Open
blakemcneill opened this issue Apr 7, 2021 · 2 comments
Labels
bug good first issue These issues are a great way to get to know Hound's codebase help wanted needs investigation

Comments

@blakemcneill
Copy link

blakemcneill commented Apr 7, 2021

When performing this type of search, the "searching" loading graphic sits there and no results appear. It does show how time it took to find the results and the number of results so it does seem like it finds the matches.

For example, searching for "Payments" returns results but searching for "(?i)Payments" causes the search to hang.

Running this from a local Docker instance on Windows 10. Repository is from BitBucket.

Edit: Forgot to mention. Checking ignore case does work.

@rfan-debug
Copy link
Contributor

rfan-debug commented Jun 11, 2021

Oh. I found the same issue on docker instance running on linux, too. Our repository is git. Probably it is an issue related to the query parser.

@salemhilal
Copy link
Contributor

Sorry for the late reply here!

I reproduced this bug locally. It looks like it's happening here:

hound/ui/assets/js/hound.js

Lines 402 to 410 in b8a39b2

getRegExp : function() {
var regexp = this.refs.q.getDOMNode().value.trim()
if (this.refs.lsearch.getDOMNode().checked) {
regexp = EscapeRegExp(regexp)
}
return new RegExp(
regexp,
this.refs.icase.getDOMNode().checked ? 'ig' : 'g');
},

For those who had to figure out what (i?) did specifically (like me), that syntax ignores casing in a specific group. Check out this link (search for "Grouping"):
https://pkg.go.dev/regexp/syntax

The problem is that the same expression is used in the client-side to highlight matches. That syntax isn't valid, apparently:
image

So while the server works and returns results just fine, the client errors out when trying to render them.

I think the fix here is to catch this exception and default to an empty regular expression instead. An even better implementation could be to strip out case modifiers if we can find them, but that would make the matched results imprecise and might be a little harder to implement properly (since this would likely need to happen when there's any other mismatch between server- and client-side RegExp evaluation).

@salemhilal salemhilal added good first issue These issues are a great way to get to know Hound's codebase help wanted labels Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue These issues are a great way to get to know Hound's codebase help wanted needs investigation
Projects
None yet
Development

No branches or pull requests

3 participants