Skip to content

Commit

Permalink
Try to work around a shell.trashItem bug on Windows…
Browse files Browse the repository at this point in the history
…by proxying to the main process.

NOTE: We might also have to do this for `shell.showItemInFolder`.

See #1123.
  • Loading branch information
savetheclocktower committed Jan 5, 2025
1 parent a99dd97 commit b863774
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/tree-view/lib/tree-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ class TreeView {

this.emitter.emit('will-delete-entry', meta);

let promise = shell.trashItem(selectedPath).then(() => {
let promise = atom.trashItem(selectedPath).then(() => {
this.emitter.emit('entry-deleted', meta);
}).catch(() => {
this.emitter.emit('delete-entry-failed', meta);
Expand Down
16 changes: 15 additions & 1 deletion src/application-delegate.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ module.exports = class ApplicationDelegate {

pickFolder(callback) {
const responseChannel = 'atom-pick-folder-response';
ipcRenderer.on(responseChannel, function(event, path) {
ipcRenderer.on(responseChannel, function (event, path) {
ipcRenderer.removeAllListeners(responseChannel);
return callback(path);
});
Expand Down Expand Up @@ -132,6 +132,20 @@ module.exports = class ApplicationDelegate {
return ipcHelpers.on(ipcRenderer, 'did-leave-full-screen', callback);
}

trashItem(filePath) {
// A simple wrapper around `shell.trashItem`, which currently can only be
// called from the main process on Windows.
return ipcRenderer.invoke('trashItem', filePath).then(({ outcome, error, result }) => {
if (outcome === 'success') {
// `result` is undefined, but we might as well guard against an
// Electron API change in the future.
return result;
} else if (outcome === 'failure') {
return Promise.reject(error);
}
});
}

async openWindowDevTools() {
// Defer DevTools interaction to the next tick, because using them during
// event handling causes some wrong input events to be triggered on
Expand Down
10 changes: 10 additions & 0 deletions src/atom-environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,16 @@ class AtomEnvironment {
return this.setFullScreen(!this.isFullScreen());
}

// A proxy to `shell.trashItem` — which ought to work in the renderer, but
// suffers from a bug on Windows. We work around it by delegating to the main
// process.
//
// Undocumented for now, but may eventually be an official part of the API
// for when community packages need to delete files.
trashItem(filePath) {
return this.applicationDelegate.trashItem(filePath);
}

// Restore the window to its previous dimensions and show it.
//
// Restores the full screen and maximized state after the window has resized to
Expand Down
24 changes: 24 additions & 0 deletions src/main-process/atom-application.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,30 @@ ipcMain.handle('isDefaultProtocolClient', (_, { protocol, path, args }) => {
ipcMain.handle('setAsDefaultProtocolClient', (_, { protocol, path, args }) => {
return app.setAsDefaultProtocolClient(protocol, path, args);
});

// Handle file deletion requests.
//
// Works around https://github.com/electron/electron/issues/29598, which seems
// to be the cause of failed deletion attempts on Windows.
ipcMain.handle('trashItem', async (_, filePath) => {
// We can't toss a promise over the wall, so we'll `await` it on our side and
// report the progress back to the renderer.
//
// If we return an `Error` object from this handler in the case of error,
// `ipcRenderer.invoke` will detect it and wrap it with its own explanation.
// We want to preserve the original error and hide the implementation
// details, so we instead return an object with an explicit `outcome`
// property to avoid this behavior.
try {
// `shell.trashItem` resolves with an empty value on success…
let result = await shell.trashItem(filePath);
return { outcome: 'success', result };
} catch (error) {
// …and rejects on failure.
return { outcome: 'failure', error }
}
});

// The application's singleton class.
//
// It's the entry point into the Pulsar application and maintains the global state
Expand Down

0 comments on commit b863774

Please sign in to comment.