git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Matthew DeVore <matvore@google.com>
Date
Sep 19, 2018, 17:39 UTC
Message-ID
<CAMfpvhJ21ZtpjcJvOH7s=5r496z4Fz+tw94ymGD54xe=0RJ8Eg@mail.gmail.com>
In-Reply-To
<CAPig+cTO=BJ+-6+73fDmdWX7axCZvsyKMxE0E1bhGX-YUFrR-w@mail.gmail.com>
On Mon, Sep 17, 2018 at 5:34 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 28 quoted lines
>
> 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.)
>
> > ---
> > 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.

I agree with moving this code to a separate file. I also notice that there is logic in a file called t/chainlint.sed which normalizes scripts for the purpose of lint checks (e.g. it removes heredocs) and I ought to be re-using this logic in order to make the new lint check robust.

I think I'd like to shelve this particular commit and come back to it once I have the time and I'm more familiar with the code base. It is not critical to this patchset.

Previous: Eric SunshineNext: Matthew DeVore
Message 11 of 66 in “Cleanup tests for test_cmp argument ordering and "|" placement”
  1. 0/2 Cleanup tests for test_cmp argument ordering and "|" placementMatthew DeVore, Sep 15, 2018
  2. 1/2 t/*: fix pipe placement and remove \'sMatthew DeVore, Sep 15, 2018
  3. Jonathan NiederSep 17, 2018
  4. Matthew DeVoreSep 17, 2018
  5. 2/2 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Sep 15, 2018
  6. Matthew DeVoreSep 17, 2018
  7. Junio C HamanoSep 15, 2018
  8. 0/6 Clean up tests for test_cmp arg ordering and pipe placementMatthew DeVore, Sep 17, 2018
  9. 4/6 tests: Add linter check for pipe placement styleMatthew DeVore, Sep 17, 2018
  10. Eric SunshineSep 18, 2018
  11. Matthew DeVoreSep 19, 2018
  12. 0/5 Clean up tests for test_cmp arg ordering and pipe placementMatthew DeVore, Sep 21, 2018
  13. 1/5 CodingGuidelines: add shell piping guidelinesMatthew DeVore, Sep 21, 2018
  14. Eric SunshineSep 21, 2018
  15. Matthew DeVoreSep 21, 2018
  16. SZEDER GáborSep 24, 2018
  17. Matthew DeVoreSep 25, 2018
  18. SZEDER GáborSep 27, 2018
  19. Matthew DeVoreOct 1, 2018
  20. 2/5 tests: standardize pipe placementMatthew DeVore, Sep 21, 2018
  21. 3/5 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Sep 21, 2018
  22. 4/5 tests: don't swallow Git errors upstream of pipesMatthew DeVore, Sep 21, 2018
  23. 5/5 t9109: don't swallow Git errors upstream of pipesMatthew DeVore, Sep 21, 2018
  24. 0/7 Clean up tests for test_cmp arg ordering and pipe placementMatthew DeVore, Oct 3, 2018
  25. 1/7 t/README: reformat Do, Don't, Keep in mind listsMatthew DeVore, Oct 3, 2018
  26. Junio C HamanoOct 5, 2018
  27. Matthew DeVoreOct 5, 2018
  28. 2/7 Documentation: add shell guidelinesMatthew DeVore, Oct 3, 2018
  29. Junio C HamanoOct 5, 2018
  30. Matthew DeVoreOct 5, 2018
  31. 3/7 tests: standardize pipe placementMatthew DeVore, Oct 3, 2018
  32. 4/7 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Oct 3, 2018
  33. 5/7 tests: don't swallow Git errors upstream of pipesMatthew DeVore, Oct 3, 2018
  34. Junio C HamanoOct 5, 2018
  35. Matthew DeVoreOct 5, 2018
  36. Matthew DeVoreOct 5, 2018
  37. 6/7 t9109: don't swallow Git errors upstream of pipesMatthew DeVore, Oct 3, 2018
  38. 7/7 tests: order arguments to git-rev-list properlyMatthew DeVore, Oct 3, 2018
  39. Matthew DeVoreOct 3, 2018
  40. Junio C HamanoOct 5, 2018
  41. 0/7 subject: Clean up tests for test_cmp arg ordering and pipe placementMatthew DeVore, Oct 5, 2018
  42. 1/7 t/README: reformat Do, Don't, Keep in mind listsMatthew DeVore, Oct 5, 2018
  43. 2/7 Documentation: add shell guidelinesMatthew DeVore, Oct 5, 2018
  44. 3/7 tests: standardize pipe placementMatthew DeVore, Oct 5, 2018
  45. 4/7 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Oct 5, 2018
  46. 5/7 tests: don't swallow Git errors upstream of pipesMatthew DeVore, Oct 5, 2018
  47. 6/7 t9109: don't swallow Git errors upstream of pipesMatthew DeVore, Oct 5, 2018
  48. 7/7 tests: order arguments to git-rev-list properlyMatthew DeVore, Oct 5, 2018
  49. Junio C HamanoOct 6, 2018
  50. 1/6 CodingGuidelines: add shell piping guidelinesMatthew DeVore, Sep 17, 2018
  51. Eric SunshineSep 18, 2018
  52. Matthew DeVoreSep 19, 2018
  53. Eric SunshineSep 19, 2018
  54. Junio C HamanoSep 19, 2018
  55. Matthew DeVoreSep 19, 2018
  56. 2/6 tests: standardize pipe placementMatthew DeVore, Sep 17, 2018
  57. 3/6 t/*: fix ordering of expected/observed argumentsMatthew DeVore, Sep 17, 2018
  58. 4/6 tests: add linter check for pipe placement styleMatthew DeVore, Sep 17, 2018
  59. 5/6 tests: split up pipesMatthew DeVore, Sep 17, 2018
  60. Eric SunshineSep 18, 2018
  61. Matthew DeVoreSep 19, 2018
  62. 6/6 t9109-git-svn-props.sh: split up several pipesMatthew DeVore, Sep 17, 2018
  63. Eric SunshineSep 18, 2018
  64. Matthew DeVoreSep 19, 2018
  65. Eric SunshineSep 19, 2018
  66. Matthew DeVoreSep 19, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.