having `--dry-run` for `trash-put`

[mmahmoudian]

Is your feature request related to a problem? Please describe. For scripting and even sometimes for daily usage (esp. when running with xargs), using trash-put is like driving blind as user don't know what is passed to it and what will be deleted.

Describe the solution you'd like I believe, similar to #93, it would be essential to have --dry-run for this command. This comes handy when combined with --verbose.

Describe alternatives you've considered The alternative is to have --interactive similar to what rm has.

Additional context -- Not applicable --

P.s: I just got to know this tool recently and it is pretty handy. Thanks for maintaining it.

[andreafrancia]

Please describe an actual example where you have needed this feature and explain what you could do with this new option and what you can not do without. Expose this or these examples in the form of command prefix with "$ " and output of these commands as the were launched in the terminals.

[mmahmoudian]

describe an actual example where you have needed this feature:

Of course I am not willing to put the exact files and their names on Github, so I have to come up with a work-around: creating toy data. I hope you find this detailed example satisfactory

In this hypothetical example, that is similar to my usecase for managing backups, we have a folder with bunch of backup files that have been created in a course of few weeks. We want to keep the latest 3 backup files and trash the rest.

Lets create a toy environment:

#!/bin/env bash

# create a temp folder
OUR_TMP_DIR=$(mktemp --directory)

for i in {1..15}; do
    # create random files
    mktemp --tmpdir="${OUR_TMP_DIR}"
    # just to have some time difference between each file so we can sort them later
    sleep 0.5s
done

/tmp/tmp.aPSCVlOFbn/tmp.eaKTQu9NIS
/tmp/tmp.aPSCVlOFbn/tmp.MBQb4SsNvg
/tmp/tmp.aPSCVlOFbn/tmp.XYLkI8TCI1
/tmp/tmp.aPSCVlOFbn/tmp.BWWDzUtlJP
/tmp/tmp.aPSCVlOFbn/tmp.26hPQd768F
/tmp/tmp.aPSCVlOFbn/tmp.csPLpOIfFH
/tmp/tmp.aPSCVlOFbn/tmp.TsrKGMbOOq
/tmp/tmp.aPSCVlOFbn/tmp.vyjJT6W68a
/tmp/tmp.aPSCVlOFbn/tmp.d2xOoNRi0b
/tmp/tmp.aPSCVlOFbn/tmp.7BwNoSqCuu
/tmp/tmp.aPSCVlOFbn/tmp.JJmGWQOqGk
/tmp/tmp.aPSCVlOFbn/tmp.cqpEK2BmZm
/tmp/tmp.aPSCVlOFbn/tmp.oiY7yYiLJY
/tmp/tmp.aPSCVlOFbn/tmp.xVLmgv1v6P
/tmp/tmp.aPSCVlOFbn/tmp.oF5H55l32V

Now you think this is easy but:

$ ls -1 --sort="time" --reverse /tmp/tmp.aPSCVlOFbn| head -n -3  | xargs -I {} trash-put "/tmp/tmp.aPSCVlOFbn/{}"
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.eaKTQu9NIS' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.MBQb4SsNvg' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.XYLkI8TCI1' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.BWWDzUtlJP' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.26hPQd768F' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.csPLpOIfFH' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.TsrKGMbOOq' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.vyjJT6W68a' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.d2xOoNRi0b' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.7BwNoSqCuu' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.JJmGWQOqGk' trashed in /tmp/.Trash-1001
trash-put: '/tmp/tmp.aPSCVlOFbn/tmp.cqpEK2BmZm' trashed in /tmp/.Trash-1001

there will be no output because trash-put does not have --verbose

Now we can check how it went

$ ls -alh /tmp/tmp.aPSCVlOFbn
total 0
drwx------  2 mehrad mehrad 100 Apr 17 20:41 ./
drwxrwxrwt 28 root   root   820 Apr 17 20:41 ../
-rw-------  1 mehrad mehrad   0 Apr 17 20:41 tmp.oF5H55l32V
-rw-------  1 mehrad mehrad   0 Apr 17 20:41 tmp.oiY7yYiLJY
-rw-------  1 mehrad mehrad   0 Apr 17 20:41 tmp.xVLmgv1v6P

It would have been much nicer, safer and more user-friendly to e able to add --dry-run and --verbose to the trash-put and know if xargs is passing the correct format to the trash-put.

---

explain what you could do with this new option

Without --dry-run it would be risky to use trash-cli while developing scripts and the dev should go an extra mile and put things into variables and check the edge cases. With --dry-run the files are not deleted and just the output is produced (e.g when combined with --verbose). Pretty much the same use-case as explained in #93 .

and what you can not do without.

Without --dry-run user have to be very careful on what is getting passed by xargs (or if used in find ... -exec), or instead generate the file names separately, check them and then pass it to trash-put. Additionally without --dry-run the user would not know if for example /tmp can be trashed. they have to either read the code, or run the trash-put /tmp and then see the error, but as you know, blindly running commands on / folders are super risky and wrong. With --dry-run, the user have a safely net to test without potentially breaking their system.

Now the actual issue I faced: I had -f in the ls command, and that meant that the ./ and ../ were passed to the pipe. Basically the result was disastrous because I made a rookie mistake of not checking the output of head and directly passed it to xargs. Having safe-guards like --dry-run can genuinely help with preventing such mistakes.

As a side-note, I believe trash-put ../ should be illegal and should throw error, but that's another story for another day :)

[andreafrancia]

As a side-note, I believe trash-put ../ should be illegal and should throw error, but that's another story for another day :) This shouldn't have happened.

I just published a recent version with this regression removed, please download the 0.24.4.17 version, you can download it using pip install trash-cli.

[andreafrancia]

Now the actual issue I faced: I had -f in the ls command, and that meant that the ./ and ../ were passed to the pipe. Basically the result was disastrous because I made a rookie mistake of not checking the output of head and directly passed it to xargs. Having safe-guards like --dry-run can genuinely help with preventing such mistakes.

I understand your need. You can add "echo" to work around the missing of the --dry-run option.

$ ls -1 --sort="time" --reverse /tmp/tmp.aPSCVlOFbn| head -n -3  | xargs -I {} echo trash-put "/tmp/tmp.aPSCVlOFbn/{}"

If you really need this feature and if you feel up to it you can try adding the '--dry-run' option to your own copy of trash-cli. I think that should be a matter of adding an if statement in the trash_file_in method of the Janitor class in the zone of line 74:

        persisting_job = self.persister.try_persist(trashinfo_data.value())
        trashed_file = self.executor.execute(persisting_job, log_data)
        trashed = self.trash_dir.try_trash(trashee.path, trashed_file)
        if isinstance(trashed, Left):
            return make_error(trashed)

To carry the value of the option you can add another field to the class Context