Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tree-view] Remove deprecated usage of shell.moveItemToTrash #1109

Merged

Conversation

savetheclocktower
Copy link
Contributor

Identify the Bug

To delete files, tree-view uses Electron’s shell.moveItemToTrash method. That’s a synchronous method that was already deprecated in Electron 12 and is completely missing from Electron 30.

This can be fixed when we actually move to Electron 30, but there’s no reason not to backport it and fix it ahead of time.

Description of the Change

The asynchronous shell.trashItem is its replacement. Some small amount of rearrangement is needed to keep the logic working identically, but otherwise this was an easy fix.

Alternate Designs

None! This was the clear alternative.

Possible Drawbacks

In my estimation, the only things that could be affected here are things that aren’t covered by specs.

Verification Process

Specs should pass.

If you want to verify manually, try

  • deleting a file or directory in tree-view,
  • having it succeed,
  • ensuring the deleted item is removed from the tree, and
  • verifying that its parent folder becomes the new selected item.

Release Notes

  • [tree-view] Moved to a more modern API for file removal in preparation for an Electron upgrade.

The cause was Electron’s removal of the synchronous `shell.moveItemToTrash` method in favor of the asynchronous `shell.trashItem` method.

(This is a BACKPORT candidate; it should work just fine in Electron 12.)
Copy link
Contributor

@mauricioszabo mauricioszabo left a comment

Choose a reason for hiding this comment

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

I think I had to make this change for the newer Electron branch. Still, good idea to backport 👍

@savetheclocktower
Copy link
Contributor Author

I think I had to make this change for the newer Electron branch. Still, good idea to backport 👍

It might've gotten lost in the mix — I branched off the branch you said was the most recent and it wasn't present.

@savetheclocktower savetheclocktower merged commit 12d169d into pulsar-edit:master Oct 7, 2024
103 checks passed
@savetheclocktower savetheclocktower deleted the fix-tree-view branch October 7, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pulsar-next] Deleting files via tree-view doesn't work
2 participants