Page MenuHomePhabricator

[LINTER] Accept hard line breaks in markdown files
ClosedPublic

Authored by Fabien on Sep 18 2020, 13:13.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Commits
rABC88ac041b3196: [LINTER] Accept hard line breaks in markdown files
Summary

D7483 showed that the trailing whitespaces can actually be a feature from the markdown syntax, so they need to be excluded from the linter.
Exactly 2 whitespace is a [[ https://github.github.com/gfm/#hard-line-breaks | hard break ]]with the github flavored markdown (and maybe others).

Test Plan

No issue:

arc lint --everything

Add trailing a single or 3+ spaces to some markdown files, then:

arc lint

Ensure there is no error reported.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
linter_trailing_whitespaces_md
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 12802
Build 25677: Build Diffbuild-clang-10 · build-without-wallet · build-diff · build-clang-tidy
Build 25676: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Sep 18 2020, 13:13
jasonbcox requested changes to this revision.Sep 18 2020, 15:52
jasonbcox added a subscriber: jasonbcox.

I don't think this makes sense. Even if markdown supports trailing whitespace as a line break, it doesn't mean it's a good idea. Consider:

  • Not linting this will now allow accidental trailing whitespaces.
  • Trailing whitespace does not render well in all IDEs/editors.
  • Intentional line breaks should be obvious, which trailing whitespace are not.
This revision now requires changes to proceed.Sep 18 2020, 15:52
Fabien retitled this revision from [LINTER] Exclude markdown files from the trailing whitespaces linter to [LINTER] Accept hard line breaks in markdown files.Sep 18 2020, 17:16
Fabien edited the summary of this revision. (Show Details)
Fabien edited the test plan for this revision. (Show Details)
Fabien edited the summary of this revision. (Show Details)

Don't exclude all markdown, but adds an exception to the linter for the hard break special case

This revision is now accepted and ready to land.Sep 18 2020, 18:21