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

From: Derrick Stolee <derrickstolee@github.com>
Date: Apr 18, 2022, 12:16 UTC
Message-ID: <8ec81980-32a4-dbd7-a788-e7f5a012ba09@github.com>
In-Reply-To: <pull.1218.git.1650096550631.gitgitgadget@gmail.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.

Show 6 quoted lines

>  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.)

Show 10 quoted lines

>  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