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

Avoid adding duplicates to the list of welcome projects, and ... #2812

Merged
merged 1 commit into from
Feb 8, 2013
Merged

Avoid adding duplicates to the list of welcome projects, and ... #2812

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 24 additions & 3 deletions src/project/ProjectManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,9 +695,12 @@ define(function (require, exports, module) {
* or the one for the current build.
*/
function isWelcomeProjectPath(path) {
var canonPath = FileUtils.canonicalizeFolderPath(path);
if (canonPath === _getWelcomeProjectPath()) {
return true;
}
var welcomeProjects = _prefs.getValue("welcomeProjects") || [];
welcomeProjects.push(_getWelcomeProjectPath());
return welcomeProjects.indexOf(FileUtils.canonicalizeFolderPath(path)) !== -1;
return welcomeProjects.indexOf(canonPath) !== -1;
}

/**
Expand Down Expand Up @@ -755,6 +758,7 @@ define(function (require, exports, module) {
var rootEntry = fs.root;
var projectRootChanged = (!_projectRoot || !rootEntry) ||
_projectRoot.fullPath !== rootEntry.fullPath;
var i;

// Success!
var perfTimerName = PerfUtils.markStart("Load Project: " + rootPath),
Expand Down Expand Up @@ -1328,9 +1332,26 @@ define(function (require, exports, module) {

// Init PreferenceStorage
var defaults = {
projectPath: _getWelcomeProjectPath() /* initialize to brackets source */
projectPath: _getWelcomeProjectPath() /* initialize to welcome project */
};
_prefs = PreferencesManager.getPreferenceStorage(PREFERENCES_CLIENT_ID, defaults);

if (!_prefs.getValue("welcomeProjectsFixed")) {
Copy link
Member

@gruehle gruehle Feb 8, 2013

Choose a reason for hiding this comment

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

I understand the intent of this code, but I'm a bit worried about adding backward compatibility code like this so early in development. Over time, this type of code becomes difficult to manage and maintain.

Personally, I would be fine with just removing this block. If you feel strongly about keeping it, I think it would be best to file a code cleanup bug to remove this code after a sprint or two (this is making the assumption that most current Brackets users are updating every sprint or two, and would have already had there old prefs cleaned up).

Thoughts?

Copy link
Member

@peterflynn peterflynn Feb 8, 2013

Choose a reason for hiding this comment

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

Removing it after a couple sprints seems reasonable to me, fwiw. I do think we should have it in there temporarily, though -- I bet lots of people don't clear prefs routinely and will hit the issue I saw given enough time (and I think the end result is that we just stop remembering pref changes without warning).

Copy link
Author

@njx njx Feb 8, 2013

Choose a reason for hiding this comment

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

Yes, I think we should leave it in for a few sprints.

// One-time cleanup of duplicates in the welcome projects list--there used to be a bug where
// we would add lots of duplicate entries here.
var welcomeProjects = _prefs.getValue("welcomeProjects");
if (welcomeProjects) {
var newWelcomeProjects = [];
var i;
for (i = 0; i < welcomeProjects.length; i++) {
if (newWelcomeProjects.indexOf(welcomeProjects[i]) === -1) {
newWelcomeProjects.push(welcomeProjects[i]);
}
}
_prefs.setValue("welcomeProjects", newWelcomeProjects);
_prefs.setValue("welcomeProjectsFixed", true);
}
}

// Event Handlers
$(FileViewController).on("documentSelectionFocusChange", _documentSelectionFocusChange);
Expand Down