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

Fix bug #7300 (Prefs never saved again after opening ancestor folder of user prefs.json) #7376

Merged
merged 1 commit into from
Apr 2, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/preferences/PreferenceStorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
/**
* PreferenceStorage defines an interface for persisting preference data as
* name/value pairs for a module or plugin.
*
* @deprecated Use PreferencesManager APIs instead.
*/
define(function (require, exports, module) {
"use strict";
Expand Down
22 changes: 11 additions & 11 deletions src/preferences/PreferencesBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ define(function (require, exports, module) {
/**
* MemoryStorage is not stored in a file, so fileChanged is ignored.
*
* @param {string} filename File that has changed
* @param {string} filePath File that has changed
*/
fileChanged: function (filename) {
fileChanged: function (filePath) {
}
};

Expand Down Expand Up @@ -241,10 +241,10 @@ define(function (require, exports, module) {
/**
* If the filename matches this Storage's path, a changed message is triggered.
*
* @param {string} filename File that has changed
* @param {string} filePath File that has changed
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit: the preferred nomenclature is pathname

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it? I get 2x as many hits for 'filepath' in our codebase than 'pathname.' I'm not sure we've settled on any strict convention. I'm just certain that 'filename' is misleading here :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Or I wonder if it should be fullPath to harmonize with the FileSystem nomenclature?

*/
fileChanged: function (filename) {
if (filename === this.path) {
fileChanged: function (filePath) {
if (filePath === this.path) {
$(this).trigger("changed");
}
}
Expand Down Expand Up @@ -485,10 +485,10 @@ define(function (require, exports, module) {
* Tells the Scope that the given file has been changed so that the
* Storage can be reloaded if needed.
*
* @param {string} filename Name of the file that has changed
* @param {string} filePath File that has changed
*/
fileChanged: function (filename) {
this.storage.fileChanged(filename);
fileChanged: function (filePath) {
this.storage.fileChanged(filePath);
},

/**
Expand Down Expand Up @@ -1693,11 +1693,11 @@ define(function (require, exports, module) {
* Tells the PreferencesSystem that the given file has been changed so that any
* related Scopes can be reloaded.
*
* @param {string} filename Name of the file that has changed
* @param {string} filePath File that has changed
*/
fileChanged: function (filename) {
fileChanged: function (filePath) {
_.forEach(this._scopes, function (scope) {
scope.fileChanged(filename);
scope.fileChanged(filePath);
});
},

Expand Down
14 changes: 14 additions & 0 deletions src/preferences/PreferencesImpl.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ define(function (require, exports, module) {
"use strict";

var PreferencesBase = require("./PreferencesBase"),
ProjectManager = require("project/ProjectManager"),
Async = require("utils/Async"),

// The SETTINGS_FILENAME is used with a preceding "." within user projects
Expand Down Expand Up @@ -103,6 +104,7 @@ define(function (require, exports, module) {
});
});


// "State" is stored like preferences but it is not generally intended to be user-editable.
// It's for more internal, implicit things like window size, working set, etc.
var stateManager = new PreferencesBase.PreferencesSystem();
Expand All @@ -112,6 +114,18 @@ define(function (require, exports, module) {
smUserScope.addLayer(stateProjectLayer);
var smUserScopeLoading = stateManager.addScope("user", smUserScope);


// Listen for times where we might be unwatching a root that contains one of the user-level prefs files,
// and force a re-read of the file in order to ensure we can write to it later (see #7300).
$(ProjectManager).on("projectClose", function (event, rootDir) {
var prefsDir = brackets.app.getApplicationSupportDirectory() + "/";
if (prefsDir.indexOf(rootDir.fullPath) === 0) {
manager.fileChanged(userPrefFile);
stateManager.fileChanged(userStateFile);
}
});


// Semi-Public API. Use this at your own risk. The public API is in PreferencesManager.
exports.manager = manager;
exports.projectStorage = projectStorage;
Expand Down
18 changes: 13 additions & 5 deletions src/project/ProjectManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
* the file tree.
*
* This module dispatches these events:
* - beforeProjectClose -- before _projectRoot changes
* - beforeProjectClose -- before _projectRoot changes, but working set files still open
* - projectClose -- *just* before _projectRoot changes; working set already cleared & project root unwatched
* - beforeAppClose -- before Brackets quits entirely
* - projectOpen -- after _projectRoot changes and the tree is re-rendered
* - projectRefresh -- when project tree is re-rendered for a reason other than
Expand Down Expand Up @@ -159,6 +160,7 @@ define(function (require, exports, module) {
/**
* @private
* @see getProjectRoot()
* @type {Directory}
*/
var _projectRoot = null;

Expand Down Expand Up @@ -674,15 +676,15 @@ define(function (require, exports, module) {
var events = $._data(_projectTree[0], "events"),
eventsForType = events ? events[type] : null,
event = eventsForType ? _.find(eventsForType, function (e) {
return e.namespace === namespace && e.selector === selector;
}) : null,
return e.namespace === namespace && e.selector === selector;
}) : null,
eventHandler = event ? event.handler : null;
if (!eventHandler) {
console.error(type + "." + namespace + " " + selector + " handler not found!");
}
return eventHandler;
};
var createCustomHandler = function(originalHandler) {
var createCustomHandler = function (originalHandler) {
return function (event) {
var $node = $(event.target).parent("li"),
methodName;
Expand Down Expand Up @@ -1102,15 +1104,21 @@ define(function (require, exports, module) {
if (_projectRoot && _projectRoot.fullPath === rootPath) {
return (new $.Deferred()).resolve().promise();
}

// About to close current project (if any)
if (_projectRoot) {
// close current project
$(exports).triggerHandler("beforeProjectClose", _projectRoot);
}

// close all the old files
DocumentManager.closeAll();

_unwatchProjectRoot().always(function () {
// Done closing old project (if any)
if (_projectRoot) {
$(exports).triggerHandler("projectClose", _projectRoot);
}

startLoad.resolve();
});
}
Expand Down