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

Increase the boost for prefix matches. #3493

Merged
merged 3 commits into from
Apr 24, 2013
Merged

Increase the boost for prefix matches. #3493

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b267420
Increase the boost for prefix matches.
dangoor Apr 19, 2013
c667fcb
Make numConsecutive actually mean what it says.
dangoor Apr 23, 2013
1552890
Extract consecutive boost value to variable.
dangoor Apr 24, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/utils/StringMatch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ define(function (require, exports, module) {
var SPECIAL_POINTS = 35;
var MATCH_POINTS = 10;
var LAST_SEGMENT_BOOST = 1;
var BEGINNING_OF_NAME_POINTS = 25;
var BEGINNING_OF_NAME_POINTS = 10;
var DEDUCTION_FOR_LENGTH = 0.2;
var CONSECUTIVE_MATCHES_POINTS = 10;
var CONSECUTIVE_MATCHES_POINTS = 7;
var NOT_STARTING_ON_SPECIAL_PENALTY = 25;

// Used in match lists to designate matches of "special" characters (see
Expand Down Expand Up @@ -562,10 +562,24 @@ define(function (require, exports, module) {
// handles the initial value of lastMatchIndex which is used for
// constructing ranges but we don't yet have a true match.
if (score > 0 && lastMatchIndex + 1 === c) {
// Continue boosting for each additional match at the beginning
// of the name
if (c - numConsecutive === lastSegmentStart) {
if (DEBUG_SCORES) {
scoreDebug.beginning += BEGINNING_OF_NAME_POINTS;
}
newPoints += BEGINNING_OF_NAME_POINTS;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: the order of these two additions is the opposite of the other pairs here.

Copy link
Member

Choose a reason for hiding this comment

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

Oops sorry, that comment was intended for the next block of code, on line 579...

}
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little confused about how to interpret numConsecutive. If I'm reading the code correctly, it only means "number of consecutive characters" if the consecutive run started on a special char -- otherwise it is always 1.

I think the code here still works correctly, since segment start is always a special (I can't think of any way to get a false positive or negative). But it does make the code more confusing.

What if we change it so numConsecutive is always incremented, so it means what it says, and then have something like:

var pointMultiplier = currentRangeStartedOnSpecial ? numConsecutive : 1;
...
newPoints += CONSECUTIVE_MATCHES_POINTS * pointMultiplier;


numConsecutive++;

// Consecutive matches that started on a special are a
// good indicator of intent, so we award an added bonus there.
if (currentRangeStartedOnSpecial) {
numConsecutive++;
newPoints += CONSECUTIVE_MATCHES_POINTS * numConsecutive;
if (DEBUG_SCORES) {
scoreDebug.consecutive += CONSECUTIVE_MATCHES_POINTS * numConsecutive;
}
Copy link
Member

Choose a reason for hiding this comment

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

If I'm reading this right, this is basically adding the same bonus as the always case, just a second time. Would it read more clearly to just store CONSECUTIVE_MATCHES_POINTS * numConsecutive in a var, double it if (currentRangeStartedOnSpecial), and then have a shared block of code to do the +=s?

}

if (DEBUG_SCORES) {
Expand Down
28 changes: 26 additions & 2 deletions test/spec/StringMatch-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,32 @@ define(function (require, exports, module) {

it("should find the right jsu", function () {
expect(goodRelativeOrdering("jsu", [
"src/language/JSLintUtils.js",
"src/language/JSUtil.js"
"src/language/JSUtil.js",
"src/language/JSLintUtils.js"
])).toBe(true);
});

it("should find the right trange", function () {
expect(goodRelativeOrdering("trange", [
"src/document/TextRange.js",
"src/extensions/default/JavaScriptQuickEdit/unittest-files/jquery-ui/demos/slider/range.html"
])).toBe(true);
});

it("should prefer prefix matches", function () {
expect(goodRelativeOrdering("asc", [
"ASC.js",
"ActionScriptCompiler.js"
])).toBe(true);
expect(goodRelativeOrdering("st", [
"str",
"String",
"stringMatch",
"StringMatcher",
"screenTop",
"scrollTo",
"setTimeout",
"switch"
])).toBe(true);
});
});
Expand Down