Re: [PATCH v2 01/10] t/test-lib: teach --chain-lint to detect broken &&-chains in subshells

[Jonathan Nieder <jrnieder@gmail.com>]

Eric Sunshine wrote:
> On Mon, Jul 30, 2018 at 2:14 PM Jonathan Nieder <jrnieder@gmail.com> wrote:

>>         (
>>                 chks_sub=$(cat <<TXT | sed '\''s,^,sub dir/,'\''
>> $chks
>> TXT
>> ) &&
>>
>> Ugly quoting, useless use of "cat", etc, aside, I don't think it's
>> missing any &&.  Hints?
>
> Yes, it's a false positive.
>
> The subshell linter would normally fold out the here-doc content, but
> 'sed' isn't a proper programming language, so the linter can't
> recognize arbitrary here-doc tags. Instead it has hard-coded knowledge
> of the tags commonly used in the Git tests, specifically EOF, EOT, and
> INPUT_END.

Oh, hmm.  I also see some others (outside subshells, though):

	EXPECT_END
	FRONTEND_END
	END_PART1
	SETUP_END
	EOF2
	EXPECTED
	END_OF_LOG
	INPUT_END
	END_EXPECT

I wonder if it should look for something like [A-Z][A-Z_]* to catch
all of these.

> The linter also deals with multi-line $(...) expressions, however, it
> currently only recognizes them when the $( is on its own line.

That's reasonable, especially if "on its own line" means "at end of
line".

What would help most is if the error message could explain what is
going on, but I understand that that can be hard to do in a sed
script.

[...]
> I could try to update the linter to not trip over this sort of input,
> however, this test code is indeed ugly and difficult to understand,
> and your rewrite[1] of it makes it far easier to grok, so I'm not sure
> the effort would be worthwhile. What do you think?

I'd be happy to look over a change that handles more here-doc
delimiters or produces a clearer message for tests in poor style, but
I agree with you that it's not too important.

Thanks for looking it over.

Sincerely,
Jonathan

> [1]: https://public-inbox.org/git/20180730190738.GD156463@aiede.svl.corp.google.com/