On Sat, Jul 6, 2024 at 2:55 AM Jeff King <peff@xxxxxxxx> wrote: > On Sat, Jul 06, 2024 at 02:47:57AM -0400, Eric Sunshine wrote: > > Of course, the more I think about it, the more I dislike relying upon > > what is effectively an accident of implementation; i.e. that in the > > typical case, the heredoc will already have been latched by the time > > ScriptParser::parse_cmd() has identified a `test_expect_success` > > command, due to the fact that ShellParser::parse_cmd() has that > > special case which peeks for `\n` immediately following some other > > command terminator. As such, fixing ScriptParser::parse_cmd() to only > > call check_test() once it is sure that a '\n' has been encountered is > > becoming more appealing, though it is of course a more invasive and > > fundamental change than the posted patch. > > Rats, I just agreed with your earlier email. ;) I am OK with the > slightly hacky version we've posted (modulo the fixes I discussed > elsewhere). But if you want to take a little time to explore the more > robust fix, I am happy to review it. The primary reason I said "the more I dislike relying upon ... an accident of implementation" is that this limitation is not documented anywhere other than in this email thread. That said, I don't mind the posted version of the patch being picked up. The "correct" approach can always be implemented atop it at a later time.
