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

Commit

Permalink
Rename singleSegmentSearch StringMatch option to segmentedSearch. Def…
Browse files Browse the repository at this point in the history
…aults to false

(which is a change in behavior for quick navigate that should actually be fine for
those uses). Also augmented the test cases based on review feedback.
  • Loading branch information
dangoor committed May 13, 2013
1 parent 89ba092 commit b6792bd
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 26 deletions.
4 changes: 3 additions & 1 deletion src/search/QuickOpen.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,9 @@ define(function (require, exports, module) {
this._resultsFormatterCallback = this._resultsFormatterCallback.bind(this);

// StringMatchers that cache in-progress query data.
this._filenameMatcher = new StringMatch.StringMatcher();
this._filenameMatcher = new StringMatch.StringMatcher({
segmentedSearch: true
});
this._matchers = {};
}

Expand Down
29 changes: 15 additions & 14 deletions src/utils/StringMatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -687,8 +687,8 @@ define(function (require, exports, module) {

/*
* Match str against the query using the QuickOpen algorithm provided by
* the functions above. The general idea is to prefer matches in the last
* segment (the filename) and matches of "special" characters. stringMatch
* the functions above. The general idea is to prefer matches of "special" characters and,
* optionally, matches that occur in the "last segment" (generally, the filename). stringMatch
* will try to provide the best match and produces a "matchGoodness" score
* to allow for relative ranking.
*
Expand All @@ -701,10 +701,11 @@ define(function (require, exports, module) {
*
* @param {string} str The string to search
* @param {string} query The query string to find in string
* @param {{preferPrefixMatches:?boolean, singleSegmentSearch:?boolean}} options to control search behavior.
* @param {{preferPrefixMatches:?boolean, segmentedSearch:?boolean}} options to control search behavior.
* preferPrefixMatches puts an exact case-insensitive prefix match ahead of all other matches,
* even short-circuiting the match logic. This option implies singleSegmentSearch.
* singleSegmentSearch does not treat segments of the string specially.
* even short-circuiting the match logic. This option implies segmentedSearch=false.
* When segmentedSearch is true, the string is broken into segments by "/" characters
* and the last segment is searched first and matches there are scored higher.
* @param {?Object} special (optional) the specials data from findSpecialCharacters, if already known
* This is generally just used by StringMatcher for optimization.
* @return {{ranges:Array.<{text:string, matched:boolean, includesLastSegment:boolean}>, matchGoodness:int, scoreDebug: Object}} matched ranges and score
Expand Down Expand Up @@ -735,7 +736,7 @@ define(function (require, exports, module) {
var compareStr = str.toLowerCase();

if (options.preferPrefixMatches) {
options.singleSegmentSearch = true;
options.segmentedSearch = false;
}

if (options.preferPrefixMatches && compareStr.substr(0, query.length) === query) {
Expand All @@ -751,14 +752,14 @@ define(function (require, exports, module) {

// For strings that are not broken into multiple segments, we can potentially
// avoid some extra work
if (options.singleSegmentSearch) {
lastSegmentStart = 0;
matchList = _generateMatchList(query, compareStr, special.specials,
0);
} else {
if (options.segmentedSearch) {
lastSegmentStart = special.specials[special.lastSegmentSpecialsIndex];
matchList = _wholeStringSearch(query, compareStr, special.specials,
special.lastSegmentSpecialsIndex);
} else {
lastSegmentStart = 0;
matchList = _generateMatchList(query, compareStr, special.specials,
0);
}

// If we get a match, turn this into a SearchResult as expected by the consumers
Expand Down Expand Up @@ -833,10 +834,10 @@ define(function (require, exports, module) {
* You are free to store other data on this object to assist in higher-level caching.
* (This object's caches are all stored in "_" prefixed properties.)
*
* @param {{preferPrefixMatches:?boolean, singleSegmentSearch:?boolean}} options to control search behavior.
* @param {{preferPrefixMatches:?boolean, segmentedSearch:?boolean}} options to control search behavior.
* preferPrefixMatches puts an exact case-insensitive prefix match ahead of all other matches,
* even short-circuiting the match logic. This option implies singleSegmentSearch.
* singleSegmentSearch does not treat segments of the string specially.
* even short-circuiting the match logic. This option implies segmentedSearch=false.
* segmentedSearch treats segments of the string specially.
*/
function StringMatcher(options) {
this.options = options;
Expand Down
71 changes: 60 additions & 11 deletions test/spec/StringMatch-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ define(function (require, exports, module) {
ranges = result.stringRanges;
expect(ranges.length).toBe(3);

expect(stringMatch("src/search/QuickOpen.js", "qo")).toEqual({
expect(stringMatch("src/search/QuickOpen.js", "qo", { segmentedSearch: true })).toEqual({
matchGoodness: jasmine.any(Number),
label: "src/search/QuickOpen.js",
stringRanges: [
Expand All @@ -443,7 +443,7 @@ define(function (require, exports, module) {
});

it("should prefer special characters", function () {
expect(stringMatch("src/document/DocumentCommandHandler.js", "dch")).toEqual({
expect(stringMatch("src/document/DocumentCommandHandler.js", "dch", { segmentedSearch: true })).toEqual({
matchGoodness: jasmine.any(Number),
label: "src/document/DocumentCommandHandler.js",
stringRanges: [
Expand Down Expand Up @@ -502,31 +502,52 @@ define(function (require, exports, module) {
{ text: "/foo/bar/src.js", matched: false, includesLastSegment: true }
]
});

var result = stringMatch("src/foo/bar/src.js", "fbs", {
preferPrefixMatches: true
});

expect(result).toEqual({
matchGoodness: jasmine.any(Number),
label: "src/foo/bar/src.js",
stringRanges: [
{ text: "src/", matched: false, includesLastSegment: true },
{ text: "f", matched: true, includesLastSegment: true },
{ text: "oo/", matched: false, includesLastSegment: true },
{ text: "b", matched: true, includesLastSegment: true },
{ text: "ar/", matched: false, includesLastSegment: true },
{ text: "s", matched: true, includesLastSegment: true },
{ text: "rc.js", matched: false, includesLastSegment: true }
]
});

expect(result.matchGoodness).toBeGreaterThan(-Number.MAX_VALUE);

expect(stringMatch("long", "longerQuery", {
preferPrefixMatches: true
})).toEqual(null);
});

it("should optionally allow single segment matches", function () {
expect(stringMatch("brackets/utils/brackets.js", "brackut", {
singleSegmentSearch: true
})).toEqual({
it("should default to single segment matches", function () {
var expectedResult = {
matchGoodness: jasmine.any(Number),
label: "brackets/utils/brackets.js",
stringRanges: [
{ text: "brack", matched: true, includesLastSegment: true },
{ text: "ets/", matched: false, includesLastSegment: true },
{ text: "ut", matched: true, includesLastSegment: true },
{ text: "ils/brackets.js", matched: false, includesLastSegment: true }
{ text: "ets/utils/brackets.js", matched: false, includesLastSegment: true }
]
});
};

expect(stringMatch("brackets/utils/brackets.js", "brack")).toEqual(expectedResult);

expect(stringMatch("brackets/utils/brackets.js", "brack", { segmentedSearch: false })).toEqual(expectedResult);
});

var goodRelativeOrdering = function (query, testStrings) {
var lastScore = -Infinity;
var goodOrdering = true;
testStrings.forEach(function (str) {
var result = stringMatch(str, query);
var result = stringMatch(str, query, { segmentedSearch: true });

// note that matchGoodness is expressed in negative numbers
if (result.matchGoodness < lastScore) {
Expand Down Expand Up @@ -761,6 +782,34 @@ define(function (require, exports, module) {
);
});

it("should pass the segmentedSearch option", function () {
var matcher = new StringMatch.StringMatcher({
segmentedSearch: false
});

expect(matcher.match("brackets/utils/brackets.js", "brack")).toEqual({
matchGoodness: jasmine.any(Number),
label: "brackets/utils/brackets.js",
stringRanges: [
{ text: "brack", matched: true, includesLastSegment: true },
{ text: "ets/utils/brackets.js", matched: false, includesLastSegment: true }
]
});

matcher = new StringMatch.StringMatcher({
segmentedSearch: true
});

expect(matcher.match("brackets/utils/brackets.js", "brack")).toEqual({
matchGoodness: jasmine.any(Number),
label: "brackets/utils/brackets.js",
stringRanges: [
{ text: "brackets/utils/", matched: false, includesLastSegment: false },
{ text: "brack", matched: true, includesLastSegment: true },
{ text: "ets.js", matched: false, includesLastSegment: true }
]
});
});
});
});
});

0 comments on commit b6792bd

Please sign in to comment.