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

Pass JSHint #58

Closed
mar10 opened this issue Feb 16, 2013 · 3 comments
Closed

Pass JSHint #58

mar10 opened this issue Feb 16, 2013 · 3 comments

Comments

@mar10
Copy link

mar10 commented Feb 16, 2013

It would be cool, if blockui would pass the JSHint (with default options):

http://www.jshint.com/

Currently we get a lot of warnigs like this:

Line 38: if (title) $m.append('<h1>'+ title+'</h1>');
    Expected '{' and instead saw '$m'.
Line 521: if (e.keyCode && e.keyCode == 9) {
    Expected '===' and instead saw '=='.
Line 572: setup(jQuery);
    'jQuery' is not defined.
Line 545: function focus(back) {
    'focus' is defined but never used.
@malsup
Copy link
Owner

malsup commented Feb 16, 2013

I use jshint (love it). But I don't feel obligated to use its default settings.

@malsup malsup closed this as completed Feb 16, 2013
@mar10
Copy link
Author

mar10 commented Feb 16, 2013

It wasn't my intention to missionate - moved from jslint to jshint myself, because of the flexibility ;-)

I combined your plugin together with other code into a common file, so I have different jshint option assumptions.

What do you think about adding the relaxing options like

;(function() {
/*jshint eqeqeq:false, curly:false */
"use strict";
    function setup($) {
        $.fn._fadeIn = $.fn.fadeIn;

which remedies most of the warnings in my case.

(unused:false doesn't seem to work if defined inside the local scope, but those two warnings might indicate potential for optimization, though I haven't looked into the code):

Line 27: var mode = document.documentMode || 0;
'mode' is defined but never used.
Line 545: function focus(back) {
'focus' is defined but never used.

@malsup
Copy link
Owner

malsup commented Feb 17, 2013

Ah, yes, that's a good idea to add the relaxation switches to the plugin. Will do.

malsup added a commit that referenced this issue Feb 17, 2013
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

No branches or pull requests

2 participants