Re: [PATCH] Supplant the "while case ... break ;; esac" idiom

Junio C Hamano [off-list ref] writes:

David Kastrup [off-list ref] writes:

quoted

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