Re: [PATCH v2 4/6] tests: Add linter check for pipe placement style

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 18, 2018, 00:34 UTC
Message-ID: <CAPig+cTO=BJ+-6+73fDmdWX7axCZvsyKMxE0E1bhGX-YUFrR-w@mail.gmail.com>
In-Reply-To: <44ee2bd27b510d4216525e24aa1734a1f5edbd95.1537222670.git.matvore@google.com>

On Mon, Sep 17, 2018 at 6:25 PM Matthew DeVore <matvore@google.com> wrote:

> tests: Add linter check for pipe placement style

Until now, the various "lint" checks have been for genuine portability problems (except perhaps 'test-lint-duplicates'). This new lint check makes style violations worthy of failing "make test". Is the indeed the direction we want to go? (Genuine question. I can formulate arguments for either side.)

Show 12 quoted lines

> ---
> diff --git a/t/Makefile b/t/Makefile
> @@ -101,6 +101,16 @@ test-lint-filenames:
> +test-lint-pipes:
> +       @# Do not use \ to join lines when the next line starts with a
> +       @# pipe. Instead, end the prior line with the pipe, and allow that to
> +       @# join the lines implicitly.
> +       @bad="$$(${PERL_PATH} -n0e 'm/(\n[^\n|]+\\\n[\t ]+\|[^\n]*)/ and \
> +             print qq{$$ARGV:$$1\n\n}' $(T))"; \
> +               test -z "$$bad" || { \
> +               printf >&2 "pipe at start of line in file(s):\n%s\n" "$$bad"; \
> +               exit 1; }

If we're going in the direction of linting style violations, then maybe generalize this by calling it "test-lint-style" rather than "test-lint-pipes", and perhaps move the body of the test to a new script check-shell-style.pl (or something), much as portability violations are housed in check-non-portable-shell.pl.