New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a flycheck-jshint-extract option. #825

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@tealeg

tealeg commented Dec 20, 2015

This option sets the value passed to --extract on execution of jshint,
allowing it to extract javascript embedded in an HTML file and therefore
avoid spurious linting errors on surrounding HTML elments.

Geoffrey J. Teale
Add a flycheck-jshint-extract option.
This option sets the value passed to --extract on execution of jshint,
allowing it to extract javascript embedded in an HTML file and therefore
avoid spurious linting errors on surrounding HTML elments.
@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Dec 20, 2015

Thank you for this patch. But how would you use this option? Flycheck does not use jshint in HTML files by default.

@tealeg

This comment has been minimized.

tealeg commented Dec 21, 2015

@lunaryorn I use this via multi-web-mode - but essentially any of the modes that allow multiple major modes when editing an HTML file with embedded javascript would do this. Using mulit-web-mode, when I move the point into an area of the file bounded by <script> and </script> then js2-mode is invoked exactly as if it were a .js file. The jshint output will then warn about the html tags (which are obviously invalid JS). Setting this option to either auto or always cures that problem.

Refer to the jshint manual at the URL
`http://jshint.com/docs/cli/#flags'
for more information."
:type '(choice (const :tag "No extraction rule" nil)

This comment has been minimized.

@lunaryorn

lunaryorn Dec 25, 2015

Contributor

@tealeg What would happen in this case, i.e. if no rule was specified?

This comment has been minimized.

@lunaryorn

lunaryorn Jan 2, 2016

Contributor

@tealeg Sorry, did you miss my comment? I would like to understand how this option works before I merge this pull request ☺️

This comment has been minimized.

@tealeg

tealeg Jan 3, 2016

@lunaryorn sorry I didn't see it. In this case it's the same as passing "never" which is the default.

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Dec 25, 2015

@tealeg Ah, I see. I didn't know that such a thing existed for Emacs ☺️

I'll merge this PR, but I'm unsure about the choices offered by the option. Would you mind to answer on my comment I left in the diff?

@tealeg

This comment has been minimized.

tealeg commented Jan 3, 2016

@lunaryorn I've answered the comment now, sorry for the delay!

@lunaryorn lunaryorn added the S-ready label Jan 7, 2016

@lunaryorn lunaryorn self-assigned this Jan 7, 2016

@lunaryorn lunaryorn force-pushed the flycheck:master branch 3 times, most recently from 94235da to 7d0129a Jan 23, 2016

@lunaryorn lunaryorn closed this in 58d6d78 Feb 7, 2016

@lunaryorn lunaryorn removed the S-ready label Feb 7, 2016

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Feb 7, 2016

@tealeg Merged, thank you for this contribution.

@cwhatley

This comment has been minimized.

cwhatley commented Feb 8, 2016

FYI, this broke for me when I updated. See: #871

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Feb 8, 2016

@cwhatley Sorry, my bad 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment