Disallow non-breaking spaces#55465
Disallow non-breaking spaces#55465samestep wants to merge 3 commits intopytorch:masterfrom samestep:lint-nbsp
Conversation
💊 CI failures summary and remediationsAs of commit 04e81e8 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
|
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Codecov Report
@@ Coverage Diff @@
## master #55465 +/- ##
==========================================
- Coverage 77.43% 77.42% -0.01%
==========================================
Files 1895 1895
Lines 187529 187529
==========================================
- Hits 145212 145200 -12
- Misses 42317 42329 +12 |
janeyx99
left a comment
There was a problem hiding this comment.
This change looks good.
I notice a lot of the lint changes are trailing non-breaking spaces. To my understanding, the trailing whitespace check only checks if there are trailing normal spaces, right? Could that check be expanded to include any type of whitespace?
It could, but then only the trailing U+00A0 characters would be caught, and actually, those are a minority of the ones removed by this PR as far as I can tell. In general I agree that it would be nice to include any type of whitespace in the trailing-spaces check, but I don't know how to easily write a regex that does that. |
Co-authored-by: Jane (Yuan) Xu <31798555+janeyx99@users.noreply.github.com>
|
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: malfet found a couple of these in pytorch#55346; this PR removes the rest and adds a lint that prevents them from being accidentally added again in the future. It also removes the `-o` flag added in pytorch#53733 (which was unnecessarily hiding context without reducing the number of lines of output), and updates the lint error messages to reflect that the individual line numbers are shown in the logs. Pull Request resolved: pytorch#55465 Test Plan: The "Lint / quick-checks" job in GitHub Actions should succeed on this PR. To verify that the lint does correctly find and error on non-breaking spaces, checkout 695699d and run it locally: ```sh (! git --no-pager grep -In $'\u00a0' -- . || (echo "The above lines have non-breaking spaces (U+00A0); please convert them to spaces (U+0020)"; false)) ``` It should print over a hundred lines of output and exit with status 1. Reviewed By: janeyx99 Differential Revision: D27622136 Pulled By: samestep fbshipit-source-id: e7ffd5a9519093e7a0ffdf55e9291f63e21ce841
@malfet found a couple of these in #55346; this PR removes the rest and adds a lint that prevents them from being accidentally added again in the future. It also removes the
-oflag added in #53733 (which was unnecessarily hiding context without reducing the number of lines of output), and updates the lint error messages to reflect that the individual line numbers are shown in the logs.Test plan:
The "Lint / quick-checks" job in GitHub Actions should succeed on this PR. To verify that the lint does correctly find and error on non-breaking spaces, checkout ece0751 and run it locally:
It should print over a hundred lines of output and exit with status 1.