Re: [PATCH] chainlint.pl: recognize test bodies defined via heredoc

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 10, 2024, 02:38 UTC
Message-ID: <CAPig+cQ5FgEADF+v+TAY4aqg-rC1OAx_MzuqK=a7MJDYDLLEPA@mail.gmail.com>
In-Reply-To: <20240710004817.GB1526498@coredump.intra.peff.net>

On Tue, Jul 9, 2024 at 8:48 PM Jeff King <peff@peff.net> wrote:

Show 10 quoted lines

> On Mon, Jul 08, 2024 at 04:06:47PM -0400, Eric Sunshine wrote:
> > One thing we may want to measure is how much extra time we're wasting
> > for the (very) common case of latching heredoc bodies only to then
> > ignore them. In particular, we may want to add a flag to ShellParser
> > telling it whether or not to latch heredoc bodies, and enable that
> > flag in subclass ScriptParser, but leave it disabled in subclass
> > TestParser since only ScriptParser currently cares about the heredoc
> > body.
>
> I doubt it's much to hold on to a few extra small buffers.

I was more concerned about the extra substr() consuming additional CPU time and inflating wall-clock time.

> So it does seem to make a small difference, but we're within the noise.

Okay. Thanks for measuring.

Show 8 quoted lines

> > In the long run, I think we probably want to build a full parse tree,
> > attach relevant information (such as a heredoc body) to each node, and
> > then walk the tree, rather than trying to perform on-the-fly lints and
> > other operations on the token stream as is currently the case.
> > [...]
>
> Yeah, all of that sounds very sensible long term, but probably not worth
> worrying about for this topic.

Agreed.