make code grunt-contrib-jshint 0.3.0 compatible

[mgol]

No description provided.

[mgol]

Note that I had to switch to false both the noempty option because of two while-s with empty blocks and the strict option because turning strict mode on has been reverted recently.

[rwaldron]

cc @jaubourg

[gibson042]

This one will work because it changes the numbers to strings, but will probably increase file size.

[mgol]

It probably will but so does using triple = instead of double in most places and many such cases are not, strictly speaking, required for the code to work. And since this is one of very few places were !=/== exist and current .jshintrc configuration forbids it...

If you decide to not change it then it should at least be surrounded by jshint comment blocks disabling the eqeqeq option.

[gibson042]

I personally find the code taking advantage of function hoisting to be more readable and better organized. It might be time to try undef:true and latedef:false, per jshint/jshint#424 (comment), which would eliminate the need for these large block moves.

[mgol]

Ok, I'll check how it works tomorrow.

[mgol]

I updated the pull request according to review comments; most changes were, indeed, not needed. It seems everything works fine after an update to grunt-contrib-jshint 0.2.0 now so I updated it in package.json, too.

@rwldrn & @gibson042: as for the !== window comparisons - all unit tests after my modifications pass on both IE9 & IE10. However, I was thinking: these != comparisons are still needed for 1.x and it will cause jsHint breakage so /*jshint comments will be needed there anyway. We can either keep 1.x & 2.x different here or change comparisons back to != and wrap them in those jsHint comments disabling eqeqeq for a moment.

[gibson042]

It is appropriate to keep the strict this/window comparisons in 2.0. Could you open an analogous PR for 1.9-stable that instead uses // Support: IE<9 and /*jshint eqeqeq:false */?

[mgol]

Sure, I'll do it within a few hours.

BTW, jsHint >= 1.0 used by this upgraded grunt-contrib-jshint allows using space between /* and jshint or global so maybe it would be better to style it like that?

/* jshint eqeqeq: false */

Seems more in line with jQuery liberal spacing policy.

[gibson042]

Yeah, I like it.

[mgol]

I cleaned up jsHint conf options: regexdash doesn't seem to take any effect (and it's not in docs), I also removed wsh (why was it needed? nothing in the code requires it), I added eqeqeq (it's not enforced by default and the code base adheres to it anyway) and es5 (fine for any browser supported by jQuery 2.0).

[mgol]

Please note that for this code to build you must still use node 0.8.x, not 0.10.0 as long as pull request #1202 is not pulled in.

[mgol]

Ah, I also removed the evil option - it seems dangerous and since it's not needed at all (it is still needed in 1.9-stable, though)...

[mgol]

I created an analogous pull request for 1.9-stable: #1204

[mikesherov]

We can always turn on eqeqeq and inline comment turn it off for the isWindow check in 1.9, although @timmywil was at one point against inline jshint commenting.

[mgol]

That's exactly what I did in #1204. Of course, another option is to turn off eqeqeq in 1.9-stable but since it's needed only in 3 places and it's a good practice to not use == if not needed...

[mgol]

Rebased to master; thanks to that it now correctly builds on node 0.10.0 so it's easier to test it.

Also, restored the regexdash option as it's still needed, though not documented on jshint.com/docs for some reason.

[mgol]

BTW, if/when you merge this one I'll rebase all my other pull requests so that they can be checked against the newest jsHint version.

[mgol]

Re-based to master and corrected some recently added leading white spaces which made grunt fail after re-basing.

Any chance of merging it soon so that new such mistakes aren't introduced?

[mgol]

I made analogous updates to pull request #1204

[mgol]

Re-based to master. Most of changes from this pull request has been already merged in commit 1205103 but a few remain.

[dmethvin]

Somehow I missed this and cherry-picked gh-1204 to get the 1.x changes. Do these look good? 1205103 If so just close this one.

[mgol]

@dmethvin, I've just rebased this one to master so that only the remaining parts are to be merged. There are only a few differences related mostly to comparisons with window which required == instead === in oldIE but are fine here. It was already discussed and both @rwldrn and @gibson042 agreed it's fine.

[mgol]

I've just run the full unit test suite from this branch on IE9 and are all passed.

[mgol]

Well, the commit description on the rebased one is now a little outdated but you can change it when merging anyway.