-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Conversation
… to wait for extension loading before initializing brackets.test.
reviewing |
@@ -324,7 +323,16 @@ define(function (require, exports, module) { | |||
|
|||
PerfUtils.addMeasurement("Application Startup"); | |||
|
|||
ProjectManager.loadProject(); | |||
}); | |||
// load project before loading extensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops. This was me. The comment should read extensions before project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Initial review complete. Need to merge with master. |
if (module && module.uri) { | ||
|
||
// Remove window name from base path. Maintain trailing slash. | ||
pathname = window.location.pathname; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to explicitly decode the URI returned from window.location.pathname by calling decodeURI() as I did in #922.
Without this call the path is encoded with %XX for all non-English characters and users who install or run Brackets from a path with some non-English characters will have issues in launching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
} | ||
} | ||
|
||
Async.doInParallel(extensions, function (item) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should doSequentially() be used here to enforce loading in a particular order (I guess testing in parallel is ok)?
That seems a bit wasteful, it would be nice to have something like loadInParallelThenExecuteSequentiallyInOriginalOrder().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests won't run in parallel, they'll just load. But yes, their load order will be unstable. The new SpecRunner will sort all suites alphabetically.
javascript quick edit unit tests
First pass at unit tests for an extension. The unittests.js file, if it exists, is executed for each extension in user or default folder.
Added FileUtils.getNativeModuleDirectoryPath() api call to help extension author access files in the extension folder.