Re: [PATCH 17/19] t4000-t4999: detect and signal failure within loop

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Dec 10, 2021 at 4:53 AM Fabian Stelzer <fs@gigacodes.de> wrote:
> On 09.12.2021 00:11, Eric Sunshine wrote:
> >-      for i in 1 2 3 4 5 6 7 8 9; do echo "$i$i$i$i$i$i"; done >x &&
> >+      for i in 1 2 3 4 5 6 7 8 9; do echo "$i$i$i$i$i$i" || return 1; done >x &&
>
> I understand why the `|| return` in loops makes sense. But for these very
> simple ones just using `echo` I'll probably be confused if a linter starts
> to complain about them (probably depending on how specific the lint error
> will be).

I understand your concern and had second thoughts about flagging this
sort of thing since there are so many loops which obviously shouldn't
fail, thus inserting `|| return 1` in them is just busy work to pacify
a linter which isn't smart enough to distinguish the important cases
from the unimportant ones. As such, I had some trouble justifying (to
myself) that this linter complaint is really a good idea, but
eventually convinced myself that it's worthwhile in the long run for
the same reasons I mentioned in my response to another patch in this
series. Namely, (1) there are good number of cases in which the loop
body is populated with "real" commands which could fail, so we really
do want to be told about missing `|| return 1` in general, and (2)
it's easier to have a single simple rule which we can point test
authors at ("end all loop bodies with `|| return 1` (or `|| exit 1` in
a subshell)") rather than complex rules laying out cases when you must
or need not use `|| return 1`.