RhetTbull / osxphotos

Python app to work with pictures and associated metadata from Apple Photos on macOS. Also includes a package to provide programmatic access to the Photos library, pictures, and metadata.
MIT License
1.84k stars 94 forks source link

Add confirmation before cleanup deletion #733

Open infused-kim opened 1 year ago

infused-kim commented 1 year ago

Sorry, one more from me :)

Is your feature request related to a problem? Please describe. I ran an export and saw that 133 photos were missing.

So I wanted to run another export just for missing ones to add them to an album with: osxphotos export icloud_photo_library_export/ --load-config osxphotos.conf --missing --download-missing --add-missing-to-album "osxphotos_missing"

The app logged:

Did not find any photos to export
Cleaning up /Volumes/Photos/icloud_photo_library_export/icloud_photo_library_export

And then proceeded to delete for much longer than I would have expected. I assume it started to delete all the photos, since it cleans up anything that was not exported.

It was my mistake to run this command with cleanup = true, but it is probably an easy mistake to make for people who use the config option.

Describe the solution you'd like I think the app should say something like "Cleanup will delete XXX (X%) photos. Please confirm whether you would like to proceed [y/N]".

An even better option would be to give the user the option to show which files will be deleted in an fzf style UI, but that's probably quite a bit of effort to implement and perhaps a v2 iteration (if ever).

Describe alternatives you've considered I haven't been able to come up with an alternative, except to be very careful with the arguments.

Additional context My config was:

[export]
update = true
cleanup = true

report = "photos_export_report.sqlite"
append = true

db = "../Photos.photoslibrary"
tmpdir = "osxphotos_tmp"
directory = "{created.year}/{created.year}-{created.mm}"

theme = "light"
sidecar = [ "xmp",]

person_keyword = true
keyword_template = [
    "Album: {folder_album}",
    "{label}",
    "{favorite?is_favorite}",
    "is_{media_type}",
]
RhetTbull commented 1 year ago

I'll take a look at this. I agree it's an easy mistake to make. But many people use cleanup and I know several users use it in an unattended fashion so changing the behavior now might break some workflows. An interactive UI might be nice but I have many other priorities at the moment so I don't think I'll realistically get around to that.

RhetTbull commented 1 year ago

One option might be a new --cleanup-prompt which prompts the user before deleting files.

I always run with --dry-run any time I'm trying a new configuration. This would have caught it for you.