-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
Regexes don't work #8
Comments
Does the I just checked out version 1.6 and it looks like the same issue is there for me. Is this the first time you've run these queries or was this working for you before? This is a major issue, but I'm hoping that at least it's not a regression. |
It was the first time I tried a query which doesn't work. PS: Both queries don't work for me although they should. I updated my comment above to clarify this. |
@gsingh93 I've found the "error". StackOverflow splits the string
I don't think there is a method for solving this problem, which is correctly guessing such combinations all the time, let alone taking CSS positioning into account. |
Thanks for looking into this. Spans and anchor tags especially are used to separate a lot of text, so we're going to run into this for a lot of regexes. For example, on this page search for "with GitHub". You won't find anything because "GitHub" is wrapped in an anchor tag. Maybe we should group all adjacent inline elements together when searching for text, and if not all inline elements, at least spans and anchor tags. |
Exactly, I've thought about the same thing. The problem, however, is to do the highlighting afterwards. You have to know which character (in the grouped text) belongs to which HTML element on the site. Otherwise characters which are actually not included in the search result get highlighted. Example:
Grouped version:
Now how can the extension determine the part (or rather the element in the source HTML) to highlight? It can't, we therefore have to choose the parent |
I think while it wouldn't be easy, it's still possible. When we merge the elements to get the combined text, we keep a list of the original elements that were merged and a mapping of ranges (starting and ending positions in the combined text) to these elements. If we find a match in the combined text, we simply need to find which element the match starts in, and since we know where the match starts in the combined text, we can use our mapping of ranges to elements to know which element the match starts in. Now since we know the matched text, we know the match is continuous, and we know all of the merged elements in order, we can just iterate through the elements highlighting as much of the match as the element contains until we've highlighted the entire match. For example, if we have It's been a while since I wrote the highlighting code, so while this seems doable to me, I might be missing something. Let me know if this makes sense. |
That sounds good! I've made a little proof of concept script: HTML:
TypeScript:
(I found an interesting online editor which also supports TypeScript: http://fiddlesalad.com/typescript/ just copy & paste my code!) Question:
TODOs:
|
This looks good. My only concern about combining the whole DOM tree is that two completely unrelated pieces of text (possibly on opposite sides of the screen) could then be matched. This technically could happen with spans too, but if we code towards the general case (which is that people generally use spans/inline elements correctly), then I think we should be fine. We could also just test both versions on different sites and see whether we get more false positives or false negatives with each version. |
👍 One could also take the elements' positions into account ... but it only gets more complicated over time! I think prototyping the span code (for now) and getting a first functional search engine would be the best |
In Chrome 35, it does not even find single-letter regexps (or if does, it does not highlight them). What's the point? |
@jayeye If you want to report a bug, post both the page and the regex query you're using. Otherwise, there's no way we can help you. |
I currently face some strange problems.
I use the official 1.7 version from the Chrome Web Store (updated on April 27, 2014).
modified \d+
Also try:
asked 39
in this case.Am I missing something?
The text was updated successfully, but these errors were encountered: