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

Fixes #2608 (Quick Open shows red on first use) #3184

Merged
merged 5 commits into from
Apr 4, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 11 additions & 7 deletions src/search/QuickOpen.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,10 @@ define(function (require, exports, module) {
* list items are re-rendered. Both happen synchronously just after we return. Called even when results is empty.
*/
QuickNavigateDialog.prototype._handleResultsReady = function (e, results) {
// Give visual clue when there are no results
var isNoResults = (results.length === 0 && !this._isValidLineNumberQuery(this.$searchField.val()));
this.$searchField.toggleClass("no-results", isNoResults);
// Give visual clue when there are no results (unless we're in "Go To Line" mode, where there
// are never results, or we're in file search mode and waiting for the index to get rebuilt)
var isNoResults = (results.length === 0 && (fileList || currentPlugin) && !this._isValidLineNumberQuery(this.$searchField.val()));
this.$searchField.toggleClass("no-results", Boolean(isNoResults));
};

/**
Expand Down Expand Up @@ -786,12 +787,12 @@ define(function (require, exports, module) {

// Start fetching the file list, which will be needed the first time the user enters an un-prefixed query. If FileIndexManager's
// caches are out of date, this list might take some time to asynchronously build. See searchFileList() for how this is handled.
fileList = null;
fileListPromise = FileIndexManager.getFileInfoList("all")
.done(function (files) {
fileList = files;
fileListPromise = null;
});
this._filenameMatcher.reset();
}.bind(this));
Copy link
Member

Choose a reason for hiding this comment

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

So if I understand this right, nothing will actually refresh the results list once the new index has been built... but the next time the user changes the query text, even within the same Quick Open session, the results will start reflecting the new index?

Copy link
Member

Choose a reason for hiding this comment

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

Also... do we need both this _filenameMatcher.reset() and matcher.reset() above? I think they will always be the same object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right that there's no automatic refresh until the user changes the query string. You're also right that only one call to .reset() is needed.

Do you think it's worthwhile to force a refresh when the file list updates? It's not immediately obvious to me how we'd do that, but I'm guessing there's got to be a way we can prod the smartautocomplete widget into acting as if the query changed.

Copy link
Member

Choose a reason for hiding this comment

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

No, I don't think we need to bother with a forced refresh... just wanted to make sure I understood the intent of the code.

};

function getCurrentEditorSelectedText() {
Expand Down Expand Up @@ -833,8 +834,11 @@ define(function (require, exports, module) {
beginSearch("@", getCurrentEditorSelectedText());
}
}



// Listen for a change of project to invalidate our file list
$(ProjectManager).on("projectOpen", function () {
fileList = null;
});

// TODO: allow QuickOpenJS to register it's own commands and key bindings
CommandManager.register(Strings.CMD_QUICK_OPEN, Commands.NAVIGATE_QUICK_OPEN, doFileSearch);
Expand Down
17 changes: 12 additions & 5 deletions src/utils/StringMatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -761,11 +761,7 @@ define(function (require, exports, module) {
* (This object's caches are all stored in "_" prefixed properties.)
*/
function StringMatcher() {
// We keep track of the last query to know when we need to invalidate.
this._lastQuery = null;

this._specialsCache = {};
this._noMatchCache = {};
this.reset();
}

/**
Expand All @@ -782,6 +778,17 @@ define(function (require, exports, module) {
*/
StringMatcher.prototype._noMatchCache = null;

/**
* Clears the caches. Use this in the event that the caches may be invalid.
*/
StringMatcher.prototype.reset = function () {
// We keep track of the last query to know when we need to invalidate.
this._lastQuery = null;

this._specialsCache = {};
this._noMatchCache = {};
};

/**
* Performs a single match using the stringMatch function. See stringMatch for full documentation.
*
Expand Down
16 changes: 14 additions & 2 deletions test/spec/StringMatch-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ define(function (require, exports, module) {
});

describe("StringMatcher", function () {
it("should manage its caches properly", function () {
beforeEach(function () {
this.addMatchers({
toBeInCache: function (matcher, cacheName) {
var value = matcher[cacheName][this.actual];
Expand All @@ -607,7 +607,9 @@ define(function (require, exports, module) {
return value !== undefined;
}
});

});

it("should manage its caches properly", function () {
var matcher = new StringMatch.StringMatcher();
expect(matcher._noMatchCache).toEqual({});
expect(matcher._specialsCache).toEqual({});
Expand Down Expand Up @@ -642,6 +644,16 @@ define(function (require, exports, module) {
var lengthResult = matcher.match("length", "l");
expect(lengthResult).toBeTruthy();
});

it("can reset the caches", function () {
var matcher = new StringMatch.StringMatcher();
matcher.match("foo", "spec/live");
expect("foo").toBeInCache(matcher, "_specialsCache");
expect("foo").toBeInCache(matcher, "_noMatchCache");
matcher.reset();
expect("foo").not.toBeInCache(matcher, "_specialsCache");
expect("foo").not.toBeInCache(matcher, "_noMatchCache");
});
});
});
});