Re: [PATCH v2 2/2] sparse-checkout: document interactions with submodules

From: Derrick Stolee <stolee@gmail.com>
Date: Dec 30, 2019, 13:11 UTC
Message-ID: <d934ac0a-d6d7-cf65-5d61-1196abc63ff8@gmail.com>
In-Reply-To: <CAPig+cTvFW_84TqKsOPjjBM37cX1OL9uX15APcEYpuKVjvM+dg@mail.gmail.com>

On 12/27/2019 3:20 PM, Eric Sunshine wrote:

Show 20 quoted lines

> On Fri, Dec 27, 2019 at 1:48 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
>> @@ -340,4 +340,32 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '
>> +test_expect_success 'interaction with submodules' '
>> +       ...
>> +       cat >expect <<-EOF &&
>> +               a
>> +               folder1
>> +               modules
>> +       EOF
> 
> You would normally use \-EOF rather than -EOF to make it clear that no
> interpolation is needed/expected within the here-doc body. However,
> this script is already full of -EOF when \-EOF ought to be used, so
> being consistent with existing tests may override an objection.
> 
> Likewise, please note for future reference that the usual way
> here-docs are formatted in Git test scripts is to indent the body of
> the here-doc to the same level as the command which opens it. That is:

Thanks for pointing out the difference, except...

>     cat >expect <<\-EOF &&

This should be <<-\EOF

Show 8 quoted lines

>     a
>     folder1
>     modules
>     EOF
> 
> But, again, this script is already full of these malformatted
> here-docs, so maintaining consistency with the existing test in the
> script is probably okay.

Hm. Having these lines have the same tabbing hurts my eyes (it is harder to see where the contents end, much like if we didn't tab inside a subshell or an if block).

This is also a place where we are inconsistent, and it's not just my fault for writing the test script in my own style. Here are a few scripts that tab the same way as here:

	t0008-ignore.sh
	t4124-apply-ws-rule.sh
	t9400-diff-highlight.sh

These are definitely the minority. I just mention them so anyone who does a cleanup of this whitespace inconsistency takes the time to look for all examples.

For now, I'll fix the here-doc interpolation issue for this test, but keep the whitespace matching the rest of the test script. I'll add this concern to my next series.

Thanks, -Stolee