On Tue, Dec 17, 2019 at 7:02 AM Denton Liu <liu.denton@xxxxxxxxx> wrote:
> Since output is silenced when running without `-v` and debugging output
> is useful with `-v`, remove redirections to /dev/null as it is not
> useful.
>
> Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx>
> ---
> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
> @@ -5,7 +5,7 @@ test_description='CRLF conversion'
> has_cr() {
> - tr '\015' Q <"$1" | grep Q >/dev/null
> + tr '\015' Q <"$1" | grep Q
> }
I'm not sure that I agree with this change since dropping >/dev/null
doesn't improve the situation when someone is trying to debug a
failing test. What this change will do is fill the -v output with a
bunch of lines ending with "Q" when CR is expected -- the normal
_successful_ case for about half the calls to has_cr() -- so -v output
will become a lot noisier without really adding much, if any,
debugging value. If you really want to help people trying to diagnose
failures, I could see you replacing has_cr() with two new functions
which actually provide useful diagnostic output; for instance
something like:
expect_cr () {
if ! tr '\015' Q <"$1" | grep Q >/dev/null
then
echo "missing CR termination in: $1" >&2 &&
return 1
fi
}
expect_no_cr () {
if tr '\015' Q <"$1" | grep Q >/dev/null
then
echo "unexpected CR termination in: $1" >&2 &&
return 1
fi
}
However, I'm not convinced that introducing and debugging these
functions is worth the effort over simply dropping this patch from the
series.
