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

Refactor global brackets ready and htmlContentLoadComplete events to a new Global module #1425

Merged
merged 7 commits into from
Aug 24, 2012

Conversation

jasonsanjose
Copy link
Member

@ghost ghost assigned joelrbrandt Aug 24, 2012
@joelrbrandt
Copy link
Contributor

@jason-sanjose are commits a031653 and 418a989 supposed to be part of this pull?

@jasonsanjose
Copy link
Member Author

Yes. Those unit tests broke after the refactoring.

// a handler for the brackets 'ready' and 'htmlContentLoadComplete' events
global.brackets.ready = ready;
global.brackets.htmlContentLoadComplete = htmlContentLoadComplete;

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if adding these handlers to global.brackets is dangerous. It means that people could write code that registers handlers without actually loading this module. That allows the race condition we're trying to avoid with this refactoring.

Given that this module will be loaded asynchronously by require, I think it's certainly the case that if there was a not-using-require script include that immediately accessed brackets.ready, it would not yet be defined.

@joelrbrandt
Copy link
Contributor

@jason-sanjose initial review done -- lookin' good. Back at ya! Happy to chat over IRC if that's easier.

@@ -341,7 +257,11 @@ define(function (require, exports, module) {
var initialProjectPath = ProjectManager.getInitialProjectPath();
ProjectManager.openProject(initialProjectPath).done(function () {
_initTest();
_initExtensions().always(_onBracketsReady);

// WARNING: brackets.ready won't fire if ANY extension fails to
Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't called brackets.ready anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also the following comment needs to be cleaned up: line 61 of src/index.html

…s/Global. Add require("utils/Global") for any occurrences. Fix comments.
@jasonsanjose
Copy link
Member Author

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants