From 5b28a2961f1ac6f246236b742198aa92f886669a Mon Sep 17 00:00:00 2001 From: Kevin Dangoor Date: Tue, 21 Oct 2014 14:41:00 -0400 Subject: [PATCH] Handle case matches on special characters (extra boost). This is a fix for #3263. --- src/search/QuickOpen.js | 2 +- src/utils/StringMatch.js | 50 +++++++++++++++++++++++------------ test/spec/StringMatch-test.js | 46 +++++++++++++++++++++++++------- 3 files changed, 71 insertions(+), 27 deletions(-) diff --git a/src/search/QuickOpen.js b/src/search/QuickOpen.js index 15b753a5bbd..b6e3ee57ef4 100644 --- a/src/search/QuickOpen.js +++ b/src/search/QuickOpen.js @@ -680,7 +680,7 @@ define(function (require, exports, module) { displayName += '(' + item.matchGoodness + ') '; + sd.notStartingOnSpecial + ', case: ' + sd.case + '">(' + item.matchGoodness + ') '; } // Put the path pieces together, highlighting the matched parts diff --git a/src/utils/StringMatch.js b/src/utils/StringMatch.js index d40bd05d42a..fa95cdc241f 100644 --- a/src/utils/StringMatch.js +++ b/src/utils/StringMatch.js @@ -117,15 +117,20 @@ define(function (require, exports, module) { // Constants for scoring + var SPECIAL_CASE_POINTS = 50; // Both a Special and a Case match var SPECIAL_POINTS = 40; var MATCH_POINTS = 10; var MATCH_CASE_POINTS = 7; // Consecutive non-case matches have higher priority var CONSECUTIVE_MATCHES_POINTS = 8; - var BEGINNING_OF_NAME_POINTS = 10; + var BEGINNING_OF_NAME_POINTS = 13; var LAST_SEGMENT_BOOST = 1; var DEDUCTION_FOR_LENGTH = 0.2; var NOT_STARTING_ON_SPECIAL_PENALTY = 25; + function SpecialCaseMatch(index) { + this.index = index; + } + // Used in match lists to designate matches of "special" characters (see // findSpecialCharacters above function SpecialMatch(index) { @@ -213,12 +218,12 @@ define(function (require, exports, module) { * @param {string} query the search string (generally lower cased) * @param {string} str the string to compare with (generally lower cased) * @param {string} originalQuery the "non-normalized" query string (used to detect case match priority) - * @param {string} OriginalStr the "non-normalized" string to compare with (used to detect case match priority) + * @param {string} originalStr the "non-normalized" string to compare with (used to detect case match priority) * @param {Array} specials list of special indexes in str (from findSpecialCharacters) * @param {int} startingSpecial index into specials array to start scanning with * @return {Array.} matched indexes or null if no matches possible */ - function _generateMatchList(query, str, originalQuery, OriginalStr, specials, startingSpecial) { + function _generateMatchList(query, str, originalQuery, originalStr, specials, startingSpecial) { var result = []; // used to keep track of which special character we're testing now @@ -266,10 +271,15 @@ define(function (require, exports, module) { specialsCounter = i; } else if (query[queryCounter] === str[specials[i]]) { // we have a match! do the required tracking + strCounter = specials[i]; + if (originalQuery[queryCounter] === originalStr[strCounter]) { + result.push(new SpecialCaseMatch(strCounter)); + } else { + result.push(new SpecialMatch(strCounter)); + } specialsCounter = i; queryCounter++; - strCounter = specials[i]; - result.push(new SpecialMatch(strCounter++)); + strCounter++; return true; } } @@ -300,7 +310,7 @@ define(function (require, exports, module) { // searching from. queryCounter--; - if (item instanceof SpecialMatch) { + if (item instanceof SpecialMatch || item instanceof SpecialCaseMatch) { // pulled off a special, which means we need to make that special available // for matching again specialsCounter--; @@ -347,7 +357,7 @@ define(function (require, exports, module) { // we look character by character for matches if (query[queryCounter] === str[strCounter]) { // got a match! record it, and switch back to searching specials - if (originalQuery[queryCounter] === OriginalStr[strCounter]) { + if (originalQuery[queryCounter] === originalStr[strCounter]) { result.push(new CaseMatch(strCounter++)); } else { result.push(new NormalMatch(strCounter++)); @@ -392,13 +402,13 @@ define(function (require, exports, module) { * @param {string} query the search string (generally lower cased) * @param {string} str the string to compare with (generally lower cased) * @param {string} originalQuery the "non-normalized" query string (used to detect case match priority) - * @param {string} OriginalStr the "non-normalized" string to compare with (used to detect case match priority) + * @param {string} originalStr the "non-normalized" string to compare with (used to detect case match priority) * @param {Array} specials list of special indexes in str (from findSpecialCharacters) * @param {int} startingSpecial index into specials array to start scanning with * @param {boolean} lastSegmentStart which character does the last segment start at * @return {{remainder:int, matchList:Array.}} matched indexes or null if no matches possible */ - function _lastSegmentSearch(query, str, originalQuery, OriginalStr, specials, startingSpecial, lastSegmentStart) { + function _lastSegmentSearch(query, str, originalQuery, originalStr, specials, startingSpecial, lastSegmentStart) { var queryCounter, matchList; // It's possible that the query is longer than the last segment. @@ -417,7 +427,7 @@ define(function (require, exports, module) { for (queryCounter = 0; queryCounter < query.length; queryCounter++) { matchList = _generateMatchList(query.substring(queryCounter), str, originalQuery.substring(queryCounter), - OriginalStr, specials, startingSpecial); + originalStr, specials, startingSpecial); // if we've got a match *or* there are no segments in this string, we're done if (matchList || startingSpecial === 0) { @@ -445,17 +455,17 @@ define(function (require, exports, module) { * @param {string} queryLower the search string (will be searched lower case) * @param {string} compareLower the lower-cased string to search * @param {string} originalQuery the "non-normalized" query string (used to detect case match priority) - * @param {string} OriginalStr the "non-normalized" string to compare with (used to detect case match priority) + * @param {string} originalStr the "non-normalized" string to compare with (used to detect case match priority) * @param {Array} specials list of special indexes in str (from findSpecialCharacters) * @param {int} lastSegmentSpecialsIndex index into specials array to start scanning with * @return {Array.} matched indexes or null if no matches possible */ - function _wholeStringSearch(queryLower, compareLower, originalQuery, OriginalStr, specials, lastSegmentSpecialsIndex) { + function _wholeStringSearch(queryLower, compareLower, originalQuery, originalStr, specials, lastSegmentSpecialsIndex) { var lastSegmentStart = specials[lastSegmentSpecialsIndex]; var result; var matchList; - result = _lastSegmentSearch(queryLower, compareLower, originalQuery, OriginalStr, specials, lastSegmentSpecialsIndex, lastSegmentStart); + result = _lastSegmentSearch(queryLower, compareLower, originalQuery, originalStr, specials, lastSegmentSpecialsIndex, lastSegmentStart); if (result) { matchList = result.matchList; @@ -466,7 +476,7 @@ define(function (require, exports, module) { var remainderMatchList = _generateMatchList(result.remainder, compareLower.substring(0, lastSegmentStart), result.originalRemainder, - OriginalStr.substring(0, lastSegmentStart), + originalStr.substring(0, lastSegmentStart), specials.slice(0, lastSegmentSpecialsIndex), 0); if (remainderMatchList) { @@ -480,7 +490,7 @@ define(function (require, exports, module) { } else { // No match in the last segment, so we start over searching the whole // string - matchList = _generateMatchList(queryLower, compareLower, originalQuery, OriginalStr, specials, 0); + matchList = _generateMatchList(queryLower, compareLower, originalQuery, originalStr, specials, 0); } return matchList; @@ -571,7 +581,7 @@ define(function (require, exports, module) { } newPoints += MATCH_POINTS; - if (match instanceof CaseMatch) { + if (match instanceof CaseMatch || match instanceof SpecialCaseMatch) { if (DEBUG_SCORES) { scoreDebug.case += MATCH_CASE_POINTS; } @@ -625,6 +635,11 @@ define(function (require, exports, module) { scoreDebug.special += SPECIAL_POINTS; } newPoints += SPECIAL_POINTS; + } else if (match instanceof SpecialCaseMatch) { + if (DEBUG_SCORES) { + scoreDebug.special += SPECIAL_CASE_POINTS; + } + newPoints += SPECIAL_CASE_POINTS; } score += newPoints; @@ -651,7 +666,7 @@ define(function (require, exports, module) { // Check to see if this new matched range is starting on a special // character. We penalize those ranges that don't, because most // people will search on the logical boundaries of the name - currentRangeStartedOnSpecial = match instanceof SpecialMatch; + currentRangeStartedOnSpecial = match instanceof SpecialMatch || match instanceof SpecialCaseMatch; } else { currentRange.text += str[c]; } @@ -975,6 +990,7 @@ define(function (require, exports, module) { exports._setDebugScores = _setDebugScores; exports._generateMatchList = _generateMatchList; exports._SpecialMatch = SpecialMatch; + exports._SpecialCaseMatch = SpecialCaseMatch; exports._NormalMatch = NormalMatch; exports._CaseMatch = CaseMatch; exports._computeRangesAndScore = _computeRangesAndScore; diff --git a/test/spec/StringMatch-test.js b/test/spec/StringMatch-test.js index edeff0e34f9..c9f3a45242e 100644 --- a/test/spec/StringMatch-test.js +++ b/test/spec/StringMatch-test.js @@ -63,17 +63,22 @@ define(function (require, exports, module) { var generateMatchList = StringMatch._generateMatchList; var SpecialMatch = StringMatch._SpecialMatch; + var SpecialCaseMatch = StringMatch._SpecialCaseMatch; var NormalMatch = StringMatch._NormalMatch; var CaseMatch = StringMatch._CaseMatch; beforeEach(function () { SpecialMatch.prototype.type = "special"; NormalMatch.prototype.type = "normal"; + CaseMatch.prototype.type = "case"; + SpecialCaseMatch.prototype.type = "specialCase"; }); afterEach(function () { delete SpecialMatch.prototype.type; delete NormalMatch.prototype.type; + delete SpecialCaseMatch.prototype.type; + delete CaseMatch.prototype.type; }); var path = "src/document/DocumentCommandHandler.js"; @@ -145,7 +150,7 @@ define(function (require, exports, module) { expect(result).toEqual([new SpecialMatch(0), new CaseMatch(1), new CaseMatch(2), new CaseMatch(3), new CaseMatch(4), new CaseMatch(5), new CaseMatch(6)]); result = generateMatchList("abcdefz", btpathLower, "ABCDEFZ", btpath, btspecials.specials, 0); - expect(result).toEqual([new SpecialMatch(0), new NormalMatch(1), new NormalMatch(2), new NormalMatch(3), new NormalMatch(4), new NormalMatch(5), new NormalMatch(6)]); + expect(result).toEqual([new SpecialCaseMatch(0), new NormalMatch(1), new NormalMatch(2), new NormalMatch(3), new NormalMatch(4), new NormalMatch(5), new NormalMatch(6)]); result = generateMatchList("abcdefe", btpathLower, "abcdefe", btpath, btspecials.specials, 0); expect(result).toEqual([new SpecialMatch(0), new SpecialMatch(7), new SpecialMatch(12), new SpecialMatch(16), new CaseMatch(17), new CaseMatch(18), new SpecialMatch(19)]); @@ -155,7 +160,7 @@ define(function (require, exports, module) { var strLower = str.toLowerCase(); result = generateMatchList("_computerangesa", strLower, "_computerangesa", str, strSpecials.specials, 0); expect(result).toEqual([ - new SpecialMatch(0), new SpecialMatch(1), new CaseMatch(2), + new SpecialCaseMatch(0), new SpecialCaseMatch(1), new CaseMatch(2), new CaseMatch(3), new CaseMatch(4), new CaseMatch(5), new CaseMatch(6), new CaseMatch(7), new SpecialMatch(8), new CaseMatch(9), new CaseMatch(10), new CaseMatch(11), @@ -198,16 +203,20 @@ define(function (require, exports, module) { var SpecialMatch = StringMatch._SpecialMatch; var NormalMatch = StringMatch._NormalMatch; var CaseMatch = StringMatch._CaseMatch; + var SpecialCaseMatch = StringMatch._SpecialCaseMatch; + beforeEach(function () { SpecialMatch.prototype.type = "special"; NormalMatch.prototype.type = "normal"; CaseMatch.prototype.type = "case"; + SpecialCaseMatch.prototype.type = "specialCase"; }); afterEach(function () { delete SpecialMatch.prototype.type; delete NormalMatch.prototype.type; delete CaseMatch.prototype.type; + delete SpecialCaseMatch.prototype.type; }); it("should compare results in the final segment properly", function () { @@ -274,8 +283,8 @@ define(function (require, exports, module) { new CaseMatch(15), new SpecialMatch(21), new SpecialMatch(28), - new SpecialMatch(35), - new SpecialMatch(36), + new SpecialCaseMatch(35), + new SpecialCaseMatch(36), new CaseMatch(37) ] }); @@ -336,7 +345,7 @@ define(function (require, exports, module) { var comparePath = path.toLowerCase(); expect(wholeStringSearch("sdoc", comparePath, "sdoc", path, sc.specials, sc.lastSegmentSpecialsIndex)).toEqual([ - new SpecialMatch(0), + new SpecialCaseMatch(0), new SpecialMatch(13), new CaseMatch(14), new SpecialMatch(21) @@ -351,7 +360,7 @@ define(function (require, exports, module) { expect(wholeStringSearch("z", comparePath, "z", path, sc.specials, sc.lastSegmentSpecialsIndex)).toEqual(null); expect(wholeStringSearch("docdoc", comparePath, "docdoc", path, sc.specials, sc.lastSegmentSpecialsIndex)).toEqual([ - new SpecialMatch(4), + new SpecialCaseMatch(4), new CaseMatch(5), new CaseMatch(6), new SpecialMatch(13), @@ -580,9 +589,9 @@ define(function (require, exports, module) { it("should place QuickOpen well relative to other quicks", function () { expect(goodRelativeOrdering("quick", [ + "samples/root/Getting Started/screenshots/quick-edit.png", "src/search/QuickOpen.js", "test/spec/QuickOpen-test.js", - "samples/root/Getting Started/screenshots/quick-edit.png", "src/extensions/default/QuickOpenCSS/main.js" ])).toBe(true); }); @@ -673,15 +682,34 @@ define(function (require, exports, module) { ])).toBe(true); expect(goodRelativeOrdering("st", [ "str", - "String", "stringMatch", - "StringMatcher", "screenTop", "scrollTo", "setTimeout", + "String", + "StringMatcher", "switch" ])).toBe(true); }); + + it("should have good ordering with case matches", function () { + expect(goodRelativeOrdering("func", [ + "function", + "Function" + ])).toBe(true); + expect(goodRelativeOrdering("Func", [ + "Function", + "function" + ])).toBe(true); + expect(goodRelativeOrdering("Pack", [ + "Package.js", + "package.json" + ])).toBe(true); + expect(goodRelativeOrdering("Pack", [ + "src/extensibility/Package.js", + "src/node/package.json" + ])).toBe(true); + }); }); describe("scoring", function () {