Skip to content

Commit

Permalink
Merge pull request #1109 from savetheclocktower/fix-tree-view
Browse files Browse the repository at this point in the history 
[tree-view] Remove deprecated usage of `shell.moveItemToTrash`
  • Loading branch information
@savetheclocktower
savetheclocktower authored Oct 7, 2024
2 parents 433cf8f + a564e93 commit 12d169d
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 27 deletions.
48 changes: 34 additions & 14 deletions packages/tree-view/lib/tree-view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ class TreeView {
return dialog.attach();
}

removeSelectedEntries() {
async removeSelectedEntries() {
let activePath = this.getActivePath();
let selectedPaths, selectedEntries;
if (this.hasFocus()) {
Expand All @@ -896,13 +896,25 @@ class TreeView {
}
}

return atom.confirm({
atom.confirm({
message: `Are you sure you want to delete the selected ${selectedPaths.length > 1 ? 'items' : 'item'}?`,
detailedMessage: `You are deleting:\n${selectedPaths.join('\n')}`,
buttons: ['Move to Trash', 'Cancel']
}, (response) => {
}, async (response) => {
if (response === 0) { // Move to Trash
let failedDeletions = [];
let deletionPromises = [];

// Since this goes async, all entries that correspond to paths we're
// about to delete will soon detach from the tree. So we should figure
// out ahead of time which element we're going to select when we're
// done.
let newSelectedEntry;
let firstSelectedEntry = selectedEntries[0];
if (firstSelectedEntry) {
newSelectedEntry = firstSelectedEntry.closest('.directory:not(.selected)');
}

for (let selectedPath of selectedPaths) {
// Don't delete entries which no longer exist. This can happen, for
// example, when
Expand All @@ -913,17 +925,24 @@ class TreeView {
// but the parent folder is deleted first.
if (!fs.existsSync(selectedPath)) continue;

this.emitter.emit('will-delete-entry', { pathToDelete: selectedPath });
let meta = { pathToDelete: selectedPath };

// TODO: `shell.trashItem` is the favored way to do this.
if (shell.moveItemToTrash(selectedPath)) {
this.emitter.emit('entry-deleted', { pathToDelete: selectedPath });
} else {
this.emitter.emit('delete-entry-failed', { pathToDelete: selectedPath });
this.emitter.emit('will-delete-entry', meta);

let promise = shell.trashItem(selectedPath).then(() => {
this.emitter.emit('entry-deleted', meta);
}).catch(() => {
this.emitter.emit('delete-entry-failed', meta);
failedDeletions.push(selectedPath);
}
repoForPath(selectedPath)?.getPathStatus(selectedPath);
}).finally(() => {
repoForPath(selectedPath)?.getPathStatus(selectedPath);
});

deletionPromises.push(promise);
}

await Promise.allSettled(deletionPromises);

if (failedDeletions.length > 0) {
atom.notifications.addError(
this.formatTrashFailureMessage(failedDeletions),
Expand All @@ -934,10 +953,11 @@ class TreeView {
}
);
}
let firstSelectedEntry = selectedEntries[0];
if (firstSelectedEntry) {
this.selectEntry(firstSelectedEntry.closest('.directory:not(.selected)'));

if (newSelectedEntry) {
this.selectEntry(newSelectedEntry);
}

if (atom.config.get('tree-view.squashDirectoryNames')) {
return this.updateRoots();
}
Expand Down
2 changes: 1 addition & 1 deletion packages/tree-view/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"license": "MIT",
"engines": {
"atom": "*",
"node": ">=12"
"node": ">=14"
},
"private": true,
"dependencies": {
Expand Down
42 changes: 30 additions & 12 deletions packages/tree-view/spec/tree-view-package-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3042,7 +3042,10 @@ describe("TreeView", function () {
fileView.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1}));
treeView.focus();

spyOn(shell, 'moveItemToTrash').andReturn(false);
spyOn(shell, 'trashItem').andCallFake(() => {
return Promise.reject(false);
})

spyOn(atom, 'confirm').andCallFake((options, callback) => callback(0));

atom.commands.dispatch(treeView.element, 'tree-view:remove');
Expand Down Expand Up @@ -3093,7 +3096,9 @@ describe("TreeView", function () {
return atom.commands.dispatch(treeView.element, 'tree-view:remove');
});

waitsFor('directory to be deleted', () => callback.mostRecentCall.args[0].pathToDelete === dirPath2);
waitsFor('directory to be deleted', () =>
callback.mostRecentCall?.args?.[0].pathToDelete === dirPath2
);

return runs(function () {
const openFilePaths = atom.workspace.getTextEditors().map(editor => editor.getPath());
Expand Down Expand Up @@ -3122,7 +3127,9 @@ describe("TreeView", function () {
return atom.commands.dispatch(treeView.element, 'tree-view:remove');
});

waitsFor('directory to be deleted', () => callback.mostRecentCall.args[0].pathToDelete === dirPath2);
waitsFor('directory to be deleted', () =>
callback.mostRecentCall?.args?.[0].pathToDelete === dirPath2
);

return runs(function () {
const openFilePaths = atom.workspace.getTextEditors().map(editor => editor.getPath());
Expand Down Expand Up @@ -3156,7 +3163,9 @@ describe("TreeView", function () {
return atom.commands.dispatch(treeView.element, 'tree-view:remove');
});

waitsFor('directory to be deleted', () => callback.mostRecentCall.args[0].pathToDelete === dirPath2);
waitsFor('directory to be deleted', () =>
callback.mostRecentCall?.args?.[0].pathToDelete === dirPath2
);

return runs(function () {
const openFilePaths = atom.workspace.getTextEditors().map(editor => editor.getPath());
Expand Down Expand Up @@ -3187,30 +3196,33 @@ describe("TreeView", function () {
return atom.commands.dispatch(treeView.element, 'tree-view:remove');
});

waitsFor('directory to be deleted', () => callback.mostRecentCall.args[0].pathToDelete === dirPath2);
waitsFor('directory to be deleted', () =>
callback.mostRecentCall?.args?.[0].pathToDelete === dirPath2
);

return runs(function () {
const openFilePaths = atom.workspace.getTextEditors().map(editor => editor.getPath());
expect(openFilePaths).toEqual([undefined]);
});
});

it("focuses the directory's parent folder", function () {
it("focuses the directory's parent folder", async function () {
jasmine.useRealClock();
const callback = jasmine.createSpy('onEntryDeleted');
treeView.onEntryDeleted(callback);

dirView2.dispatchEvent(new MouseEvent('click', {bubbles: true, detail: 1}));
treeView.focus();

spyOn(atom, 'confirm').andCallFake((_, callback) =>callback(0));

atom.commands.dispatch(treeView.element, 'tree-view:remove');

waitsFor('directory to be deleted', () => callback.mostRecentCall.args[0].pathToDelete === dirPath2);
waitsFor('directory to be deleted', () =>
callback.mostRecentCall?.args?.[0].pathToDelete === dirPath2
);

runs(() => {
console.log('most recent', callback.mostRecentCall);
expect(root1).toHaveClass('selected')
expect(root1).toHaveClass('selected');
});
});
});
Expand Down Expand Up @@ -3764,14 +3776,20 @@ describe("TreeView", function () {
treeView.onEntryDeleted(callback);

const pathToDelete = treeView.selectedEntry().getPath();
expect(treeView.selectedEntry().getPath()).toContain(path.join('dir2', 'new2'));
expect(
treeView.selectedEntry().getPath()
).toContain(path.join('dir2', 'new2'));

const dirView = findDirectoryContainingText(treeView.roots[0], 'dir2');
expect(dirView).not.toBeNull();
spyOn(dirView.directory, 'updateStatus');

spyOn(atom, 'confirm').andCallFake((options, callback) => callback(0));
atom.commands.dispatch(treeView.element, 'tree-view:remove');

waitsFor('onEntryDeleted to be called', () => callback.mostRecentCall.args[0].pathToDelete === pathToDelete);
waitsFor('onEntryDeleted to be called', () =>
callback.mostRecentCall?.args?.[0].pathToDelete === pathToDelete
);

return runs(() => expect(dirView.directory.updateStatus).toHaveBeenCalled());
}));
Expand Down

0 comments on commit 12d169d

Please sign in to comment.