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

Adds option for strongly preferred prefix and single segment search. #3588

Merged
merged 6 commits into from
May 14, 2013
Merged

Adds option for strongly preferred prefix and single segment search. #3588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fca72ea
Adds option for strongly preferred prefix and single segment search.
dangoor Apr 24, 2013
232ee9b
Turn on preferPrefixMatches for JS code hints.
dangoor Apr 24, 2013
0178539
Extract function to create prefix search result.
dangoor Apr 25, 2013
89ba092
Merge branch 'master' into dangoor/prefer-prefix
dangoor May 13, 2013
b6792bd
Rename singleSegmentSearch StringMatch option to segmentedSearch. Def…
dangoor May 13, 2013
13a0077
Quick Open plugins can include matcherOptions to set StringMatch options
dangoor May 14, 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
5 changes: 3 additions & 2 deletions src/extensions/default/JavaScriptCodeHints/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,9 @@ define(function (require, exports, module) {
var $deferredHints = $.Deferred();
scopeResponse.promise.done(function () {
cachedType = session.getType();

matcher = new StringMatch.StringMatcher();
matcher = new StringMatch.StringMatcher({
preferPrefixMatches: true
});
cachedHints = session.getHints(query, matcher);

if ($deferredHints.state() === "pending") {
Expand Down
15 changes: 11 additions & 4 deletions src/search/QuickOpen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ define(function (require, exports, module) {
/**
* Defines API for new QuickOpen plug-ins
*/
function QuickOpenPlugin(name, languageIds, done, search, match, itemFocus, itemSelect, resultsFormatter) {
function QuickOpenPlugin(name, languageIds, done, search, match, itemFocus, itemSelect, resultsFormatter, matcherOptions) {
this.name = name;
this.languageIds = languageIds;
this.done = done;
Expand All @@ -107,6 +107,7 @@ define(function (require, exports, module) {
this.itemFocus = itemFocus;
this.itemSelect = itemSelect;
this.resultsFormatter = resultsFormatter;
this.matcherOptions = matcherOptions;
}

/**
Expand All @@ -120,6 +121,7 @@ define(function (require, exports, module) {
* itemFocus: function(?SearchResult|string),
* itemSelect: funciton(?SearchResult|string),
* resultsFormatter: ?function(SearchResult|string, string):string
* matcherOptions: Object
* } pluginDef
*
* Parameter Documentation:
Expand All @@ -137,6 +139,8 @@ define(function (require, exports, module) {
* The selected search result item (as returned by search()) is passed as an argument.
* resultFormatter - takes a query string and an item string and returns
* a <LI> item to insert into the displayed search results. If null, default is provided.
* matcherOptions - options to pass along to the StringMatcher (see StringMatch.StringMatcher
* for available options)
*
* If itemFocus() makes changes to the current document or cursor/scroll position and then the user
* cancels Quick Open (via Esc), those changes are automatically reverted.
Expand All @@ -159,7 +163,8 @@ define(function (require, exports, module) {
pluginDef.match,
pluginDef.itemFocus,
pluginDef.itemSelect,
pluginDef.resultsFormatter
pluginDef.resultsFormatter,
pluginDef.matcherOptions
));
}

Expand All @@ -184,7 +189,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 Expand Up @@ -552,7 +559,7 @@ define(function (require, exports, module) {
// Look up the StringMatcher for this plugin.
var matcher = this._matchers[currentPlugin.name];
if (!matcher) {
matcher = new StringMatch.StringMatcher();
matcher = new StringMatch.StringMatcher(plugin.matcherOptions);
this._matchers[currentPlugin.name] = matcher;
}
return plugin.search(query, matcher);
Expand Down
101 changes: 84 additions & 17 deletions src/utils/StringMatch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,16 +421,12 @@ define(function (require, exports, module) {
* The parameters and return value are the same as for getMatchRanges.
*
* @param {string} query the search string (will be searched lower case)
* @param {string} str the original string to search
* @param {string} compareStr the lower-cased string to search
* @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(query, str, specials, lastSegmentSpecialsIndex) {
// set up query as all lower case and make a lower case string to use for comparisons
query = query.toLowerCase();
var compareStr = str.toLowerCase();

function _wholeStringSearch(query, compareStr, specials, lastSegmentSpecialsIndex) {
var lastSegmentStart = specials[lastSegmentSpecialsIndex];
var result;
var matchList;
Expand Down Expand Up @@ -458,7 +454,7 @@ define(function (require, exports, module) {
} else {
// No match in the last segment, so we start over searching the whole
// string
matchList = _generateMatchList(query, compareStr, specials, 0, lastSegmentStart);
matchList = _generateMatchList(query, compareStr, specials, 0);
}

return matchList;
Expand Down Expand Up @@ -653,11 +649,46 @@ define(function (require, exports, module) {
}
return result;
}


/*
* If we short circuit normal matching to produce a prefix match,
* this function will generate the appropriate SearchResult.
* This function assumes that the prefix match check has already
* been performed.
*
* @param {string} str The string with the prefix match for the query
* @param {string} query The query that matched the beginning of str
* @return {{ranges:Array.<{text:string, matched:boolean, includesLastSegment:boolean}>, matchGoodness:int, scoreDebug: Object}} ranges has a matching range for beginning of str
* and a non-matching range for the end of the str
* the score is -Number.MAX_VALUE in all cases
*/
function _prefixMatchResult(str, query) {
var result = new SearchResult(str);
result.matchGoodness = -Number.MAX_VALUE;
if (DEBUG_SCORES) {
result.scoreDebug = {
beginning: Number.MAX_VALUE
};
}
result.stringRanges = [{
text: str.substr(0, query.length),
matched: true,
includesLastSegment: true
}];
if (str.length > query.length) {
result.stringRanges.push({
text: str.substring(query.length),
matched: false,
includesLastSegment: true
});
}
return result;
}

/*
* 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 @@ -670,12 +701,20 @@ define(function (require, exports, module) {
*
* @param {string} str The string to search
* @param {string} query The query string to find in string
* @param {?Object} (optional) the specials data from findSpecialCharacters, if already known
* @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 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
*/
function stringMatch(str, query, special) {
function stringMatch(str, query, options, special) {
Copy link
Member

Choose a reason for hiding this comment

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

This is public API, so it would be better to add the new arg at the end for backwards compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're probably right, though I made this choice because the special parameter is just a speed optimization used by StringMatcher and is not likely used elsewhere. options, on the other hand, is likely to be used.

I could keep the new signature and deprecate the old by looking for the presence of special.specials. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Good point -- in light of that, it's fine by me as-is

var result;


options = options || {};

// No query? Short circuit the normal work done and just
// return a single range that covers the whole string.
if (!query) {
Expand All @@ -692,14 +731,36 @@ define(function (require, exports, module) {
return result;
}

// comparisons are case insensitive, so switch to lower case here
query = query.toLowerCase();
var compareStr = str.toLowerCase();

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

if (options.preferPrefixMatches && compareStr.substr(0, query.length) === query) {
return _prefixMatchResult(str, query);
}

// Locate the special characters and then use orderedCompare to do the real
// work.
if (!special) {
special = findSpecialCharacters(str);
}
var lastSegmentStart = special.specials[special.lastSegmentSpecialsIndex];
var matchList = _wholeStringSearch(query, str, special.specials,
var lastSegmentStart, matchList;

// For strings that are not broken into multiple segments, we can potentially
// avoid some extra work
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
// of this API.
Expand Down Expand Up @@ -772,8 +833,14 @@ 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, 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 segmentedSearch=false.
* segmentedSearch treats segments of the string specially.
*/
function StringMatcher() {
function StringMatcher(options) {
this.options = options;
Copy link
Member

Choose a reason for hiding this comment

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

It might be useful for some QuickOpen providers to be able to set these options too -- singleSegmentSearch would be useful for almost all of them. But they don't control the new'ing of StringMatcher. I wonder if we could document some official way for them to set options -- e.g. munging matcher.options is officially allowed as long as you do it before the first call to match()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at addQuickOpenPlugin, QuickOpen providers could use their own matchers but, of course, that's pushing more work onto the plugin providers. What if pluginDef could include matcher options?

Also, would it make sense to make singleSegmentSearch the default behavior?

Copy link
Member

Choose a reason for hiding this comment

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

Passing options via pluginDef sounds like a good way to go.

Having singleSegmentSearch on by default also sounds good -- it is an API change, but I'm pretty sure all of the extensions that integrate with QuickOpen are searching symbols and not paths... so it would be an improvement for all of them. It seems fairly safe to do.

Copy link
Member

Choose a reason for hiding this comment

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

One other note on passing options: we should keep it fairly scalable so that other StringMatch settings could eventually be passed in, e.g. to control segmentation (maybe a plugin wants segmented-style search but split based on something other than "/"es, etc.).

this.reset();
}

Expand Down Expand Up @@ -831,7 +898,7 @@ define(function (require, exports, module) {
this._specialsCache[str] = special;
}

var result = stringMatch(str, query, special);
var result = stringMatch(str, query, this.options, special);

// If this query was not a match, we cache that fact for next time.
if (!result) {
Expand Down
Loading