sahib / rmlint

Extremely fast tool to remove duplicates and other lint from your filesystem
http://rmlint.rtfd.org
GNU General Public License v3.0
1.87k stars 130 forks source link

Add set -e to the bash script? #469

Open ghost opened 3 years ago

ghost commented 3 years ago

In general, set -e is not such a great thing, as it requires extra care when working with pipes. (http://mywiki.wooledge.org/BashFAQ/105)

However, rmlint.sh does not seem to be using a lot of pipes, and generally is using a very fragile technique of bash script generation, so I expect set -e to bring more good than harm, in case some strange file names are escaped incorrectly, for example.

SeeSpotRun commented 3 years ago

I like the link. Especially the conclusions at the end.

Will test it and see if it breaks any nosetests, and then maybe come back and discuss some more.

SeeSpotRun commented 3 years ago

Testing result: didn't break any of the existing nosetests.

@sahib: any philosophical or technical objections?

sahib commented 3 years ago

I'm a fan of the proposal. There's no good reason set -e isn't already there, mostly because I didn't know about it many years ago. You might also think about adding -u (abort if expanding undefined variables) and maybe also -o pipefail. I can think of some drawbacks though, which need to be evaluated first: