David Kastrup <dak@xxxxxxx> writes: > I am somewhat taken aback that a commit message considered offensive > (though I still have a problem understanding why and certainly did not > intend this) has been committed into master without giving me a chance > to amend it. Heh, that's simple. I changed my mind ;-) When A and B test for preconditions, and C, D, and E are operations with error reports as their side effects, we can write our loop in these forms: (1) while A && B && C && D && E || false; do :; done (2) while A && B && C && D && E || break; do :; done (3) while A && B; do C && D && E || break; do :; done (4) while :; do A && B && C && D && E || break; done and all of them are equivalent. But obviously the only sane version is (3). If your complaint were against things like (1) and (2), I would have completely agreed with you. If you want "effects", you do so between do and done. Although you can use break between do and done if you need to conditionally break out of the loop after causing some effect there, between while and do is where you are only supposed to decide if you want to break out of the loop without causing "effects". But what you were complaining about was different. If we were to ignore broken shells that do not return success from a case statement with no matching pattern, the following two are equivalent: while case "$sth" in foo) break ;; esac; do ...; done while case "$sth" in foo) false ;; esac; do ...; done Their "case" are used to decide if you want to break out of the loop; the former is (1) being a bit more explicit, and (2) used to be a bit more efficient when false was not built-in. Now the latter reason is mostly historical and it is not a valid reason to choose the former over the latter anymore. But that does not make it any more confusing than the latter to a person who knows what "break" means in a loop. An explicit 'break' is still more, eh,... explicit ;-) But the "break" never was the issue. Return value of "case" was. The reason I took your patch and proposed commit log message (almost) as-is was because you rewrote "case" to "test". That IS an improvement, especially in the presense of a shell in the field that does not implement case statement correctly, and you talk about that in the later part of the commit log message. The only "offending" part was "I consider...ugly", which is your opinion but I think you as the patch author deserve to express that. I do not think it would not have helped the FreeBSD shell a bit if you removed that "ugliness" by merely replacing "break" with "false", so I think the comment was not just offending but irrelevant, though. All the rest of your commit message is correct. The spec of "case" might not be obvious to everybody that it ought to return success when no pattern matched. And I found your wording to fold the bug decription of some BSD shells there amusing ;-) - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
