Hi Antranig, Thanks for your response.
As for the globals comment, I just cleaned them all up, so hopefully you wont see this in the current code. However, here's an example from what I changed https://github.com/fluid-project/infusion/commit/d800c49b532c6535b323e0df9d04540a4b390b76 Look at "portalRootId" in src/webapp/tests/component-tests/reorderer/js/ModuleLayoutTestConstants.js I think this one is actually an error. Also there were a bunch of cases where globals were omitted and others that were added when they shouldn't be (likely cut and paste errors as the variable didn't occur in the code at all). Thanks Justin On 2011-03-08, at 12:45 PM, Antranig Basman wrote: > On 08/03/2011 10:22, Justin Obara wrote: >> Hey Antranig, > Hi Justin - yes, I agree about the "console" issue and will update the > implementation. In most cases I have used the //jslint:ok option I have > appended an additional comment with a pointer to why the construction is ok. > You can add any further material on the line following the comment that you > like. In fact, in "fully mature" implementations of lints in the C world, the > "nolint" directive has to contain some specification of the SPECIFIC kind of > warning that the directive suppresses. At the moment the only useful > "categorisation" of warning categories in the JSLint implementation is > derived from the message keys that they emit, so this wouldn't be very > semantic without a lot of work grouping messages into functional categories, > probably without a lot of gain - this "exception-barring" should be something > we do extremely sparingly in any case. More simply, we can add an option if > we want, to enable or disable processing of this comment on a per-file basis. > What sorts of elements have you found inserted "hostilely" into the globals > comment? > >> Thanks for continuing to work on this. It's a shame that our fixes weren't >> accepted upstream. >> >> Here some of my comments/concerns: >> >> I think we shouldn't select "Assume console, alert, ..." as not all browsers >> support "console" >> I'm not sure i agree with your new white space options. I think they're >> probably purely stylistic, but I think it is better to be consistent one way >> or the other. >> the //jslint:ok comment seems useful, but dangerous. It is a little easier >> to verify the globals comment, but I've found this also to have been >> misused. Is someone able to add an explanation after the comment, explaining >> why it is an exception? >> >> Thanks >> Justin >> >> On 2011-03-08, at 12:04 PM, Antranig Basman wrote: >> >>> I have placed a revised version of the implementation of the JSLint tool >>> that we have inherited from Douglas Crockford up at >>> http://ponder.org.uk/fluid/fulljslint.html - the source code is also >>> available for inspection in my github area. >>> Since we have established that no economies of development are possible by >>> sharing even the most conservative fixes upstream, I've taken the >>> opportunity to overhaul the implementation thoroughly, and have fixed >>> several bugs relating to outright misparses as well as misinterpretation of >>> wrapping constructs, as will as implementing some new options and >>> configuration to make it easier for our project to use. I am surfacing a >>> list of what some of these are (in cases where there might be different >>> opinions) so that we can decide now we have a free hand what kinds of code >>> we want to tolerate. Please also speak up if you have any suggestions for >>> completely new features of JSLint that are not in this set, since our >>> implementation is now our own and it has been found fairly straightforward >>> to implement new features. >>> >>> Firstly, the reason for the original fork in the first place, an option to >>> tolerate the for (var x in ...) construction which recent versions of >>> JSLint threw out as an unconditional parse error, aborting further >>> processing. (forvar) >>> >>> Secondly, an option to tolerate two variants for block indentation of >>> "run-on" control structures, being if...else and try..catch..finally - >>> original JSLint would ONLY tolerate the following variant >>> if (cond) { >>> material 1; >>> } else if { >>> material 2; >>> } >>> whereas we may now tolerate BOTH the above variant and ALSO the following >>> (I believe more commonly seen) form (elsecatch) >>> if (cond) { >>> material 1; >>> } >>> else if { >>> material 2; >>> } >>> >>> Thirdly, an option to tolerate zero or one spaces in a few cases around >>> operators - the vast majority remain at the original defaults of requiring >>> exactly 1 space for binary operators (&&, ===, etc.) and zero spaces for >>> unary operators (++, --) but at least in my opinion, our rules for spaces >>> following the "function" keyword have been annoyingly inconsistent, with >>> exactly zero spaces tolerated in one case and exactly one space tolerated >>> in another. With the option (operator), either zero or one space are >>> tolerated - for example, both >>> var x = function (x) {.... >>> and >>> var x = function(x) {... >>> are acceptable. >>> >>> The indentation rules in general are unchanged in this implementation, >>> apart from a few bug fixes in cases of multiple constructs per line which >>> would sometimes lead to original JSLint to recommend NO indentation on the >>> next, multiply nested line which was clearly a bug (constructions like >>> var y = fluid.transform(list, function(value, key) { >>> ... would often require faulty indenting HERE. ) >>> >>> Fourthly, there is a new "emergency escape" rule in the form of a specially >>> formatted comment accepted on a line (this is a common option with the >>> implementations of linting tools in the C world from which JSLint takes >>> heritage) - of the form // jslint:ok - this allows a one-off override of >>> the linting rules for a particular line of code that has been determined >>> through specific inspection to be safe. Clearly this rule must be used very >>> sparingly since most linting violations (especially in the new >>> implementation) are genuine. It was used a few times in the renderer code, >>> particularly to allow exceptions for the "var x is already defined" warning >>> caused by JavaScript's anomalous scoping rules, and for the "do not declare >>> functions in a loop" rule which is a blanket recommendation which JSLint >>> makes without any inspection of the flow involved. In particular, functions >>> which do not bind to the loop counter variable for the loop they are nested >>> in should be excepted from thi > s rule. Lots of the renderer code is quite intricate and allowing a handful > of violations (in my opinion) allowed the code to remain more readable by not > having to arbitrarily rename some variables which clearly served the same > function, or reduce locality by breaking small anonymous functions out of > loops. >>> >>> Please chime in with your opinions on what you think we should do about >>> these various options and constructions :) >>> >>> Antranig. >>> _______________________________________________________ >>> fluid-work mailing list - [email protected] >>> To unsubscribe, change settings or access archives, >>> see http://fluidproject.org/mailman/listinfo/fluid-work >> >> > _______________________________________________________ fluid-work mailing list - [email protected] To unsubscribe, change settings or access archives, see http://fluidproject.org/mailman/listinfo/fluid-work
