Expand whitespace / line-endings linter to JS, CSS, HTML and l10n files
Categories
(Developer Infrastructure :: Lint and Formatting, task)
Tracking
(firefox79 fixed)
Tracking | Status | |
---|---|---|
firefox79 | --- | fixed |
People
(Reporter: Gijs, Assigned: Gijs)
References
Details
Attachments
(4 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
Bug 1644863 - fix trailing whitespace and windows line endings in locale files, r?#fluent-reviewers!
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
We have a linter that complains about trailing whitespace and windows line endings, https://searchfox.org/mozilla-central/source/tools/lint/file-whitespace/__init__.py . It's invoked by this configuration file: https://searchfox.org/mozilla-central/source/tools/lint/file-whitespace.yml and for changes in CI based on https://searchfox.org/mozilla-central/rev/4bb2401ecbfce89af06fb2b4d0ea3557682bd8ff/taskcluster/ci/source-test/mozlint.yml#331-349 . This only lints C++, rust and python files.
It would be nice to do the same for:
*.jsm
*.js
*.css
*.xhtml
*.html
*.ftl
*.properties
*.dtd
Comment 1•4 years ago
|
||
Prettier should already be managing jsm and js files: https://searchfox.org/mozilla-central/rev/4bb2401ecbfce89af06fb2b4d0ea3557682bd8ff/.prettierrc#2
I know it will do some formatting for xhtml/html, though we don't have that enabled at the moment.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
Assignee | ||
Comment 3•4 years ago
|
||
Depends on D79201
Assignee | ||
Comment 4•4 years ago
|
||
Depends on D79202
Assignee | ||
Comment 5•4 years ago
|
||
Depends on D79203
Updated•4 years ago
|
Comment 6•4 years ago
|
||
@Pike
Do you see any problem in having trailing spaces trimmed in localization files? Thinking mostly in terms of cross-channel content when this lands.
Fluent has a specific way to deal with trailing whitespaces, and we enforced all trailing spaces to be converted to \u0020
in .properties, so I don't expect issues there. I've also checked D79203 and it looks good, only trailing spaces and wrong line endings (sigh).
Assignee | ||
Comment 7•4 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #6)
@Pike
Do you see any problem in having trailing spaces trimmed in localization files? Thinking mostly in terms of cross-channel content when this lands.Fluent has a specific way to deal with trailing whitespaces, and we enforced all trailing spaces to be converted to
\u0020
in .properties, so I don't expect issues there. I've also checked D79203 and it looks good, only trailing spaces and wrong line endings (sigh).
I should maybe note explicitly that I made an exception for https://searchfox.org/mozilla-central/rev/4bb2401ecbfce89af06fb2b4d0ea3557682bd8ff/toolkit/locales/en-US/chrome/global/mozilla.dtd#9-12 . I think we don't need the trailing space, because there is a newline and at least in HTML that would just get shown as a single space anyway, but it was like 1am when I was writing the patch and I just didn't want to worry about it - I assume we'll get rid of it when converting that stuff to FTL.
Comment 8•4 years ago
|
||
I need to have a deeper look, unless flod beats me to it.
I'm mostly concerned about the changes to comments leading to duplicated comments in the merged content.
One thing to do would be to pull this revision and run cross-channel against it, and see what actually happens. Happy to look into that when I'm back from PTO.
For now, I left myself as a blocking reviewer on the l10n patch.
Comment 9•4 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #8)
One thing to do would be to pull this revision and run cross-channel against it, and see what actually happens. Happy to look into that when I'm back from PTO.
Sounds good to me, and I have no idea how to test things locally without breaking things :-)
In the meantime, this landed, and we might be lucky
https://hg.mozilla.org/users/axel_mozilla.com/gecko-strings-quarantine/rev/4704a134c94204abe54fb86ff209d33c634ff438
Comments were updated, and no duplication. No clue what's the criteria that causes comments to be duplicated or not, since I've seen both.
Updated•4 years ago
|
Comment 10•4 years ago
|
||
Comment 11•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2a9ac536f873
https://hg.mozilla.org/mozilla-central/rev/1b9de9e2fc19
https://hg.mozilla.org/mozilla-central/rev/2b4dde8c9c1e
https://hg.mozilla.org/mozilla-central/rev/987355bf1f1a
Comment hidden (obsolete) |
Updated•4 years ago
|
Updated•2 years ago
|
Description
•