Closed
Bug 1230776
Opened 9 years ago
Closed 9 years ago
Support ESLint in HTML files
Categories
(Testing :: General, defect)
Testing
General
Tracking
(firefox46 fixed)
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
References
()
Details
Attachments
(1 file)
+++ This bug was initially created as a clone of Bug #1229858 +++
This would be very useful for mochitests.
Assignee | ||
Comment 1•9 years ago
|
||
Bug 1230776: Support scripts from HTML files in eslint. r?Mossop
Attachment #8696279 -
Flags: review?(dtownsend)
Assignee | ||
Comment 2•9 years ago
|
||
Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27303/diff/1-2/
Comment 3•9 years ago
|
||
Note: using this add-on actually causes eslint within Sublime to be broken:
https://github.com/roadhump/SublimeLinter-eslint/issues/87
https://github.com/roadhump/SublimeLinter-eslint/issues/88
https://github.com/roadhump/SublimeLinter-eslint/issues/99 -> https://github.com/SublimeLinter/SublimeLinter3/issues/356
Whilst we might not want to block on these, it'd be good to see if we can push them forward to fix them for our developers.
Assignee | ||
Comment 4•9 years ago
|
||
Hm. Breaking Sublime is probably a non-starter...
Maybe we should enable it with the --plugin command line flag rather than in the global config until that's fixed.
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27303/diff/2-3/
Attachment #8696279 -
Attachment description: MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?Mossop → MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps
Attachment #8696279 -
Flags: review?(dtownsend) → review?(gps)
Assignee | ||
Comment 6•9 years ago
|
||
Switching to r?gps since the mach_commands.py changes are now non-trivial.
Assignee | ||
Comment 7•9 years ago
|
||
There's another drawback to this plugin: It only handles <script> tags with no type attribute, or type="text/javascript", neither of which we tend to use very much.
It might make sense to just create our own plugin, so we can also handle XUL and the other script types that we commonly use, and ideally warn when we see scripts with ;version= tags in their types. In the mean time, I'll send a PR to the plugin's author for better type attribute support.
I think this solution is better than nothing, for the time being, though.
Comment 8•9 years ago
|
||
Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps
https://reviewboard.mozilla.org/r/27303/#review24765
::: python/mach_commands.py
(Diff revision 3)
> -ESLINT_PROMPT = '''
> -Would you like to use eslint
> -'''.strip()
> -
> -ESLINT_PLUGIN_MOZILLA_PROMPT = '''
> -eslint-plugin-mozilla is an eslint plugin containing rules that help enforce
> -JavaScript coding standards in the Mozilla project. Would you like to use this
> -plugin
> -'''.strip()
> -
> -ESLINT_PLUGIN_REACT_PROMPT = '''
> -eslint-plugin-react is an eslint plugin containing rules that help React
> -developers follow strict guidelines. Would you like to install it
> -'''.strip()
> -
If you are going to remove unused variables, please write a commit message saying they are unused so I don't have to spend time trying to figure out why you removed them.
Ideally, this should have been done as a separate commit, as the change is unrelated to the main objective of this patch.
::: python/mach_commands.py:156
(Diff revision 3)
> - @CommandArgument('path', nargs='?', default='.',
> - help='Path to files to lint, like "browser/components/loop" '
> + @CommandArgument('-e', '--ext', default='[.js,.jsm,.jsx,.html]',
> + help='Filename extensions to lint, default: "[.js,.jsm,.jsx,.html]".')
> - 'or "mobile/android". '
> - 'Defaults to the current directory if not given.')
I don't understand why you removed this.
Attachment #8696279 -
Flags: review?(gps)
Assignee | ||
Comment 9•9 years ago
|
||
https://reviewboard.mozilla.org/r/27303/#review24765
> If you are going to remove unused variables, please write a commit message saying they are unused so I don't have to spend time trying to figure out why you removed them.
>
> Ideally, this should have been done as a separate commit, as the change is unrelated to the main objective of this patch.
It seemed related to me. I was adding a new plugin, and was about to add another one of these messages for it, when I checked and realized none of them were actually used.
I still should have mentioned it in the commit message, though. Sorry.
> I don't understand why you removed this.
Mainly because it was preventing me from testing this, since it took the argument to my `--plugin` flag and moved it to end of the arguments list, after my other paths, and it didn't actually serve a purpose anymore. It might have been better as a separate commit, but that commit would have evaporated after I rebased on top of bug 1230300 (which you'd just r+ed but hadn't landed yet), so I decided to just leave it as-is.
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27303/diff/3-4/
Attachment #8696279 -
Flags: review?(gps)
Comment 11•9 years ago
|
||
Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps
https://reviewboard.mozilla.org/r/27303/#review25033
You snuck in some test changes. They look sane enough to me.
Attachment #8696279 -
Flags: review?(gps) → review+
Assignee | ||
Comment 12•9 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #11)
> Comment on attachment 8696279 [details]
> MozReview Request: Bug 1230776: Support scripts from HTML files in eslint.
> r?gps
>
> https://reviewboard.mozilla.org/r/27303/#review25033
>
> You snuck in some test changes. They look sane enough to me.
They were necessary for the tree to pass eslint. Those were the only HTML files I could fix with just trivial changes. I'll move them to a separate commit.
Thanks.
Assignee | ||
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/5803942d32dc6318004c4a9c17b51355aa1c0f57
Bug 1230776: Fix ESLint failures in HTML test files. r=trivial
https://hg.mozilla.org/integration/fx-team/rev/dd26de01e3d255d6f62e679a58a0113ac1128183
Bug 1230776: Support scripts from HTML files in eslint. r=gps
Comment 14•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5803942d32dc
https://hg.mozilla.org/mozilla-central/rev/dd26de01e3d2
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•