Skip to content
This repository has been archived by the owner on Feb 12, 2021. It is now read-only.

Fuzzy match logic update #112

Merged
merged 24 commits into from
Jan 13, 2020
Merged

Fuzzy match logic update #112

merged 24 commits into from
Jan 13, 2020

Conversation

theClueless
Copy link
Collaborator

@theClueless theClueless commented Dec 29, 2019

Problem context:

  1. Wox's search mechanism uses character matching- in a given query string "sql" matching with program title compare string "Microsoft SQL Management Studio" scenario, the match fails for 'Regular' search precision level. This is due to 'RawScore' obtain as 26, not enough to make the Score required for 'Regular' search precision level of 50.

  2. Searching for results that may contain partial words such as "term" should return "Windows Terminal' without having to type in "Windows Te".

Just an overall more natural search experience while retaining the capability of using character search as well.

Fixes issues #84 & #92

Changes:

  • Remove String Builder as it now has no use.
  • Updated SearchPrecisionScore property + in Settings class conversion from string to SearchPrecisionScore.
  • Updated some logging
  • Updated stringmatcher:
  • Added flags that the method will find:.
    • allSubstringsContainedInCompareString: If all query's sub-strings are contained in the string to compare.
    • Added the ability to fix a word location if found contained fully, for example:
      □ Query: sql
      □ String to compare: server sql mg
      □ In old alg:
      Start index = 0, "server sql" and matched indexs are = 0,8,9
      □ In new alg: it will see that the second sql is better match and result would have a full substring as follows:
      Start index = 7, "sql" and matched indexes are = 7,8,9

@jjw24
Copy link
Owner

jjw24 commented Dec 30, 2019

This is getting a bit hard to read and maintain (not saying the original isn't). I will see if I can make it more readable

Make variables more descriptive of the state they represent
- Move checking if there is a prev compare string char match into function
- Move updating of index list when a better match is found for the first substring logic into function
@theClueless
Copy link
Collaborator Author

I can do some work on readability if you want. I agree it became a little complicated :)

@jjw24
Copy link
Owner

jjw24 commented Jan 6, 2020

Hey I have started updating the code, it's just in a separate branch for now and will merge in once done. Want to also tweak the scoring for the new code as well. I will aim to do this later in the day.

https://github.com/jjw24/Wox/tree/WIP_fuzzyMatchUpdates

- Moved if statement that checks if all query substrings are matched into a funciton
- convert into shorthand expression the if statement that checks if all words are fully matched
1. Remove the github repo reference as we have mixed in substring matching
2. Added context on how the logic is run
 occurs when query contains more than one whitespace eg. 'sql  manag'
Failed if query text is 'sql servman'- returns true when should be false
- moved it up so evaluation is included in the final substring check
checking if all substrings contained in compareString
Not necessary to have and not needed to add another dimension to the scoring
Two scoring changes only as a result of substring matching.
makes more sense and less conversion to int for actual precision score
@jjw24 jjw24 changed the title Fuzzymatchupdates Fuzzy match logic update Jan 7, 2020
@jjw24 jjw24 added the enhancement New feature or request label Jan 7, 2020
@jjw24
Copy link
Owner

jjw24 commented Jan 7, 2020

@theClueless my core changes basically removed containedFully flag as feeling it adds more complication to the match result, and tweaked if all query substrings are contained in compare string, then add additional scoring 10 score per query string character.

@theClueless
Copy link
Collaborator Author

it might be a more complex but checking it is very simple and deciding to add a little to the score be very powerful, maybe mainly when we have more than one word in the query string, for example:
in query: "sql m" both: "sql motor" and "sql management" will get the same value so just adding a small increment (like a const value) could add something.
Also I think that adding 10 per char is too much, in a big query string it will override all the other factors too much, what do you think? (maybe factor it differently according to the query size?)

@jjw24
Copy link
Owner

jjw24 commented Jan 8, 2020

it might be a more complex but checking it is very simple and deciding to add a little to the score be very powerful, maybe mainly when we have more than one word in the query string, for example:
in query: "sql m" both: "sql motor" and "sql management" will get the same value so just adding a small increment (like a const value) could add something.

From memory I wasn't able to get it to return the correct state when query string is 'sql studio'(I think it's something like this, can verify later) and I suspect it's how containedFully is determined. But doesn't it work the same way as if all substrings in query string matched compare string?

@jjw24
Copy link
Owner

jjw24 commented Jan 8, 2020

Also I think that adding 10 per char is too much, in a big query string it will override all the other factors too much, what do you think? (maybe factor it differently according to the query size?)

For me mainly 10 because that's how query string 'sql' can pass regular search precision, and users shouldn't need to type in long queries to get the result they want. Happy to hear a different cal method though

Copy link
Collaborator Author

@theClueless theClueless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding some comments

Wox.Infrastructure/StringMatcher.cs Outdated Show resolved Hide resolved
Wox.Infrastructure/StringMatcher.cs Outdated Show resolved Hide resolved
Wox.Infrastructure/StringMatcher.cs Outdated Show resolved Hide resolved
Wox.Infrastructure/StringMatcher.cs Show resolved Hide resolved
Wox.Test/FuzzyMatcherTest.cs Show resolved Hide resolved
Copy link
Collaborator Author

@theClueless theClueless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't let me approve since I'm the owner of the original PR, can you approve.
looks great :) I think these are important changes 👍

@jjw24
Copy link
Owner

jjw24 commented Jan 13, 2020

Awesome, thanks for your time and effort to make this better. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants