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

Fix for dropping projects when cancelling switch (#1780) #2013

Merged
merged 2 commits into from
Nov 2, 2012
Merged

Fix for dropping projects when cancelling switch (#1780) #2013

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c247e2
Fix for dropping projects when cancelling switch
jbalsas Nov 1, 2012
3be8717
Update remove project comment
jbalsas Nov 1, 2012
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
9 changes: 7 additions & 2 deletions src/extensions/default/RecentProjects/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ define(function (require, exports, module) {
SidebarView = brackets.getModule("project/SidebarView"),
Menus = brackets.getModule("command/Menus"),
PopUpManager = brackets.getModule("widgets/PopUpManager"),
FileUtils = brackets.getModule("file/FileUtils");
FileUtils = brackets.getModule("file/FileUtils"),
NativeFileSystem = brackets.getModule("file/NativeFileSystem").NativeFileSystem;

var $dropdownToggle;
var MAX_PROJECTS = 20;
Expand Down Expand Up @@ -148,7 +149,11 @@ define(function (require, exports, module) {
// Remove the project from the list.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update this comment to something like:

// If folder does not exist on disk then remove the project from the list

var index = recentProjects.indexOf(root);
if (index !== -1) {
recentProjects.splice(index, 1);
NativeFileSystem.requestNativeFileSystem(root,
function () {},
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to define an empty function here. Use null instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using null shows a console error "object is not a function" in NativeFileSystem when the folder does exist. Are we fine with that? I supposed it'd be better to avoid it...

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I missed that. Keep it like it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, it looks like the success callback is not nullable/optional according to the W3C spec, so we currently have the "correct" behavior in NativeFileSystem. Seems a little annoying from an API design point of view, but we want to stick with the spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@njx Thanks for the info! It somehow felt that it'd be safer to have nullable parameters to avoid possible errors. However, it also feels strange to "request a file system" and not wanting to do anything with it :)

function () {
recentProjects.splice(index, 1);
});
}
});
closeDropdown();
Expand Down