archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <>
To: Eric Sunshine via GitGitGadget <>
Cc:, Jeff King <>,
	Junio C Hamano <>,
	Eric Sunshine <>
Subject:'s new "deparse" output (was: [PATCH v2] [...])
Date: Mon, 24 Oct 2022 11:57:36 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

[Please ignore the just-sent empty;
local PBCAK problem :)]

On Tue, Sep 13 2022, Eric Sunshine via GitGitGadget wrote:

> From: Eric Sunshine <>
> When `` detects problems in a test definition, it emits the
> test definition with "?!FOO?!" annotations highlighting the problems it
> discovered. For instance, given this problematic test:
>     test_expect_success 'discombobulate frobnitz' '
>         git frob babble &&
>         (echo balderdash; echo gnabgib) >expect &&
>         for i in three two one
>         do
>             git nitfol $i
>         done >actual
>         test_cmp expect actual
>     '
> will output:
>     # chainlint:
>     # chainlint: discombobulate frobnitz
>     git frob babble &&
>     (echo balderdash ; ?!AMP?! echo gnabgib) >expect &&
>     for i in three two one
>     do
>     git nitfol $i ?!LOOP?!
>     done >actual ?!AMP?!
>     test_cmp expect actual

I've noticed that is better in some ways, but that the
"deparse" output tends to be quite jarring. but I can't find version of
it that emitted this "will output" here.

Before this patch, or fb41727b7ed (t: retire unused chainlint.sed,
2022-09-01) we'd emit this instead:
	git frob babble &&
	( echo balderdash ; ?!AMP?! echo gnabgib ) > expect &&
	for i in three two one
	git nitfol $i ?!LOOP?!
	done > actual ?!AMP?!
	test_cmp expect actual

The difference is in whitespace, e.g. "( ", not "(", "> " not ">".  This
is just because it's emitting "raw" tokenizer tokens.

Was there maybe some local version where the whitespace munging you're
doing against $checked was different & this commit message was left

Anyway, that sort of an aside, but I did go hunting for the version with slightly better whitespace output.

But to get to the actual point: I've found the new output
harder to read sometimes, because it goes through this parse & deparse
state, so you're preserving "\n"''s.

Whereas the old "sed" output also sucked because we couldn't note where
the issue was, but we spewed out the test source verbatim.

But it seem to me that we could get much better output if the
ShellParser/Lexer etc. just kept enough state to emit "startcol",
"endcol" and "linenum" along with every token, or something like that
(you might want offsets from the beginning of the parsed source

Then when it has errors it could emit the actual source passed in, and
even do gcc/clang-style underlining.

I poked at getting that working for a few minutes, but quickly saw that
someone more familiar with the code could do it much quicker, so
consider the above a feature request :)

Another thing: When a test *ends* in a "&&" (common when you copy/paste
e.g. "test_cmp expect actual &&\n" from another test) it doesn't spot
it, but instead we get all the way to the eval/117, i.e. "broken
&&-chain or run-away HERE-DOC".

More feature requests (because for some reason you've got infinite time,
but I don't :): This software is really close to being able to also
change the tests on the fly. If you could define callbacks where you
could change subsets of the parse stream, say a single command like:

	grep some.*rx file

Tokenized as:

	["grep", "some.*rx" "file"]

If you could define an interface to have a callback function e.g. as:

	sub munge_line_tokens {
		my $t = shift;

                return unless $t->[0] eq "grep"; # no changes
                my @t = @$t;

                return [qw(if ! grep), @t[1..$#t],
                	qw(then cat), $t[-1], qw(&& false fi)];

So we could rewrite that into:

        if ! grep some.*rx foo
		cat foo &&

And other interesting auto-fixups and borderline coccinelle
transformations, e.g. changing our various:

	test "$(git ...) = "" &&


	git ... >out &&
	test_must_be_empty out

  parent reply	other threads:[~2022-10-24 10:28 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-12 23:04 [PATCH] chainlint: colorize problem annotations and test delimiters Eric Sunshine via GitGitGadget
2022-09-12 23:55 ` Junio C Hamano
2022-09-13  0:14   ` Eric Sunshine
2022-09-13  0:21     ` Junio C Hamano
2022-09-13  0:39       ` Eric Sunshine
2022-09-13  0:16   ` Jeff King
2022-09-13  0:15 ` Jeff King
2022-09-13  0:30   ` Eric Sunshine
2022-09-13  1:34     ` Jeff King
2022-09-13  0:32   ` Jeff King
2022-09-13  4:01 ` [PATCH v2] " Eric Sunshine via GitGitGadget
2022-09-13 20:40   ` Jeff King
2022-09-13 20:46     ` Junio C Hamano
2022-10-24  9:57   ` Ævar Arnfjörð Bjarmason
2022-10-24  9:57   ` Ævar Arnfjörð Bjarmason [this message]
2022-10-25  4:05     `'s new "deparse" output (was: [PATCH v2] [...]) Eric Sunshine
2022-10-25  4:15       ` Eric Sunshine
2022-10-25 10:07       ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).