Allan McRae wrote:
I have looked at the two patches and the good news is that I can spot nothing wrong! I just have a few comments about style that I would like to discuss.
[...]
My suggestion is that any thing with text (i.e. not a pure variable) is quoted. I know this is excessive in some cases (e.g. the last case) but the only exception I would be happy with is tests that are pure paths with only added "/" (e.g. $startdir/$file). Even then, maybe quotes would be nicer... I am happy to be debated on this.
To quote myself [1]:
In my opinion, it is often not obvious whether they are required or not which is why I tended to use more quotes than actually were needed. I am fairly familiar with quoting by now and can use both "quoting styles", but I think it would probably be a good idea to decide for one. Using just as much quotes as required would be a little bit shorter, but using quotes `where possible` may be a little bit more foolproof.
Meaning that I am fine with quoting strings generally. [1] http://mailman.archlinux.org/pipermail/pacman-dev/2009-November/010115.html
I wonder if the tests of return values should explicitly test for "== 0" or "!= 0". e.g. these test have become less clear to me when I read the code.
- if [ $ret -gt 0 ]; then + if (( ret )); then
- elif [ $ret -ne 0 ]; then + elif (( ret )); then
Yep, sounds reasonable, especially since we already explicitly test all (or at least many) other non-boolean variables.