Re: [PATCH v8 3/6] t3321: add test cases about the notes stripspace behavior

>> ---
>>  t/t3321-notes-stripspace.sh | 291 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 291 insertions(+)
>>  create mode 100755 t/t3321-notes-stripspace.sh
>
>Looks quite thorough.

>> diff --git a/t/t3321-notes-stripspace.sh b/t/t3321-notes-stripspace.sh
>> new file mode 100755
>> index 00000000..7c26b184
>> --- /dev/null
>> +++ b/t/t3321-notes-stripspace.sh
>> @@ -0,0 +1,291 @@
>> +#!/bin/sh
>> +#
>> +# Copyright (c) 2007 Teng Long
>
>It's 2023 now.

>> +test_expect_success 'add note by editor' '
>> +	test_when_finished "git notes remove" &&
>> +	cat >expect <<-EOF &&
>> +		first-line
>> +
>> +		second-line
>> +	EOF
>> ...
>> +test_expect_success 'append note by specifying multiple "-m"' '
>> +	test_when_finished "git notes remove" &&
>> +	cat >expect <<-EOF &&
>> +	first-line
>> +
>> +	second-line
>> +	EOF
>
>Inconsistent indentation confuses readers if the author meant
>something unexplained by the difference between the two.  Stick to
>one style (I personally prefer the "indent to the same level as the
>starting 'cat' and ending 'EOF'" but it is OK to pick the other one,
>as long as it is consistent within a single test script).

>> +	cat >note-file <<-EOF &&
>> +		${LF}
>> +		first-line
>> +		${MULTI_LF}
>> +		second-line
>> +		${LF}
>> +	EOF
>
>This is a bit misleading, as there are TWO blank lines before the
>"first-line" (one from the here text itself, the other is from
>${LF}).  
>
>I do not think it matters too much, because the point of stripspace
>is to remove any number of leading or trailing blank lines, and
>squash one or more blank lines elsewhere into one, so having two
>blank lines at the beginning or at the end of the file is just as
>good an example as having a single blank line.  I am mentioning it
>primarily because I had to spend some time thinking about ways to
>make it less misleading.