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

Live Development -- Provide API for enabling/disabling Agents #1303

Merged
merged 6 commits into from
Jul 26, 2012
Merged

Live Development -- Provide API for enabling/disabling Agents #1303

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3282802
Enabled the remaining agents
jdiehl Jul 26, 2012
bd8dc7d
provide api to enable/disable LiveDevelopment agents so that extensio…
joelrbrandt Jul 26, 2012
e67c7d2
improve variable names and add a few comments
joelrbrandt Jul 26, 2012
2223087
changed iteration variable name
joelrbrandt Jul 26, 2012
5c72190
improve another iteration variable name
joelrbrandt Jul 26, 2012
1bce0f3
better value checking
joelrbrandt Jul 26, 2012
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
50 changes: 42 additions & 8 deletions src/LiveDevelopment/LiveDevelopment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,24 @@ define(function LiveDevelopment(require, exports, module) {
"remote": require("LiveDevelopment/Agents/RemoteAgent"),
"network": require("LiveDevelopment/Agents/NetworkAgent"),
"dom": require("LiveDevelopment/Agents/DOMAgent"),
"css": require("LiveDevelopment/Agents/CSSAgent")
/* FUTURE
"css": require("LiveDevelopment/Agents/CSSAgent"),
"script": require("LiveDevelopment/Agents/ScriptAgent"),
"highlight": require("LiveDevelopment/Agents/HighlightAgent"),
"goto": require("LiveDevelopment/Agents/GotoAgent"),
"edit": require("LiveDevelopment/Agents/EditAgent")
*/
};

// Some agents are still experimental, so we don't enable them all by default
// However, extensions can enable them by calling enableAgent()
var _enabledAgents = {
"console": true,
"remote": true,
"network": true,
"dom": true,
"css": true
};
var _loadedAgents = [];

var _htmlDocumentPath; // the path of the html file open for live development
var _liveDocument; // the document open for live editing.
var _relatedDocuments; // CSS and JS documents that are used by the live HTML document
Expand Down Expand Up @@ -221,25 +230,48 @@ define(function LiveDevelopment(require, exports, module) {

/** Unload the agents */
function unloadAgents() {
var i;
for (i in agents) {
_loadedAgents.forEach(function (i) {
Copy link
Member

Choose a reason for hiding this comment

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

The first arg here is the value of _loadedAgents[i], not i itself. As-is I think this code will never try to unload anything.

Copy link
Member

Choose a reason for hiding this comment

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

Oops, never mind... now that I've read below I see _loadedAgents is actually an array of agent names (the keys of 'agents'), not the agents themselves. Could we clarify that either by renaming this array or at least by adding docs where it's declared? Also, similar to below let's not use 'i' here for something that's not actually an index.

(Alternatively, you could change _loadedAgents to be an array of the actual agent objects... that seems fine to me too).

Also, the hasOwnProperty() check can go away now that you're using forEach().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed name of _loadedAgents to _loadedAgentNames to make this more clear. Also added a comment.

And, I removed the hasOwnProperty check.

if (agents.hasOwnProperty(i) && agents[i].unload) {
agents[i].unload();
}
}
});
_loadedAgents = [];
}

/** Load the agents */
function loadAgents() {
var i, promises = [];
for (i in agents) {
if (agents.hasOwnProperty(i) && agents[i].load) {
for (i in _enabledAgents) {
if (_enabledAgents.hasOwnProperty(i) && agents.hasOwnProperty(i) && agents[i].load) {
Copy link
Member

Choose a reason for hiding this comment

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

This assumes the indices in '_enabledAgents' and 'agents' match perfectly. So the set of enabled agents must always be grouped in a contiguous block at the top of 'agents.' Can we guarantee that's always true?

Copy link
Member

Choose a reason for hiding this comment

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

Oops, strike that too... these are maps, not arrays. Could you rename 'i' to something more clear, like 'agentName'?

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 changed _enabledAgents to _enabledAgentNames, and the iteration variable to 'name'. I also added a comment to make this more clear.

promises.push(agents[i].load());
_loadedAgents.push(i);
}
}
return promises;
}

/** Enable an agent. Takes effect next time a connection is made. Does not affect
* current live development sessions.
*
* @param {string} name of agent to enable
*/
function enableAgent(name) {
if (!_enabledAgents.hasOwnProperty(name)) {
_enabledAgents[name] = true;
}
}

/** Disable an agent. Takes effect next time a connection is made. Does not affect
* current live development sessions.
*
* @param {string} name of agent to disable
*/
function disableAgent(name) {
if (_enabledAgents.hasOwnProperty(name)) {
delete _enabledAgents[name];
}
}

/** Update the status
* @param {integer} new status
*/
Expand Down Expand Up @@ -459,6 +491,8 @@ define(function LiveDevelopment(require, exports, module) {
exports.agents = agents;
exports.open = open;
exports.close = close;
exports.enableAgent = enableAgent;
exports.disableAgent = disableAgent;
exports.getLiveDocForPath = getLiveDocForPath;
exports.hideHighlight = hideHighlight;
exports.init = init;
Expand Down