Eric Sunshine wrote: > On Mon, Jul 30, 2018 at 2:14 PM Jonathan Nieder <jrnieder@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxx/