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

Live development #1418

Merged
merged 30 commits into from
Aug 28, 2012
Merged

Live development #1418

merged 30 commits into from
Aug 28, 2012

Conversation

jdiehl
Copy link
Contributor

@jdiehl jdiehl commented Aug 22, 2012

This pull request adds all extra features from the live-development branch, but only enables them if the experimental and autoconnect flags are set in LiveDevelopment/main.js. If this is merged into master, we can get rid of the live-development branch.

Additionally, there are some minor bug fixes and extensions to live-development that have no effect on the productive environment.

jdiehl and others added 26 commits July 26, 2012 08:21
Update Live Development branch to master.
… after jumping and prevents it from being removed after 1s when the flash is over)
This stuff should probably go to brackets...
Conflicts:
	src/LiveDevelopment/Agents/GotoAgent.js
	src/LiveDevelopment/Documents/HTMLDocument.js
	src/LiveDevelopment/Documents/JSDocument.js
	src/LiveDevelopment/Inspector/Inspector.json
	src/LiveDevelopment/Inspector/inspector.html
	src/LiveDevelopment/LiveDevelopment.js
_makeJSTarget(targets, trace.children[0].callFrames[0]);
}
}
for (i in res.matchedCSSRules.reverse()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line reverses the order of CSS rules in the browser menu, which represents the natural order of CSS rules from most to least precise.

case "js":
return JSDocument;
return exports.config.experimental ? JSDocument : null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

JSDocument & HTMLDocument are only returned if the experimental flag is set.

@ghost ghost assigned gruehle Aug 24, 2012
@@ -249,6 +245,10 @@ define(function LiveDevelopment(require, exports, module) {
/** Load the agents */
function loadAgents() {
var name, promises = [];
if (exports.config.experimental) {
// load all agents
_enabledAgentNames = agents;
Copy link
Member

Choose a reason for hiding this comment

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

This will lead to unexpected behavior. _enabledAgentNames becomes an alias for agents, so any time enableAgent() or disableAgent() is called, it will be changing the agents objects in addition to the _enabledAgentNames object. It would be better to iterate through the agents object and make sure all entries in the agents object have a corresponding entry in _enabledAgentNames.

@gruehle
Copy link
Member

gruehle commented Aug 27, 2012

Thanks for the annotations in the pull request!

I had couple comments, but everything else looks great.

@jdiehl
Copy link
Contributor Author

jdiehl commented Aug 28, 2012

Github ate your comments when I fixed them!

  1. I changed the way all agents are loaded by using a local variable that is set to either agents or _enabledAgentNames and used to iterate over for loading agents (both objects will return the same keys in a for loop).
  2. I changed brackets.ready to AppInit.appReady.

LiveDevelopment = require("LiveDevelopment/LiveDevelopment"),
Inspector = require("LiveDevelopment/Inspector/Inspector"),
CommandManager = require("command/CommandManager"),
Strings = require("strings");

var config = {
experimental: true, // enable experimental features
Copy link
Member

Choose a reason for hiding this comment

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

Whoops, I don't think you meant to set this to true for the pull request.

@gruehle
Copy link
Member

gruehle commented Aug 28, 2012

Github ate your comments when I fixed them!

Yeah, GitHub has a new "feature" that automatically hides comments if the line of code that was commented on has changed. You can still get to these comments by clicking "View outdated diff"

@jdiehl
Copy link
Contributor Author

jdiehl commented Aug 28, 2012

Whoops, sorry about including the experimental and autoconnect flags. I removed than and hopefully the pull request is all set now. Thanks for looking at the changes to quickly!

@gruehle
Copy link
Member

gruehle commented Aug 28, 2012

Looks good! Thanks.

Merging.

gruehle added a commit that referenced this pull request Aug 28, 2012
@gruehle gruehle merged commit 540b7c4 into adobe:master Aug 28, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants