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

Move file or folder to the trash #3879

Merged
merged 14 commits into from
May 17, 2013
Merged
Show file tree
Hide file tree
Changes from 13 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
1 change: 1 addition & 0 deletions src/command/Commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ define(function (require, exports, module) {
exports.FILE_LIVE_HIGHLIGHT = "file.previewHighlight";
exports.FILE_PROJECT_SETTINGS = "file.projectSettings";
exports.FILE_RENAME = "file.rename";
exports.FILE_DELETE = "file.delete";
exports.FILE_INSTALL_EXTENSION = "file.installExtension";
exports.FILE_EXTENSION_MANAGER = "file.extensionManager";
exports.FILE_QUIT = "file.quit"; // string must MATCH string in native code (brackets_extensions)
Expand Down
1 change: 1 addition & 0 deletions src/command/DefaultMenus.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ define(function (require, exports, module) {
project_cmenu.addMenuItem(Commands.FILE_NEW);
project_cmenu.addMenuItem(Commands.FILE_NEW_FOLDER);
project_cmenu.addMenuItem(Commands.FILE_RENAME);
project_cmenu.addMenuItem(Commands.FILE_DELETE);
project_cmenu.addMenuItem(Commands.NAVIGATE_SHOW_IN_OS);
project_cmenu.addMenuDivider();
project_cmenu.addMenuItem(Commands.EDIT_FIND_IN_SUBTREE);
Expand Down
7 changes: 6 additions & 1 deletion src/document/DocumentCommandHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,11 @@ define(function (require, exports, module) {
ProjectManager.showInTree(DocumentManager.getCurrentDocument().file);
}

function handleFileDelete() {
var entry = ProjectManager.getSelectedItem();
ProjectManager.deleteItem(entry);
}

/** Show the selected sidebar (tree or working set) item in Finder/Explorer */
function handleShowInOS() {
var entry = ProjectManager.getSelectedItem();
Expand All @@ -857,7 +862,6 @@ define(function (require, exports, module) {
}
}


// Init DOM elements
AppInit.htmlReady(function () {
var $titleContainerToolbar = $("#titlebar");
Expand All @@ -878,6 +882,7 @@ define(function (require, exports, module) {
CommandManager.register(Strings.CMD_FILE_SAVE, Commands.FILE_SAVE, handleFileSave);
CommandManager.register(Strings.CMD_FILE_SAVE_ALL, Commands.FILE_SAVE_ALL, handleFileSaveAll);
CommandManager.register(Strings.CMD_FILE_RENAME, Commands.FILE_RENAME, handleFileRename);
CommandManager.register(Strings.CMD_FILE_DELETE, Commands.FILE_DELETE, handleFileDelete);

CommandManager.register(Strings.CMD_FILE_CLOSE, Commands.FILE_CLOSE, handleFileClose);
CommandManager.register(Strings.CMD_FILE_CLOSE_ALL, Commands.FILE_CLOSE_ALL, handleFileCloseAll);
Expand Down
31 changes: 28 additions & 3 deletions src/document/DocumentManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
*
* - fileNameChange -- When the name of a file or folder has changed. The 2nd arg is the old name.
* The 3rd arg is the new name.
* - pathDeleted -- When a file or folder has been deleted. The 2nd arg is the path that was deleted.
*
* These are jQuery events, so to listen for them you do something like this:
* $(DocumentManager).on("eventname", handler);
Expand Down Expand Up @@ -478,8 +479,9 @@ define(function (require, exports, module) {
* This is a subset of notifyFileDeleted(). Use this for the user-facing Close command.
*
* @param {!FileEntry} file
* @param {boolean} skipAutoSelect - if true, don't automatically open and select the next document
*/
function closeFullEditor(file) {
function closeFullEditor(file, skipAutoSelect) {
// If this was the current document shown in the editor UI, we're going to switch to a
// different document (or none if working set has no other options)
if (_currentDocument && _currentDocument.file.fullPath === file.fullPath) {
Expand All @@ -491,7 +493,7 @@ define(function (require, exports, module) {
}

// Switch editor to next document (or blank it out)
if (nextFile) {
if (nextFile && !skipAutoSelect) {
CommandManager.execute(Commands.FILE_OPEN, { fullPath: nextFile.fullPath })
.done(function () {
// (Now we're guaranteed that the current document is not the one we're closing)
Expand Down Expand Up @@ -1063,8 +1065,9 @@ define(function (require, exports, module) {
* sort of "project file model," making this just a private event handler.
*
* @param {!FileEntry} file
* @param {boolean} skipAutoSelect - if true, don't automatically open/select the next document
*/
function notifyFileDeleted(file) {
function notifyFileDeleted(file, skipAutoSelect) {
Copy link
Contributor

Choose a reason for hiding this comment

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

skipAutoSelect parameter is unused.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that should have been passed to closeFullEditor(). Fixed.

// First ensure it's not currentDocument, and remove from working set
closeFullEditor(file);

Expand Down Expand Up @@ -1211,6 +1214,27 @@ define(function (require, exports, module) {
$(exports).triggerHandler("fileNameChange", [oldName, newName]);
}

/**
* Called after a file or folder has been deleted. This function is responsible
* for updating underlying model data and notifying all views of the change.
*
* @param {string} path The path of the file/folder that has been deleted
*/
function notifyPathDeleted(path) {
var i, docPath;

for (docPath in _openDocuments) {
if (FileUtils.isAffectedWhenRenaming(docPath, path)) {
// This will close the doc and remove from the working set
notifyFileDeleted(new NativeFileSystem.FileEntry(docPath), true);
delete _openDocuments[docPath];
}
}

// Send a "pathDeleted" event. This will trigger the views to update.
$(exports).triggerHandler("pathDeleted", path);
}

/**
* @private
* Update document
Expand Down Expand Up @@ -1262,6 +1286,7 @@ define(function (require, exports, module) {
exports.closeAll = closeAll;
exports.notifyFileDeleted = notifyFileDeleted;
exports.notifyPathNameChanged = notifyPathNameChanged;
exports.notifyPathDeleted = notifyPathDeleted;

// Setup preferences
_prefs = PreferencesManager.getPreferenceStorage(module);
Expand Down
12 changes: 9 additions & 3 deletions src/file/NativeFileSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,13 +365,19 @@ define(function (require, exports, module) {
};

/**
* Deletes a file or directory
* Deletes a file or directory by moving to the trash/recycle bin.
* @param {function()} successCallback Callback function for successful operations
* @param {function(DOMError)=} errorCallback Callback function for error operations
*/
NativeFileSystem.Entry.prototype.remove = function (successCallback, errorCallback) {
// TODO (issue #241)
// http://www.w3.org/TR/2011/WD-file-system-api-20110419/#widl-Entry-remove
var deleteFunc = brackets.fs.moveToTrash || brackets.fs.unlink;
deleteFunc(this.fullPath, function (err) {
if (err === brackets.fs.NO_ERROR) {
successCallback();
} else {
errorCallback(err);
}
});
};

/**
Expand Down
5 changes: 4 additions & 1 deletion src/nls/root/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ define({
"NOT_READABLE_ERR" : "The file could not be read.",
"NO_MODIFICATION_ALLOWED_ERR" : "The target directory cannot be modified.",
"NO_MODIFICATION_ALLOWED_ERR_FILE" : "The permissions do not allow you to make modifications.",
"FILE_EXISTS_ERR" : "The file already exists.",
"FILE_EXISTS_ERR" : "The file or directory already exists.",

// Project error strings
"ERROR_LOADING_PROJECT" : "Error loading project",
Expand All @@ -53,6 +53,8 @@ define({
"ERROR_SAVING_FILE" : "An error occurred when trying to save the file <span class='dialog-filename'>{0}</span>. {1}",
"ERROR_RENAMING_FILE_TITLE" : "Error renaming file",
"ERROR_RENAMING_FILE" : "An error occurred when trying to rename the file <span class='dialog-filename'>{0}</span>. {1}",
"ERROR_DELETING_FILE_TITLE" : "Error deleting file",
"ERROR_DELETING_FILE" : "An error occurred when trying to delete the file <span class='dialog-filename'>{0}</span>. {1}",
"INVALID_FILENAME_TITLE" : "Invalid file name",
"INVALID_FILENAME_MESSAGE" : "Filenames cannot contain the following characters: /?*:;{}<>\\|",
"FILE_ALREADY_EXISTS" : "The file <span class='dialog-filename'>{0}</span> already exists.",
Expand Down Expand Up @@ -177,6 +179,7 @@ define({
"CMD_LIVE_HIGHLIGHT" : "Live Highlight",
"CMD_PROJECT_SETTINGS" : "Project Settings\u2026",
"CMD_FILE_RENAME" : "Rename",
"CMD_FILE_DELETE" : "Delete",
"CMD_INSTALL_EXTENSION" : "Install Extension\u2026",
"CMD_EXTENSION_MANAGER" : "Extension Manager\u2026",
"CMD_QUIT" : "Quit",
Expand Down
59 changes: 58 additions & 1 deletion src/project/ProjectManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -1299,7 +1299,7 @@ define(function (require, exports, module) {

result.resolve();
} else {
// Show and error alert
// Show an error alert
Dialogs.showModalDialog(
Dialogs.DIALOG_ID_ERROR,
Strings.ERROR_RENAMING_FILE_TITLE,
Expand Down Expand Up @@ -1383,6 +1383,62 @@ define(function (require, exports, module) {
});
// No fail handler: silently no-op if file doesn't exist in tree
}

/**
* Delete file or directore from project
* @param {!Entry} entry FileEntry or DirectoryEntry to delete
*/
function deleteItem(entry) {
var result = new $.Deferred();

entry.remove(function () {
_findTreeNode(entry).done(function ($node) {
_projectTree.one("delete_node.jstree", function () {
// When a node is deleted, the previous node is automatically selected.
// This works fine as long as the previous node is a file, but doesn't
// work so well if the node is a folder
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this behavior is not quite right. Currently, you can only delete file from file tree, and this is done by using the context menu, so by definition, the file is selected (i.e. shown in editor).

To determine the file that should become selected, we should first look for Working Files. When deleting the first file in folder list, this is currently working correctly. If there are any other Working Files, then one of them is selected, otherwise nothing is selected.

For non-first files in folder list, this currently selects previous file in folder list. It should instead choose from remaining Working Files. If there are no Working Files, then I think nothing should be selected (like File > Close), but I guess this should be discussed with team.

Copy link
Member Author

Choose a reason for hiding this comment

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

Selecting files in the working set after deleting from the file tree didn't feel right. The behavior I wanted was to move the selection to the nearest tree node above the one that was deleted. However, there is a bug in the current implementation (as pointed out by your comment above). The selection should always remain in the file tree after deleting and never get moved to the working set.

Try it out with the fixes for skipAutoSelect and see if that seems better.

var sel = _projectTree.jstree("get_selected"),
entry = sel ? sel.data("entry") : null;

if (entry && entry.isDirectory) {
// Make sure it didn't turn into a leaf node. This happens if
// the only file in the directory was deleted
if (sel.hasClass("jstree-leaf")) {
sel.removeClass("jstree-leaf jstree-open");
sel.addClass("jstree-closed");
}
}
});
var oldSuppressToggleOpen = suppressToggleOpen;
suppressToggleOpen = true;
_projectTree.jstree("delete_node", $node);
suppressToggleOpen = oldSuppressToggleOpen;
});

// Notify that one of the project files has changed
$(exports).triggerHandler("projectFilesChange");

DocumentManager.notifyPathDeleted(entry.fullPath);

_redraw(true);
result.promise();
}, function (err) {
// Show an error alert
Dialogs.showModalDialog(
Dialogs.DIALOG_ID_ERROR,
Strings.ERROR_DELETING_FILE_TITLE,
StringUtils.format(
Strings.ERROR_DELETING_FILE,
StringUtils.htmlEscape(entry.fullPath),
FileUtils.getFileErrorString(err)
)
);

result.reject(err);
});

return result;
}

/**
* Forces createNewItem() to complete by removing focus from the rename field which causes
Expand Down Expand Up @@ -1448,6 +1504,7 @@ define(function (require, exports, module) {
exports.updateWelcomeProjectPath = updateWelcomeProjectPath;
exports.createNewItem = createNewItem;
exports.renameItemInline = renameItemInline;
exports.deleteItem = deleteItem;
exports.forceFinishRename = forceFinishRename;
exports.showInTree = showInTree;
});
25 changes: 21 additions & 4 deletions src/project/WorkingSetView.js
Original file line number Diff line number Diff line change
Expand Up @@ -385,14 +385,16 @@ define(function (require, exports, module) {
* Deletes all the list items in the view and rebuilds them from the working set model
* @private
*/
function _rebuildWorkingSet() {
function _rebuildWorkingSet(forceRedraw) {
$openFilesContainer.find("ul").empty();

DocumentManager.getWorkingSet().forEach(function (file) {
_createNewListItem(file);
});

_redraw();
if (forceRedraw) {
_redraw();
}
}

/**
Expand Down Expand Up @@ -524,7 +526,7 @@ define(function (require, exports, module) {
* @private
*/
function _handleWorkingSetSort() {
_rebuildWorkingSet();
_rebuildWorkingSet(true);
_scrollSelectedDocIntoView();
}

Expand All @@ -550,7 +552,18 @@ define(function (require, exports, module) {
// Rebuild the working set if any file or folder name changed.
// We could be smarter about this and only update the
// nodes that changed, if needed...
_rebuildWorkingSet();
_rebuildWorkingSet(true);
}

/**
* @private
* @param {string} path
*/
function _handlePathDeleted(path) {
// Rebuild the working set if any file or folder was deleted.
// We could be smarter about this and only delete affected
// items.
_rebuildWorkingSet(false);
}

function refresh() {
Expand Down Expand Up @@ -592,6 +605,10 @@ define(function (require, exports, module) {
_handleFileNameChanged(oldName, newName);
});

$(DocumentManager).on("pathDeleted", function (event, path) {
_handlePathDeleted(path);
});

$(FileViewController).on("documentSelectionFocusChange fileViewFocusChange", _handleDocumentSelectionChange);

// Show scroller shadows when open-files-container scrolls
Expand Down