Re: [PATCH] area: /t/t4204-log.sh, partially modernized test script t4202

[Derrick Stolee <derrickstolee@github.com>]

On 4/16/2022 4:09 AM, Jack McGuinness via GitGitGadget wrote:> From: Jack <jmcguinness2@ucmerced.edu>
> 
> Remove test body indentations made with spaces, replace with tabs.

This goal has a subtle issue that I'll point out below.

> Remove blank lines at start and end of test bodies.
These are very easy to review.
>  test_expect_success 'decorate-refs-exclude with glob' '
> @@ -2037,7 +2016,7 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 miss
>  	GNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&
>  	grep "^|\\\  merged tag" actual &&
>  	grep -e "^| | gpgsm: certificate not found" \
> -	     -e "^| | gpgsm: failed to find the certificate: Not found" actual

In this example, the "\" means that the command is continuing to the next
line. For such continuations, it is OK to use something other than a full
tab, for stylistic reasons.

Specifically here, the "-e" arguments have vertical alignment in the old
version.

Since the leading whitespace here is a tab followed by five spaces, it
would not appear as an error in "git log --check".

(This is all to say, leave this line as-is.)
>  test_expect_success 'log --end-of-options' '
> -       git update-ref refs/heads/--source HEAD &&
> -       git log --end-of-options --source >actual &&
> -       git log >expect &&
> -       test_cmp expect actual
> +	   git update-ref refs/heads/--source HEAD &&
> +	   git log --end-of-options --source >actual &&
> +	   git log >expect &&
> +	   test_cmp expect actual
>  '

Here, you have issues because you are replacing seven spaces with a tab
PLUS three spaces. While that won't be a complaint in "git log --check",
it is incorrect in this case. It should be a single tab on the left of
these lines.

I feel like maybe you did a find/replace taking four spaces and converting
them to tabs. The real situation is more subtle here.

Thanks,
-Stolee