Hi, I've created a pull request based on the feedback: https://github.com/darktable-org/darktable/pull/975
Guillaume On Mon, Aug 24, 2015 at 9:58 AM, Guillaume Benny wrote: > Hi, > > I'll try to create a pull request; I've never used github, I'll try > not to mess-up! > > For the feedback: > > * Changing the title of the button: make sense. I'll also change the > message "do you really want to > physically delete 1 selected image from disk?" to "do you really want to > send 1 selected image to trash?" > > * In the patch, when trashing is not supported, it does delete the > file. I can show a message but is it allowed to have a GUI (a modal > dialog) in an other thread since this appends in a "job"? > > * Trash by default: make sense. > > Guillaume > > > > On Mon, Aug 24, 2015 at 5:30 AM, jeremy rosen wrote: >> Hello Guillaume >> >> we have been discussing your patch with the other devs and we are OK on >> principle, but there is a couple of feedback remarks, so we would like a >> follow-up patch >> >> * we want a single button in the selection lib named "trash" when the bin is >> activated and "delete" when it's not >> * when trashing fails, you silently revert to deleting. That's a bad idea. >> Please do a modal dialog with a message asking the user if he wants to >> delete or cancel instead >> * for consistancy reasons with other desktop apps, we think "trash on" >> should be the default (even if non of the dev actually have a desktop with >> trash) >> >> on a separate note, sending a patch on the ML is perfectly fine, but it's >> more practicall for us to review patches as github pull-requests... >> >> if you're ok with using pull requests, that would be nice to submit it that >> way... >> >> On Sun, Aug 9, 2015 at 8:11 PM, Guillaume Benny <[email protected]> wrote: >>> >>> Hi, >>> >>> I have been using darktable for a few months now and I really like it. I >>> recently thought of a feature that would be useful (to me at least): sending >>> deleted files to trash instead of permanently deleting them. >>> >>> Since I'm a C++ dev for my day job, I decided to try to implement it. >>> I've attached the patch for the master branch. It works well as far as I >>> could test it. >>> >>> I hope this can be merged! >>> >>> Guillaume >>> >>> >>> >>> >>> >>> ___________________________________________________________________________ >>> darktable developer mailing list to unsubscribe send a mail to >>> [email protected] >> >> ___________________________________________________________________________ darktable developer mailing list to unsubscribe send a mail to [email protected]
