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

Commit

Permalink
Handle case matches on special characters (extra boost).
Browse files Browse the repository at this point in the history 
This is a fix for #3263.
  • Loading branch information
@dangoor
dangoor committed Oct 21, 2014
1 parent 8fd477a commit 5b28a29
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/search/QuickOpen.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ define(function (require, exports, module) {
displayName += '<span title="sp:' + sd.special + ', m:' + sd.match +
', ls:' + sd.lastSegment + ', b:' + sd.beginning +
', ld:' + sd.lengthDeduction + ', c:' + sd.consecutive + ', nsos: ' +
sd.notStartingOnSpecial + '">(' + item.matchGoodness + ') </span>';
sd.notStartingOnSpecial + ', case: ' + sd.case + '">(' + item.matchGoodness + ') </span>';
}

// Put the path pieces together, highlighting the matched parts
Expand Down
50 changes: 33 additions & 17 deletions src/utils/StringMatch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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.<SpecialMatch|NormalMatch>} 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
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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--;
Expand Down Expand Up @@ -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++));
Expand Down Expand Up @@ -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.<SpecialMatch|NormalMatch>}} 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.
Expand All @@ -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) {
Expand Down Expand Up @@ -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.<SpecialMatch|NormalMatch>} 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;
Expand All @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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];
}
Expand Down Expand Up @@ -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;
Expand Down
46 changes: 37 additions & 9 deletions test/spec/StringMatch-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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)]);
Expand All @@ -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),
Expand Down Expand Up @@ -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 () {
Expand Down Expand Up @@ -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)
]
});
Expand Down Expand Up @@ -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)
Expand All @@ -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),
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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 () {
Expand Down

0 comments on commit 5b28a29

Please sign in to comment.