Fix pinyin fuzzysearch (microsoft#131)

* fix typo

* make function obsolete 

it is not used in the code

* rewrite the function that converts chinese chars to pinyin

1. Only difference in this rewrite is instead of returning 2D array, return as a combined single string of all the possible pinyin combination. Since fuzzy search does character matching, this shouldn't be a problem.

2. Added a function that returns a custom language converter. In this case Pinyin converter. New converters can be added.

* Use new language converter param + strip out ScoreForPinyin method

* update

* Change parameter name

* fix failing tests

* WIP

* Remove todo

There should be some distinction between score after precision filter and actual raw score derived from FuzzySearch. Although so far RawScore is used in testing, but it seems to describe the structure. Originally it was to avoid assigning score directly as it would be hard to reason about that output of FuzzySearch score is.

* Add constructors, remove default to enforce required properties

* remove setting rawscore in SearchPrecision

* Change method name to reflect intention

* Change parameter name + update comment

* update

* Remove params comment

Co-authored-by: theClueless <14300910+theClueless@users.noreply.github.com>

Wox.Infrastructure/Alphabet.cs

@@ -2,28 +2,35 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text;
 using hyjiacan.util.p4n;
 using hyjiacan.util.p4n.format;
+using JetBrains.Annotations;
 using Wox.Infrastructure.Logger;
 using Wox.Infrastructure.Storage;
 using Wox.Infrastructure.UserSettings;
 
 namespace Wox.Infrastructure
 {
-    public static class Alphabet
+    public interface IAlphabet
     {
-        private static readonly HanyuPinyinOutputFormat Format = new HanyuPinyinOutputFormat();
-        private static ConcurrentDictionary<string, string[][]> PinyinCache;
-        private static BinaryStorage<ConcurrentDictionary<string, string[][]>> _pinyinStorage;
-        private static Settings _settings;
+        string Translate(string stringToTranslate);
+    }
+
+    public class Alphabet : IAlphabet
+    {
+        private readonly HanyuPinyinOutputFormat Format = new HanyuPinyinOutputFormat();
+        private ConcurrentDictionary<string, string[][]> PinyinCache;
+        private BinaryStorage<ConcurrentDictionary<string, string[][]>> _pinyinStorage;
+        private Settings _settings;
 
-        public static void Initialize(Settings settings)
+        public void Initialize([NotNull] Settings settings)
         {
-            _settings = settings;
+            _settings = settings ?? throw new ArgumentNullException(nameof(settings));
             InitializePinyinHelpers();
         }
 
-        private static void InitializePinyinHelpers()
+        private void InitializePinyinHelpers()
         {
             Format.setToneType(HanyuPinyinToneType.WITHOUT_TONE);
 
@@ -38,7 +45,35 @@ private static void InitializePinyinHelpers()
             Log.Info($"|Wox.Infrastructure.Alphabet.Initialize|Number of preload pinyin combination<{PinyinCache.Count}>");
         }
 
-        public static void Save()
+        public string Translate(string str)
+        {
+            return ConvertChineseCharactersToPinyin(str);
+        }
+
+        public string ConvertChineseCharactersToPinyin(string source)
+        {
+            if (!_settings.ShouldUsePinyin)
+                return source;
+
+            if (string.IsNullOrEmpty(source))
+                return source;
+
+            if (!ContainsChinese(source))
+                return source;
+
+            var combination = PinyinCombination(source);
+
+            var pinyinArray=combination.Select(x => string.Join("", x));
+            var acronymArray = combination.Select(Acronym).Distinct();
+
+            var joinedSingleStringCombination = new StringBuilder();
+            var all = acronymArray.Concat(pinyinArray);
+            all.ToList().ForEach(x => joinedSingleStringCombination.Append(x));
+
+            return joinedSingleStringCombination.ToString();
+        }
+
+        public void Save()
         {
             if (!_settings.ShouldUsePinyin)
             {
@@ -50,11 +85,12 @@ public static void Save()
         private static string[] EmptyStringArray = new string[0];
         private static string[][] Empty2DStringArray = new string[0][];
 
+        [Obsolete("Not accurate, eg 音乐 will not return yinyue but returns yinle ")]
         /// <summary>
         /// replace chinese character with pinyin, non chinese character won't be modified
         /// <param name="word"> should be word or sentence, instead of single character. e.g. 微软 </param>
         /// </summary>
-        public static string[] Pinyin(string word)
+        public string[] Pinyin(string word)
         {
             if (!_settings.ShouldUsePinyin)
             {
@@ -76,7 +112,7 @@ public static string[] Pinyin(string word)
         /// e.g. 音乐 will return yinyue and yinle
         /// <param name="characters"> should be word or sentence, instead of single character. e.g. 微软 </param>
         /// </summmary>
-        public static string[][] PinyinComination(string characters)
+        public string[][] PinyinCombination(string characters)
         {
             if (!_settings.ShouldUsePinyin || string.IsNullOrEmpty(characters))
             {
@@ -111,13 +147,13 @@ public static string[][] PinyinComination(string characters)
             }
         }
 
-        public static string Acronym(string[] pinyin)
+        public string Acronym(string[] pinyin)
         {
             var acronym = string.Join("", pinyin.Select(p => p[0]));
             return acronym;
         }
 
-        public static bool ContainsChinese(string word)
+        public bool ContainsChinese(string word)
         {
             if (!_settings.ShouldUsePinyin)
             {
@@ -135,7 +171,7 @@ public static bool ContainsChinese(string word)
             return chinese;
         }
 
-        private static string[] Combination(string[] array1, string[] array2)
+        private string[] Combination(string[] array1, string[] array2)
         {
             if (!_settings.ShouldUsePinyin)
             {

Wox.Infrastructure/FuzzyMatcher.cs

@@ -16,7 +16,7 @@ private FuzzyMatcher(string query, MatchOption opt)
 
         public static FuzzyMatcher Create(string query)
         {
-            return new FuzzyMatcher(query, StringMatcher.DefaultMatchOption);
+            return new FuzzyMatcher(query, new MatchOption());
         }
 
         public static FuzzyMatcher Create(string query, MatchOption opt)
@@ -26,7 +26,7 @@ public static FuzzyMatcher Create(string query, MatchOption opt)
 
         public MatchResult Evaluate(string str)
         {
-            return StringMatcher.FuzzySearch(query, str, opt);
+            return StringMatcher.Instance.FuzzyMatch(query, str, opt);
         }
     }
 }

Wox.Infrastructure/StringMatcher.cs

@@ -1,43 +1,46 @@
 using System;
 using System.Collections.Generic;
+using System.ComponentModel;
 using System.Linq;
-using System.Text;
-using Wox.Infrastructure.Logger;
-using Wox.Infrastructure.UserSettings;
 using static Wox.Infrastructure.StringMatcher;
 
 namespace Wox.Infrastructure
 {
-    public static class StringMatcher
+    public class StringMatcher
     {
-        public static MatchOption DefaultMatchOption = new MatchOption();
+        private readonly MatchOption _defaultMatchOption = new MatchOption();
 
-        public static SearchPrecisionScore UserSettingSearchPrecision { get; set; }
+        public SearchPrecisionScore UserSettingSearchPrecision { get; set; }
 
-        public static bool ShouldUsePinyin { get; set; }
+        private readonly IAlphabet _alphabet;
+
+        public StringMatcher(IAlphabet alphabet = null)
+        {
+            _alphabet = alphabet;
+        }
+
+        public static StringMatcher Instance { get; internal set; }
 
         [Obsolete("This method is obsolete and should not be used. Please use the static function StringMatcher.FuzzySearch")]
         public static int Score(string source, string target)
         {
-            if (!string.IsNullOrEmpty(source) && !string.IsNullOrEmpty(target))
-            {
-                return FuzzySearch(target, source, DefaultMatchOption).Score;
-            }
-            else
-            {
-                return 0;
-            }
+            return FuzzySearch(target, source).Score;
         }
 
         [Obsolete("This method is obsolete and should not be used. Please use the static function StringMatcher.FuzzySearch")]
         public static bool IsMatch(string source, string target)
         {
-            return FuzzySearch(target, source, DefaultMatchOption).Score > 0;
+            return Score(source, target) > 0;
         }
 
         public static MatchResult FuzzySearch(string query, string stringToCompare)
         {
-            return FuzzySearch(query, stringToCompare, DefaultMatchOption);
+            return Instance.FuzzyMatch(query, stringToCompare);
+        }
+
+        public MatchResult FuzzyMatch(string query, string stringToCompare)
+        {
+            return FuzzyMatch(query, stringToCompare, _defaultMatchOption);
         }
 
         /// <summary>
@@ -51,12 +54,18 @@ public static MatchResult FuzzySearch(string query, string stringToCompare)
         /// 6. Move onto the next substring's characters until all substrings are checked.
         /// 7. Consider success and move onto scoring if every char or substring without whitespaces matched
         /// </summary>
-        public static MatchResult FuzzySearch(string query, string stringToCompare, MatchOption opt)
+        public MatchResult FuzzyMatch(string query, string stringToCompare, MatchOption opt)
         {
-            if (string.IsNullOrEmpty(stringToCompare) || string.IsNullOrEmpty(query)) return new MatchResult { Success = false };
+            if (string.IsNullOrEmpty(stringToCompare) || string.IsNullOrEmpty(query)) return new MatchResult (false, UserSettingSearchPrecision);
 
             query = query.Trim();
 
+            if (_alphabet != null)
+            {
+                query = _alphabet.Translate(query);
+                stringToCompare = _alphabet.Translate(stringToCompare);
+            }
+
             var fullStringToCompareWithoutCase = opt.IgnoreCase ? stringToCompare.ToLower() : stringToCompare;
 
             var queryWithoutCase = opt.IgnoreCase ? query.ToLower() : query;
@@ -139,19 +148,11 @@ public static MatchResult FuzzySearch(string query, string stringToCompare, Matc
             if (allQuerySubstringsMatched)
             {
                 var score = CalculateSearchScore(query, stringToCompare, firstMatchIndex, lastMatchIndex - firstMatchIndex, allSubstringsContainedInCompareString);
-                var pinyinScore = ScoreForPinyin(stringToCompare, query);
 
-                var result = new MatchResult
-                {
-                    Success = true,
-                    MatchData = indexList,
-                    RawScore = Math.Max(score, pinyinScore)
-                };
-
-                return result;
+                return new MatchResult(true, UserSettingSearchPrecision, indexList, score);
             }
 
-            return new MatchResult { Success = false };
+            return new MatchResult (false, UserSettingSearchPrecision);
         }
 
         private static bool AllPreviousCharsMatched(int startIndexToVerify, int currentQuerySubstringCharacterIndex, 
@@ -224,46 +225,28 @@ public enum SearchPrecisionScore
             Low = 20,
             None = 0
         }
+    }
 
-        public static int ScoreForPinyin(string source, string target)
+    public class MatchResult
+    {
+        public MatchResult(bool success, SearchPrecisionScore searchPrecision)
         {
-            if (!ShouldUsePinyin)
-            {
-                return 0;
-            }
+            Success = success;
+            SearchPrecision = searchPrecision;
+        }
 
-            if (!string.IsNullOrEmpty(source) && !string.IsNullOrEmpty(target))
-            {
-                if (Alphabet.ContainsChinese(source))
-                {
-                    var combination = Alphabet.PinyinComination(source);
-                    var pinyinScore = combination
-                        .Select(pinyin => FuzzySearch(target, string.Join("", pinyin)).Score)
-                        .Max();
-                    var acronymScore = combination.Select(Alphabet.Acronym)
-                        .Select(pinyin => FuzzySearch(target, pinyin).Score)
-                        .Max();
-                    var score = Math.Max(pinyinScore, acronymScore);
-                    return score;
-                }
-                else
-                {
-                    return 0;
-                }
-            }
-            else
-            {
-                return 0;
-            }
+        public MatchResult(bool success, SearchPrecisionScore searchPrecision, List<int> matchData, int rawScore)
+        {
+            Success = success;
+            SearchPrecision = searchPrecision;
+            MatchData = matchData;
+            RawScore = rawScore;
         }
-    }
 
-    public class MatchResult
-    {
         public bool Success { get; set; }
 
         /// <summary>
-        /// The final score of the match result with all search precision filters applied.
+        /// The final score of the match result with search precision filters applied.
         /// </summary>
         public int Score { get; private set; }
 
@@ -278,7 +261,7 @@ public int RawScore
             set
             {
                 _rawScore = value;
-                Score = ApplySearchPrecisionFilter(_rawScore);
+                Score = ScoreAfterSearchPrecisionFilter(_rawScore);
             }
         }
 
@@ -287,19 +270,21 @@ public int RawScore
         /// </summary>
         public List<int> MatchData { get; set; }
 
+        public SearchPrecisionScore SearchPrecision { get; set; }
+
         public bool IsSearchPrecisionScoreMet()
         {
-            return IsSearchPrecisionScoreMet(Score);
+            return IsSearchPrecisionScoreMet(_rawScore);
         }
 
-        private bool IsSearchPrecisionScoreMet(int score)
+        private bool IsSearchPrecisionScoreMet(int rawScore)
         {
-            return score >= (int)UserSettingSearchPrecision;
+            return rawScore >= (int)SearchPrecision;
         }
 
-        private int ApplySearchPrecisionFilter(int score)
+        private int ScoreAfterSearchPrecisionFilter(int rawScore)
         {
-            return IsSearchPrecisionScoreMet(score) ? score : 0;
+            return IsSearchPrecisionScoreMet(rawScore) ? rawScore : 0;
         }
     }
 
@@ -319,4 +304,4 @@ public class MatchOption
 
         public bool IgnoreCase { get; set; } = true;
     }
-}
+}

Wox.Infrastructure/UserSettings/Settings.cs

@@ -21,19 +21,11 @@ public class Settings : BaseModel
         public string ResultFontWeight { get; set; }
         public string ResultFontStretch { get; set; }
 
+
         /// <summary>
         /// when false Alphabet static service will always return empty results
         /// </summary>
-        private bool _shouldUsePinyin = true;
-        public bool ShouldUsePinyin 
-        {
-            get { return _shouldUsePinyin;  }
-            set 
-            {
-                _shouldUsePinyin = value;
-                StringMatcher.ShouldUsePinyin = value;
-            }
-        }
+        public bool ShouldUsePinyin { get; set; } = true;
 
 
         internal StringMatcher.SearchPrecisionScore QuerySearchPrecision { get; private set; } = StringMatcher.SearchPrecisionScore.Regular;
@@ -49,14 +41,14 @@ public string QuerySearchPrecisionString
                                             .Parse(typeof(StringMatcher.SearchPrecisionScore), value);
 
                     QuerySearchPrecision = precisionScore;
-                    StringMatcher.UserSettingSearchPrecision = precisionScore;
+                    StringMatcher.Instance.UserSettingSearchPrecision = precisionScore;
                 }
                 catch (ArgumentException e)
                 {
                     Logger.Log.Exception(nameof(Settings), "Failed to load QuerySearchPrecisionString value from Settings file", e);
 
                     QuerySearchPrecision = StringMatcher.SearchPrecisionScore.Regular;
-                    StringMatcher.UserSettingSearchPrecision = StringMatcher.SearchPrecisionScore.Regular;
+                    StringMatcher.Instance.UserSettingSearchPrecision = StringMatcher.SearchPrecisionScore.Regular;
 
                     throw;
                 }