On Sat, Jul 06, 2024 at 03:15:32PM -0700, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > I'll post some patches in a moment:
> >
> > [1/3]: chainlint.pl: fix line number reporting
> > [2/3]: t/chainlint: add test_expect_success call to test snippets
> > [3/3]: t/chainlint: add tests for test body in heredoc
> >
> > with the idea that we'd apply your patch here on top of what Junio has
> > queued in jk/test-body-in-here-doc, and then these three on top.
>
> Would the final form be to have Eric's preparatory enhancement to
> chainlint and then these three first, and finally the "here-docs"
> conversion I queued from you earlier?
I had planned on top (leaving a brief moment where chainlint would
ignore the new format), but either way is fine.
My biggest question is around my patch 1 above:
- is it worth squashing in to Eric's patch? I didn't want to do that
without getting his OK on the approach.
- instead of bumping the line number in the caller, should the lexer
record the line number of the here-doc to be used later?
- the test harness in the Makefile strips the line numbers from the
chainlint output, so it's hard to verify those fixes. I saw them
only because the combination of the two bugs meant that the here-doc
had a "line 0" in it, which was enough to confuse the "sed"
invocation in the Makefile.
I did manually verify that it is OK after my fix, but do we want
that to be part of the chainlint tests? Just leaving the line
numbers in is a maintenance nightmare, since it depends on the order
of concatenating all of the tests together (so our "expect" files
would depend on all of the previous tests). But if we wanted to get
fancy, we could perhaps store relative offsets in the expect file. I
think it gets pretty complicated, though, since we print only
problematic lines.
I was going to give it a few days for Eric to chime in on those points,
and then assemble a final version for you to apply (but I certainly
don't mind if you want to pick up what's here in the meantime).
-Peff
