Re: [PATCH v2 2/4] t7401: change test_i18ncmp syntax for clarity

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 12, 2020, 20:57 UTC
Message-ID: <xmqq1rkb60ql.fsf@gitster.c.googlers.com>
In-Reply-To: <20200812192737.13971-3-shouryashukla.oo@gmail.com>

Shourya Shukla <shouryashukla.oo@gmail.com> writes:

Show 13 quoted lines

> Change the test_i18ncmp syntax from 'test_i18ncmp actual expected' to
> 'test_i18ncmp expected actual' to align it with the convention followed
> by other tests in the test script.
> ...
> @@ -285,7 +285,7 @@ EOF
>  
>  test_expect_success '--for-status' "
>  	git submodule summary --for-status HEAD^ >actual &&
> -	test_i18ncmp actual - <<EOF
> +	test_i18ncmp - actual <<-EOF
>  * sm1 $head6...0000000:
>  
>  * sm2 0000000...$head7 (2):

This one does more than what the proposed log message explains, but it does not do enough at the same time.

If "actual vs expected order" is what this commit wants to fix, then "<<EOF" vs "<<-EOF" is outside the scope of it.

Personally, I think it is a good idea to update the end-of-heredoc marker to "<<-EOF", but the patch makes its readers wonder if the author of the patch understands why it is a good idea to begin with, because the end-of-heredoc marker is the only thing the patch changes, and it does not change the indentation of heredoc itself.

The whole point of using "<<-EOF" instead of "<<EOF" is so that the result becomes easier to read with indentation, e.g.

	test_i18ncmp - actual <<-EOF
	* sm1 $head6...0000000:

	* sm2 0000000...$head7 (2):
	...
	EOF

Compared to the original:

	test_i18ncmp - actual <<EOF
* sm1 $head6...0000000:

* sm2 0000000...$head7 (2):
...
EOF

it would be easier to read.