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

Resolve the issue with the default scope being overriden #9047

Merged
redmunds merged 2 commits into from
Sep 15, 2014
Merged

Resolve the issue with the default scope being overriden #9047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2aee554
Remove debug alert.
busykai Sep 10, 2014
58a4e50
Resolve the issue with default scope overriden.
busykai Sep 10, 2014
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
1 change: 0 additions & 1 deletion src/brackets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ define(function (require, exports, module) {
* Setup Brackets
*/
function _onReady() {
window.alert("brackets.js _onReady()");
PerfUtils.addMeasurement("window.document Ready");
Copy link
Copy Markdown
Contributor

@redmunds redmunds Sep 15, 2014

Choose a reason for hiding this comment

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

Ooops! :)


// Let the user know Brackets doesn't run in a web browser yet
Expand Down
166 changes: 79 additions & 87 deletions src/preferences/PreferencesBase.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ define(function (require, exports, module) {
MemoryStorage.prototype = {

/**
* *Synchronously* returns the data stored in this storage.
* The original object (not a clone) is returned.
* Returns a promise which resolves with the data stored in this
* storage. The original object (not a clone) is returned.
*
* @return {Promise} promise that is already resolved
*/
Expand Down Expand Up @@ -1056,14 +1056,13 @@ define(function (require, exports, module) {
"default": new Scope(new MemoryStorage())
};

this._scopes["default"].load();

this._defaultContext = {
scopeOrder: ["default"],
_shadowScopeOrder: [{
id: "default",
scope: this._scopes["default"],
promise: (new Promise(function (resolve, reject) { resolve(); }))
state: "resolved",
promise: Promise.resolve()
}]
};

Expand Down Expand Up @@ -1174,58 +1173,47 @@ define(function (require, exports, module) {
* before.
*
* @param {Object} shadowEntry Shadow entry of the resolved scope
* @param {number=} index Index of item to "try". Default value is item after shadowEntry.
* @return {Promise} Resolved when scope added to scope order.
*/
_tryAddToScopeOrder: function (shadowEntry, index) {
_tryAddToScopeOrder: function (shadowEntry) {

return new Promise(function (resolve, reject) {
var lowerScope,
shadowScopeOrder = this._defaultContext._shadowScopeOrder;

// Find a resolved scope of lower priority to add it before.
// `index` is calculated on initial call. It's then incremented and
// passed in for recursive iterations.
index = index || _.findIndex(shadowScopeOrder, function (entry) {
var shadowScopeOrder = this._defaultContext._shadowScopeOrder,
index = _.findIndex(shadowScopeOrder, function (entry) {
return entry === shadowEntry;
}) + 1;
}),
$this = $(this),
i = index + 1;

if (index >= shadowScopeOrder.length) {
reject("Internal error: no scope found to add before. \"default\" is missing?..");
return;
// Find a resolved scope of lower priority to add it before.
while (i < shadowScopeOrder.length) {
if (shadowScopeOrder[i].state === "pending" ||
shadowScopeOrder[i].state === "resolved") {
break;
}

lowerScope = shadowScopeOrder[index];

var _addScope = function () {
this._pushToScopeOrder(shadowEntry.id, lowerScope.id);
$(this).trigger(SCOPEORDER_CHANGE, {
id: shadowEntry.id,
action: "added"
});
this._triggerChange({
ids: shadowEntry.scope.getKeys()
});
resolve();
}.bind(this);

// We can't synchronously determine state of a Promise, but we know that
// Scopes with MemoryStorage are resolved synchronously.
if (lowerScope.scope.storage instanceof MemoryStorage) {
_addScope();
} else {
// For FileStorage, we'll have to wait for appropriate callback.
lowerScope.promise
.then(function () {
// onFulfillment - success
_addScope();
}.bind(this))
.catch(function () {
// onRejection - recursively try next scope
this._tryAddToScopeOrder(shadowEntry, index + 1).then(resolve, reject);
}.bind(this));
}
}.bind(this));
i++;
}

switch (shadowScopeOrder[i].state) {
case "pending":
// cannot decide now, lookup once pending promise is settled
shadowScopeOrder[i].promise.then(function () {
this._tryAddToScopeOrder(shadowEntry);
}.bind(this), function () {
this._tryAddToScopeOrder(shadowEntry);
}.bind(this));
break;
case "resolved":
this._pushToScopeOrder(shadowEntry.id, shadowScopeOrder[i].id);
$this.trigger(SCOPEORDER_CHANGE, {
id: shadowEntry.id,
action: "added"
});
this._triggerChange({
ids: shadowEntry.scope.getKeys()
});
break;
default:
throw new Error("Internal error: no scope found to add before. \"default\" is missing?..");
}
},

/**
Expand All @@ -1248,7 +1236,6 @@ define(function (require, exports, module) {
* @param {Scope} scope The scope object to add
* @param {Promise} scopeLoadPromise Scope's load promise
* @param {?string} addBefore Name of the Scope before which this new one is added
* @return {Promise} Resolved when scope added to scope order.
*/
_addToScopeOrder: function (id, scope, scopeLoadPromise, addBefore) {
var defaultScopeOrder = this._defaultContext.scopeOrder,
Expand All @@ -1273,8 +1260,25 @@ define(function (require, exports, module) {
shadowEntry = {
id: id,
promise: scopeLoadPromise,
state: "pending",
scope: scope
};


if (scope.storage instanceof MemoryStorage) {
shadowEntry.state = "resolved";
} else {
scopeLoadPromise
.then(
function () {
shadowEntry.state = "resolved";
},
function () {
shadowEntry.state = "rejected";
}
);
}

if (!addBefore) {
shadowScopeOrder.unshift(shadowEntry);
} else {
Expand All @@ -1294,37 +1298,26 @@ define(function (require, exports, module) {
}
}
}

if (isPending) {
return Promise.resolve();
} else {
var tryPromises = [];

var tryPromise = new Promise(function (reject, resolve) {
scopeLoadPromise
.then(function () {

if (!isPending) {
scopeLoadPromise
.then(
function () {
this._scopes[id] = scope;
this._tryAddToScopeOrder(shadowEntry).then(resolve, reject);
}.bind(this))
.catch(function (err) {
this._tryAddToScopeOrder(shadowEntry);
}.bind(this),
function (err) {
// clean up all what's been done up to this point
_.pull(shadowScopeOrder, shadowEntry);
reject();
});
}.bind(this));
tryPromises.push(tryPromise);

}.bind(this)
);
if (this._pendingScopes[id]) {
var pending = this._pendingScopes[id];
delete this._pendingScopes[id];
pending.forEach(function (entry) {
tryPromises.push(
this._addToScopeOrder(entry.id, entry.scope, entry.promise, id)
);
this._addToScopeOrder(entry.id, entry.scope, entry.promise, id);
}.bind(this));
}

return Async.waitForAll(tryPromises, true);
}
},

Expand Down Expand Up @@ -1396,11 +1389,10 @@ define(function (require, exports, module) {
* @param {string} id Name of the Scope
* @param {Scope|Storage} scope the Scope object itself. Optionally, can be given a Storage directly for convenience.
* @param {{before: string}} options optional behavior when adding (e.g. setting which scope this comes before)
* @return {Promise} Promise that is resolved when the Scope is loaded. It is resolved
* with id and scope.
* @return {Promise} Promise that is resolved when the Scope is loaded.
*/
addScope: function (id, scope, options) {
var loadScopePromise, addToScopeOrderPromise;
var loadScopePromise;
options = options || {};

if (this._scopes[id]) {
Expand All @@ -1414,7 +1406,7 @@ define(function (require, exports, module) {

loadScopePromise = scope.load();

addToScopeOrderPromise = this._addToScopeOrder(id, scope, loadScopePromise, options.before);
this._addToScopeOrder(id, scope, loadScopePromise, options.before);

loadScopePromise
.catch(function (err) {
Expand All @@ -1425,7 +1417,7 @@ define(function (require, exports, module) {
}
});

return Async.waitForAll([loadScopePromise, addToScopeOrderPromise], true);
return loadScopePromise;
},

/**
Expand Down Expand Up @@ -1613,12 +1605,12 @@ define(function (require, exports, module) {
this._nextSave = {};

Async.doInParallel(_.values(this._scopes), function (scope) {
if (scope) {
return scope.save();
} else {
return Promise.resolve();
}
}.bind(this))
if (scope) {
return scope.save();
} else {
return Promise.resolve();
}
})
Copy link
Copy Markdown
Contributor

@redmunds redmunds Sep 15, 2014

Choose a reason for hiding this comment

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

JSLlint is complaining about this block being indented.

.then(function () {
if (this._saveInProgress.resolve) {
this._saveInProgress.resolve();
Expand All @@ -1633,7 +1625,7 @@ define(function (require, exports, module) {
this._saveInProgress.reject(err);
}
this._saveInProgress = {};
});
}.bind(this));

return this._saveInProgress.promise;
},
Expand Down
3 changes: 2 additions & 1 deletion src/preferences/PreferencesImpl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ define(function (require, exports, module) {
if (err.name && err.name === "ParsingError") {
userScopeCorrupt = true;
}
})
});
userScopeLoading
.then(fnUserScopeAlways, fnUserScopeAlways);

// "State" is stored like preferences but it is not generally intended to be user-editable.
Expand Down