markubiak / wallpaper-reddit

Downloads and sets wallpapers pulled from reddit.com
GNU General Public License v3.0
113 stars 42 forks source link

Return status of os.system calls are not checked for successful execution. #29

Closed qfulsher closed 7 years ago

qfulsher commented 7 years ago

When wallpaper-reddit runs the linux_wallpaper() call it doesn't check the return status of any of the os.system() calls. If there is an unsuccessful os.system() call then the program will still report that the command was run even though the verbose mode will print error messages for unrecognized arguments.

markubiak commented 7 years ago

Good catch. I'm not quite following what you mean about verbose mode, however.

qfulsher commented 7 years ago

With the -v argument it will say that there was some sort of problem running the setcommand (eg. Unrecognized arguments, etc). Without the verbose mode turned on the script simply reports that the command run whether it returns successfully or not.

markubiak commented 7 years ago

Alright, I've committed a change that checks to make sure all os.system() commands return an exit code of 0. Seems to work on my testing. It's in the dev branch, commit dd4adc8993da04261b37ed5b62973251efe11d5c

markubiak commented 7 years ago

I'm going to reopen this because I'd really like to work on migration from os.system() to subprocess. Subprocess actually throws errors and other nice things. I'm going to try to push out an update in the next week with this migration and Pillow 4 compatibility

markubiak commented 7 years ago

I've pushed my changes into the dev branch, the latest commit should be 7bc78628748ee612dc6bcff346eff15f9b7531bb. This seems like a much cleaner solution and is working great across my tests, if you have the time I'd appreciate if you could give it a test before I put it into a release on Tuesday.

markubiak commented 7 years ago

Moved to main branch, closed.