Improvements to python script #240
Conversation
This makes the code more repetitive, but the dryrun should be as close as possible to the real thing, and should include checking original files.
Use 'default' option in argparse Don't use argparse's type=open
Process header and footer inside main() function Print header and footer before asking for confirmation Header or footer may not be present if rmlint was run with no_header or no_footer
Show progress indicator Print item message before exec_operation, not after
python script must be run as root for chown operations. Accordingly, target uid and gid must be given by user on command line, since they are not included in json output.
Add 100% Done at end Use same colors as shell script
|
Hello again. Thanks for that PR. I will try to review until the end of this week. |
lib/formats/py.py
Outdated
| print('\nPlease hit any key before continuing to shredder your data.', file=sys.stderr) | ||
| sys.stdin.read(1) | ||
|
|
||
| print('# This is a dry run. Nothing will be modified.') |
There was a problem hiding this comment.
Maybe add a colored # for this one too.
lib/formats/py.py
Outdated
| print('\nPlease hit any key before continuing to shredder your data.', file=sys.stderr) | ||
| sys.stdin.read(1) | ||
|
|
||
| print('# This is a dry run. Nothing will be modified.') |
There was a problem hiding this comment.
This should only be printed if we're doing a dry_run...
There was a problem hiding this comment.
Sorry about that, it's fixed!
| 'reset': "\x1b[0m" if USE_COLOR else "", | ||
| 'green': "\x1b[32;01m" if USE_COLOR else "", | ||
| 'blue': "\x1b[34;01m" if USE_COLOR else "" | ||
| } | ||
|
|
There was a problem hiding this comment.
original_check needs to be adjusted to work with duplicated directories (i.e. call rmlint --equal for them).
There was a problem hiding this comment.
I haven't attempted this since it would require knowing the path to rmlint, which isn't included in the json output. As you commented below, it isn't necessary for the python script to support every feature of the sh output, at least for now.
There was a problem hiding this comment.
True, but imho this is needed for correctness, not just as feature. But no blocker for this PR, I can implement that myself.
|
I didn't have that much time to check, but I had a quick over it. Overall it looks good, but some places There also some issues on my desk:
Just a note: The original idea was to output an easily modifiable python script that can be adjusted to ones need. So I'm not necessarily for supporting every case and for exactly reproducing the functionality of the |
Closes #239.