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

Issue1339: fix cross-device trashing #1782

Merged
merged 2 commits into from Jan 13, 2016

Conversation

@achadwick
Copy link

@achadwick achadwick commented Jan 13, 2016

#1339 was annoying me in 3.5.1, so I threw together a quick fix. Implementation is now a bit closer to http://standards.freedesktop.org/trash-spec/trashspec-latest.html. This handles my silly cross-volume music library fine even for broken or unwriteable trash structures, provided the fallback is still OK.

Andrew Chadwick added 2 commits Jan 13, 2016
Use shutil.move() to perform the move to trash: this falls back to a copy
when it needs to move a file between two devices.

Addresses #1339.
Consolidate trash folder finding and building into a single function
that makes the trash dir and its required subdirs if needed. This routine
also verifies that writes to the trash directory structure are possible
before returning it, and will fall back correctly to the home trash
folder if it has to.

Ref http://standards.freedesktop.org/trash-spec/trashspec-latest.html

Fixes #1339, or at least makes the exception go away
for many important cases.
@lazka
Copy link
Member

@lazka lazka commented Jan 13, 2016

Thanks for looking into it.

What about using https://lazka.github.io/pgi-docs/#Gio-2.0/interfaces/File.html#Gio.File.trash instead?

@achadwick
Copy link
Author

@achadwick achadwick commented Jan 13, 2016

Is QuodLibet moving to Gio everywhere? Then that would be a better API. trash_free_desktop() doesn't accept GFiles as parameters though, and I've not looked further.

Conversely, and may not be an issue: fallbacks are allowed by the spec, and are nice to have. However g_file_trash() isn't described as doing that. Instead, you'll have to catch a specific Gio.IOErrorEnum case via a GError and handle it with an additional direct-deletion dialog (like Nautilus).

@lazka
Copy link
Member

@lazka lazka commented Jan 13, 2016

Is QuodLibet moving to Gio everywhere? Then that would be a better API. trash_free_desktop() doesn't accept GFiles as parameters though, and I've not looked further.

No plans atm, but I can't think of a reason why we couldn't use it in this case (except pygobject+python3 only supporting utf-8 paths).

Conversely, and may not be an issue: fallbacks are allowed by the spec, and are nice to have. However g_file_trash() isn't described as doing that. Instead, you'll have to catch a specific Gio.IOErrorEnum case via a GError and handle it with an additional direct-deletion dialog (like Nautilus).

Yeah, deleting in case of an error wouldn't be a good idea :) We currently don't do fallbacks so that wouldn't be a regression at least.

lazka added a commit that referenced this pull request Jan 13, 2016
@lazka lazka merged commit 43ca94e into quodlibet:master Jan 13, 2016
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@lazka
Copy link
Member

@lazka lazka commented Jan 13, 2016

Merged for now, thanks. We should look into using Gio sometime...

@achadwick achadwick deleted the achadwick:issue1339-fix-crossdevice-trash branch Jan 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants