Junio C Hamano <gitster@xxxxxxxxx> writes:
> 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).
Uh, it is the only version with a syntax error.
> 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.
Basically
while A && B || break; do C && D && E || break; done
> 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.
As a completely irrelevant side note: the autoconf documentation
mentions that "false" is more portable than "true" since calling it
returns a non-zero exit status even when it is not installed or
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.
I guess this has been a misunderstanding: for me, personally, the
break was the issue: I don't like breaking out of a condition, since
breaking for me is an action. I just used the fact that the BSD
shells happen not to grok the constructs (and actually through a
somewhat similar confusion between condition and action) to leverage
my dislike of this construct and propose a patch.
> The reason I took your patch and proposed commit log message
> (almost) as-is was because you rewrote "case" to "test".
Uhm, ok. It was a case of realizing "hm, this does not really look
much nicer" before I chose to switch to "test". In fact, there is one
case statement remaining which I rewrote in the previously discussed
manner, and it did not strike me as being much prettier. So maybe I
somewhat misjudged the core of my offended sense of aesthetics, but
the impetus of the discussion still carried into the commit message.
Alea iacta est ("The SHA-1 has been established").
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
-
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
