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

Make JS code hint test runs more consistent #5904

Merged
merged 4 commits into from
Nov 11, 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
4 changes: 3 additions & 1 deletion src/extensions/default/JavaScriptCodeHints/MessageIds.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ define(function (require, exports, module) {
TERN_CALLED_FUNC_TYPE_MSG = "FunctionType",
TERN_PRIME_PUMP_MSG = "PrimePump",
TERN_GET_GUESSES_MSG = "GetGuesses",
TERN_WORKER_READY = "WorkerReady";
TERN_WORKER_READY = "WorkerReady",
SET_CONFIG = "SetConfig";

// Message parameter constants
var TERN_FILE_INFO_TYPE_PART = "part",
Expand All @@ -57,6 +58,7 @@ define(function (require, exports, module) {
exports.TERN_FILE_INFO_TYPE_PART = TERN_FILE_INFO_TYPE_PART;
exports.TERN_FILE_INFO_TYPE_FULL = TERN_FILE_INFO_TYPE_FULL;
exports.TERN_FILE_INFO_TYPE_EMPTY = TERN_FILE_INFO_TYPE_EMPTY;
exports.SET_CONFIG = SET_CONFIG;
});


102 changes: 88 additions & 14 deletions src/extensions/default/JavaScriptCodeHints/ScopeManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ define(function (require, exports, module) {
LARGE_LINE_CHANGE = 100,
LARGE_LINE_COUNT = 2000,
OFFSET_ZERO = {line: 0, ch: 0};


var config = {};

/**
* An array of library names that contain JavaScript builtins definitions.
*
Expand Down Expand Up @@ -168,7 +170,9 @@ define(function (require, exports, module) {
* the message will not be posted until initialization is complete
*/
function postMessage(msg) {
currentWorker.postMessage(msg);
if (currentWorker) {
currentWorker.postMessage(msg);
}
}

/**
Expand Down Expand Up @@ -778,6 +782,9 @@ define(function (require, exports, module) {
*/
function postMessage(msg) {
addFilesPromise.done(function (ternWorker) {
if (config.debug) {
console.debug("Sending message", msg);
Copy link
Member

Choose a reason for hiding this comment

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

Nit. Looks like console.debug is deprecated. https://developer.mozilla.org/en-US/docs/Web/API/console

Copy link
Member

Choose a reason for hiding this comment

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

The Chrome docs don't say that, though it sounds identical to regular .log() there too... https://developers.google.com/chrome-developer-tools/docs/console

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, Firebug hasn't deprecated console.debug either. I kind of liked debug because Chrome shows the debug messages in blue, making them a bit easier to pick out.

}
ternWorker.postMessage(msg);
});
}
Expand All @@ -788,6 +795,9 @@ define(function (require, exports, module) {
*/
function _postMessageByPass(msg) {
ternPromise.done(function (ternWorker) {
if (config.debug) {
console.debug("Sending message", msg);
}
ternWorker.postMessage(msg);
});
}
Expand Down Expand Up @@ -949,10 +959,15 @@ define(function (require, exports, module) {

numAddedFiles += files.length;
ternPromise.done(function (worker) {
worker.postMessage({
var msg = {
type : MessageIds.TERN_ADD_FILES_MSG,
files : files
});
};

if (config.debug) {
console.debug("Sending message", msg);
}
worker.postMessage(msg);
});

} else {
Expand Down Expand Up @@ -1059,6 +1074,10 @@ define(function (require, exports, module) {
_ternWorker = new Worker(path);

_ternWorker.addEventListener("message", function (e) {
if (config.debug) {
Copy link
Member

Choose a reason for hiding this comment

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

Another nit. Can we factor this out into another function to consolidate the config.debug test and log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could, especially for cases like this one. The reason I hadn't is that sometimes work is done to produce the log output and it's faster to just do the if first... when you always call another function to do the test then the logging, you can end up doing the heavier work and then throwing it away. It seemed simpler to just always wrap in the if(). That said, there's only one case of the extra work which is the _.pluck( to produce a readable hints list to the debug log. We shouldn't run that when debugging is off.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, no problem.

console.debug("Message received", e);
}

var response = e.data,
type = response.type;

Expand All @@ -1084,6 +1103,11 @@ define(function (require, exports, module) {
}
});

// Set the initial configuration for the worker
_ternWorker.postMessage({
type: MessageIds.SET_CONFIG,
config: config
});
}
/**
* Create a new tern server.
Expand All @@ -1096,12 +1120,17 @@ define(function (require, exports, module) {
numInitialFiles = files.length;

ternPromise.done(function (worker) {
worker.postMessage({
var msg = {
type : MessageIds.TERN_INIT_MSG,
dir : dir,
files : files,
env : ternEnvironment
});
};

if (config.debug) {
console.debug("Sending message", msg);
}
worker.postMessage(msg);
});
rootTernDir = dir + "/";
}
Expand Down Expand Up @@ -1259,7 +1288,7 @@ define(function (require, exports, module) {
return this;
}

var reseting = false;
var resettingDeferred = null;

/**
* reset the tern worker thread, if necessary.
Expand All @@ -1268,26 +1297,54 @@ define(function (require, exports, module) {
* the web worker instance, and start a new one. To avoid a performance
* hit when we do this we start up a new worker, and don't kill the old
* one unitl the new one is initialized.
*
* During debugging, you can turn this automatic resetting behavior off
* by running this in the console:
* brackets._configureJSCodeHints({ noReset: true })
*
* This function is also used in unit testing with the "force" flag to
* reset the worker for each test to start with a clean environment.
*
* @param {Session} session
* @param {Document} document
* @param {boolean} force true to force a reset regardless of how long since the last one
* @return {Promise} Promise resolved when the worker is ready.
* The new (or current, if there was no reset) worker is passed to the callback.
*/
function maybeReset(session, document) {
function _maybeReset(session, document, force) {
var newWorker;
// if we're in the middle of a reset, don't have to check
// the new worker will be online soon
if (!reseting) {
if (++_hintCount > MAX_HINTS) {
reseting = true;
if (!resettingDeferred) {

// We don't reset if the debugging flag is set
// because it's easier to debug if the worker isn't
// getting shut down all the time.
if (force || (!config.noReset && ++_hintCount > MAX_HINTS)) {
if (config.debug) {
console.debug("Resetting tern worker");
}

resettingDeferred = new $.Deferred();
newWorker = new TernWorker();
newWorker.handleEditorChange(session, document, null);
newWorker.whenReady(function () {
// tell the old worker to shut down
currentWorker.closeWorker();
currentWorker = newWorker;
resettingDeferred.resolve(currentWorker);
// all done reseting
reseting = false;
resettingDeferred = null;
});
_hintCount = 0;
} else {
var d = new $.Deferred();
d.resolve(currentWorker);
return d.promise();
}
}

return resettingDeferred.promise();
}

/**
Expand Down Expand Up @@ -1338,7 +1395,7 @@ define(function (require, exports, module) {
fileInfo = getFileInfo(session),
offset = getOffset(session, fileInfo, null);

maybeReset(session, document);
_maybeReset(session, document);

hintPromise = getTernHints(fileInfo, offset, sessionType.property);

Expand Down Expand Up @@ -1372,6 +1429,9 @@ define(function (require, exports, module) {
var changed = documentChanges;
if (changed === null) {
documentChanges = changed = {from: changeList.from.line, to: changeList.from.line};
if (config.debug) {
console.debug("ScopeManager: document has changed");
}
}

var end = changeList.from.line + (changeList.text.length - 1);
Expand Down Expand Up @@ -1438,12 +1498,26 @@ define(function (require, exports, module) {
initPreferences(projectRootPath);
}


/** Used to avoid timing bugs in unit tests */
function _readyPromise() {
return deferredPreferences;
}

/**
* @private
*
* Update the configuration in the worker.
*/
function _setConfig(configUpdate) {
config = brackets._configureJSCodeHints.config;
postMessage({
type: MessageIds.SET_CONFIG,
config: configUpdate
});
}

exports._setConfig = _setConfig;
exports._maybeReset = _maybeReset;
exports.getBuiltins = getBuiltins;
exports.getResolvedPath = getResolvedPath;
exports.getTernHints = getTernHints;
Expand Down
36 changes: 33 additions & 3 deletions src/extensions/default/JavaScriptCodeHints/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,27 @@ define(function (require, exports, module) {
matcher = null, // string matcher for hints
ignoreChange; // can ignore next "change" event if true;

/**
* Sets the configuration, generally for testing/debugging use.
* Configuration keys are merged into the current configuration.
* The Tern worker is automatically updated to the new config as well.
*
* * debug: Set to true if you want verbose logging
* * noReset: Set to true if you don't want the worker to restart periodically
*
* @param {Object} configUpdate keys/values to merge into the config
*/
function setConfig(configUpdate) {
var config = setConfig.config;
Object.keys(configUpdate).forEach(function (key) {
config[key] = configUpdate[key];
});

ScopeManager._setConfig(configUpdate);
}

setConfig.config = {};

/**
* Get the value of current session.
* Used for unit testing.
Expand All @@ -79,6 +100,10 @@ define(function (require, exports, module) {
var trimmedQuery,
formattedHints;

if (setConfig.config.debug) {
console.debug("Hints", _.pluck(hints, "label"));
}

/*
* Returns a formatted list of hints with the query substring
* highlighted.
Expand Down Expand Up @@ -578,8 +603,10 @@ define(function (require, exports, module) {
* for changes
*/
function uninstallEditorListeners(editor) {
$(editor)
.off(HintUtils.eventName("change"));
if (editor) {
$(editor)
.off(HintUtils.eventName("change"));
}
}

/*
Expand Down Expand Up @@ -745,9 +772,12 @@ define(function (require, exports, module) {

return response;
}

// Register quickEditHelper.
brackets._jsCodeHintsHelper = quickEditHelper;

// Configuration function used for debugging
brackets._configureJSCodeHints = setConfig;

ExtensionUtils.loadStyleSheet(module, "styles/brackets-js-hints.css");

Expand Down
Loading