Replace permadelete with send2trash #801
Conversation
|
I don't really like this non-obvious change of the "Delete" function semantic, it looks the same in the UI but suddenly has a (slightly) different effect. I would much prefer to also rename "Delete" to "Move to trash" (and possibly change the icon, waiting for #799 to be merged is advised) to unmistakably convey what it does. Alternative way to solve this would be to extent the existing confirmation dialog. Instead of having only one "OK" Button, there are two for "Move to trash" and "Delete without trash" respectively. Or a checkbox in the dialog for "Bypass the trash and delete it instantly". I have mixed feelings for using the trash for masstag profiles. The users direct intent is to remove a profile, not a file. The fact that we store profiles in files (one file per profile) is normally hidden from the user and as such I would classify as "application internal" and continue to use Just as a FYI: The "Delete without confirmation" function was already removed in #793. And always remember kids: No backup, no pity. And RAID is not a backup. |
There was a problem hiding this comment.
I agree with what @corubba says about changing the semantic of "delete".
we should at least:
- rename all the python code from
delete...totrash...or something similar to convey in code that's no longer going to remove the file - rename all labels in menus, drop downs, interactive parts of the UI from "Delete" to "Move to trash"
- update the icon to show a trash can or something similar to what other apps do in this case
Im ok with just replacing the delete functionality with a move-to-trash one, without complicating the confirmation menu; i see already people asking to have a config option in settings to "always delete" or "always move to trash" or "always have selected the tick to move to trash" etc etc, so let's not go there :)
regarding the profile deletion, im as well unsure if we should just unlink or move to trash
@WatDuhHekBro i would too way for #799 to be merged to use the appropriate icon -- thanks!
| @@ -4,6 +4,7 @@ configobj==5.0.8 | |||
| mutagen==1.46.0 | |||
| pyparsing==3.0.9 | |||
| unidecode==1.3.6 | |||
| Send2Trash | |||
| @@ -22,7 +22,7 @@ puddletag uses several third-party modules to performs its tasks: | |||
| - [Mutagen](https://pypi.org/project/mutagen/), used as the tagging lib | |||
| - [Chromaprint](http://acoustid.org/chromaprint) (recommended), for AcoustID support | |||
| - [unidecode](https://pypi.org/project/Unidecode/) | |||
|
|
|||
| - [send2trash](https://pypi.org/project/Send2Trash/) | |||
There was a problem hiding this comment.
please keep the extra empty line
|
@WatDuhHekBro would you considering looking into update this pr? |
This PR addresses #223 by replacing all instances of
os.remove()(within puddletag itself, notdocsrc) withsend2trash().I don't think an option to enable permadeleting is necessary because if a user wants to permadelete a file, they can do so within their file manager, and they won't run into any unexpected results there.
This should affect all delete operations, including
[Shift][Del]which currently doesn't prompt the user for confirmation to delete selected files.An undo option seems to be much trickier and platform-dependent to implement, though it isn't really necessary.