Skip to content
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

Extract JS from HTML files #1352

Closed
wants to merge 1 commit into from

Conversation

BenoitZugmeyer
Copy link
Contributor

This is a feature proposal. I'm well aware that inlining JavaScript into HTML files is a bad practice in so many cases, but there is a few defendable usages.

At my work, we develop widgets mainly self-contained into a single HTML file (that's what we did since day one and it works great). In order to lint the JS contained into those files, I had to fork node-jslint in order to extract the source code from the HTML. As we now want to migrate to Jshint, I did the same, and now proposing my changes to the community, as I am sure that it can be useful to someone else.

As this is my first work on jshint, feel free to comment my code and ask me to reshape it if needed.

Edit: I didn't see the ticket #510 before pull requesting, I'm sorry. But I stay on my position that this feature would be better integrated to jshint cli, in order to be compatible with all IDE / text editor jshint plugins. This is also why I don't want to have to pipe the output of a command to jshint in order to validate HTML files.

@valueof
Copy link
Member

valueof commented Nov 12, 2013

I like the idea. Why can't we just rely on the extension and extract JS only from .html files? Also, you'll need to convert your files to tabs since this is what we use within JSHint codebase.

@BenoitZugmeyer
Copy link
Contributor Author

Awesome! Forgive me if I'm wrong but my changes are already indented with tabs.

At first, I checked the extension to decide if it should extract it. But since jshint can also read the standard input (that don't have any extension), I thought this new approach was more generic. I can't think of any example of (valid) HTML file not starting with a 'less than' character or a JS file starting with this.

@nschonni
Copy link
Contributor

@antonkovalyov GitHub has decided all indentation is 2 spaces (filled a support ticket). The temporary "fix" is to append ?ts=8 to show tabs as 8 chars https://github.com/jshint/jshint/pull/1352/files?ts=8

valueof added a commit that referenced this pull request Nov 22, 2013
valueof added a commit that referenced this pull request Nov 22, 2013
Props to @BenoitZugmeyer for the original patch.

Conflicts:
	src/cli.js
@valueof
Copy link
Member

valueof commented Nov 22, 2013

Thanks, merged a modified version of your patch into both branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants