mbox series

[0/3] chainlint: emit line numbers alongside test definitions

Message ID pull.1413.git.1668013114.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series chainlint: emit line numbers alongside test definitions | expand

Message

Jean-Noël Avila via GitGitGadget Nov. 9, 2022, 4:58 p.m. UTC
When chainlint detects problems in a test, it prints out the name of the
test script, the name of the problematic test, and a copy of the test
definition with "?!FOO?!" annotations inserted at the locations where
problems were detected. Taken together this information is sufficient for
the test author to identify the problematic code in the original test
definition. However, in a lengthy script or a lengthy test definition, the
author may still end up using the editor's search feature to home in on the
exact problem location.

This patch series further assists the test author by displaying line numbers
alongside the annotated test definition, thus allowing the author to jump
directly to each problematic line.

This feature was suggested by Ævar[1]. I suspect that Ævar's next nerd-snipe
attempt may be to have problems emitted in "path:line#:col#: message" format
to allow editors to jump directly to the problem without the user having to
type in the line number manually.

This is atop "es/chainlint-output"[2].

(Note to self: Fortify against Ævar's nerd-snipe blacklist evasion.)

FOOTNOTES

[1] https://lore.kernel.org/git/221108.86iljpqdvj.gmgdl@evledraar.gmail.com/
[2]
https://lore.kernel.org/git/pull.1375.git.git.1667934510.gitgitgadget@gmail.com/

Eric Sunshine (3):
  chainlint: sidestep impoverished macOS "terminfo"
  chainlint: latch line numbers at which each token starts and ends
  chainlint: prefix annotated test definition with line numbers

 t/Makefile     |  2 +-
 t/chainlint.pl | 69 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 48 insertions(+), 23 deletions(-)


base-commit: 73c768dae9ea4838736693965b25ba34e941ac88
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1413%2Fsunshineco%2Fchainlintline-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1413/sunshineco/chainlintline-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1413

Comments

Taylor Blau Nov. 9, 2022, 10:22 p.m. UTC | #1
On Wed, Nov 09, 2022 at 04:58:31PM +0000, Eric Sunshine via GitGitGadget wrote:
> This patch series further assists the test author by displaying line numbers
> alongside the annotated test definition, thus allowing the author to jump
> directly to each problematic line.

This is really nifty. I applied it on top of your earlier series,
intentionally broke a test and got some very pleasing chainlint output
after trying to run it.

As previously, I am no expert in the chainlint code, but everything here
looks pretty reasonable to me. And certainly it works, so I'm inclined
to start merging this and the other topic down.

It would be nice to have some more familiar eyes take a look at it,
though.

> (Note to self: Fortify against Ævar's nerd-snipe blacklist evasion.)

;-).

Thanks,
Taylor
Ævar Arnfjörð Bjarmason Nov. 11, 2022, 3:03 p.m. UTC | #2
On Wed, Nov 09 2022, Eric Sunshine via GitGitGadget wrote:

> This is atop "es/chainlint-output"[2].
>
> (Note to self: Fortify against Ævar's nerd-snipe blacklist evasion.)

My only regret is not asking for a pony :)

This looks great, thanks. I read over the v2 (just commenting on the v1
CL for the above comment). I left a note about a potential follow-up
about the color detection, but that's aside from the main change here,
so I think it would be good to just get some version of your v2 as-is,
unless you're super keen to spend more time fiddling with this...