Closed Bug 1230776 Opened 9 years ago Closed 9 years ago

Support ESLint in HTML files

Categories

(Testing :: General, defect)

defect
Not set
normal

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.
Bug 1230776: Support scripts from HTML files in eslint. r?Mossop
Attachment #8696279 - Flags: review?(dtownsend)
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/
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.
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.
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)
Switching to r?gps since the mach_commands.py changes are now non-trivial.
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 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)
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.
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 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+
(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.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Depends on: 1270926
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: