Permalink
Browse files

JSHint fixes

  • Loading branch information...
arian committed Feb 23, 2011
1 parent 9605229 commit 687d3fcd185363b817571564d8e7798dd650da81
@@ -61,7 +61,7 @@ Events.Pseudos = function(pseudos, addEvent, removeEvent){
return Object.merge.apply(this, split.map(function(item){
return pseudos[item.pseudo].options || {};
}));
- }
+ };
return {
View
@@ -98,7 +98,7 @@ var Slider = new Class({
this.drag = new Drag(knob, dragOptions);
this.attach();
- if (options.initialStep != null) this.set(options.initialStep)
+ if (options.initialStep != null) this.set(options.initialStep);
},
attach: function(){
@@ -75,7 +75,7 @@ var formObserver = function(eventName){
formListeners.push(formListener);
form.store($delegationKey + eventName + 'originalFn', formEvents)
- .store($delegationKey + eventName + 'listeners', formListeners)
+ .store($delegationKey + eventName + 'listeners', formListeners);
}
}
};
@@ -93,7 +93,7 @@ var inputObserver = function(eventName){
};
args[0].target.addEvents(events);
}
- }
+ };
};
var eventOptions = {
@@ -36,7 +36,7 @@ provides: [Element.Pin]
supportsPositionFixed = (test.offsetTop === 0);
test.dispose();
supportTested = true;
- }
+ };
Element.implement({
@@ -45,7 +45,9 @@ provides: [Element.Pin]
if (this.getStyle('display') == 'none') return this;
var pinnedPosition,
- scroll = window.getScroll();
+ scroll = window.getScroll(),
+ parent,
+ scrollFixer;
if (enable !== false){
pinnedPosition = this.getPosition(supportsPositionFixed ? document.body : this.getOffsetParent());
@@ -59,8 +61,8 @@ provides: [Element.Pin]
this.setStyle('position', 'fixed').setStyles(currentPosition);
} else {
- var parent = this.getOffsetParent(),
- position = this.getPosition(parent),
+ parent = this.getOffsetParent();
+ var position = this.getPosition(parent),
styles = this.getStyles('left', 'top');
if (parent && styles.left == 'auto' || styles.top == 'auto') this.setPosition(position);
@@ -71,7 +73,7 @@ provides: [Element.Pin]
y: styles.top.toInt() - scroll.y
};
- var scrollFixer = function(){
+ scrollFixer = function(){
if (!this.retrieve('pin:_pinned')) return;
var scroll = window.getScroll();
this.setStyles({
@@ -89,13 +91,13 @@ provides: [Element.Pin]
} else {
if (!this.retrieve('pin:_pinned')) return this;
- var parent = this.getParent(),
- offsetParent = (parent.getComputedStyle('position') != 'static' ? parent : parent.getOffsetParent());
+ parent = this.getParent();
+ var offsetParent = (parent.getComputedStyle('position') != 'static' ? parent : parent.getOffsetParent());
pinnedPosition = this.getPosition(offsetParent);
this.store('pin:_pinned', false);
- var scrollFixer = this.retrieve('pin:_scrollFixer');
+ scrollFixer = this.retrieve('pin:_scrollFixer');
if (!scrollFixer){
this.setStyles({
position: 'absolute',
@@ -83,10 +83,15 @@ Form.Validator.Inline = new Class({
showAdvice: function(className, field){
var advice = this.getAdvice(className, field);
- if (advice && !field.retrieve('$moo:' + this.getPropName(className))
- && (advice.getStyle('display') == 'none'
- || advice.getStyle('visiblity') == 'hidden'
- || advice.getStyle('opacity') == 0)){
+ if (
+ advice &&
+ !field.retrieve('$moo:' + this.getPropName(className)) &&
+ (
+ advice.getStyle('display') == 'none' ||
+ advice.getStyle('visiblity') == 'hidden' ||
+ advice.getStyle('opacity') == 0
+ )
+ ){

This comment has been minimized.

Show comment Hide comment
@anutron

anutron Feb 24, 2011

Owner

um, yuck?

@anutron

anutron Feb 24, 2011

Owner

um, yuck?

This comment has been minimized.

Show comment Hide comment
@fat

fat Feb 24, 2011

Contributor

yeah i would agree -- not really into this chunk.

@fat

fat Feb 24, 2011

Contributor

yeah i would agree -- not really into this chunk.

This comment has been minimized.

Show comment Hide comment
@arian

arian Feb 24, 2011

Owner

imo this is more readable since the && are now on the same indention level, and the || are clearly grouped. jshint complained about && or || at the beginning of the lines.

@arian

arian Feb 24, 2011

Owner

imo this is more readable since the && are now on the same indention level, and the || are clearly grouped. jshint complained about && or || at the beginning of the lines.

This comment has been minimized.

Show comment Hide comment
@subtleGradient

subtleGradient Feb 24, 2011

Owner

Then file a bug with jshint. && and similar operators are FAR more readable at the beginning of lines.

@subtleGradient

subtleGradient Feb 24, 2011

Owner

Then file a bug with jshint. && and similar operators are FAR more readable at the beginning of lines.

This comment has been minimized.

Show comment Hide comment
@cpojer

cpojer Feb 24, 2011

Owner

I agree its better at the beginning but I also don't care much in this specific case. Indentation for readability ftw.

@cpojer

cpojer Feb 24, 2011

Owner

I agree its better at the beginning but I also don't care much in this specific case. Indentation for readability ftw.

This comment has been minimized.

Show comment Hide comment
@seanmonstar

seanmonstar Feb 24, 2011

&& and || at the beginning allows for easily commenting out a condition.

@seanmonstar

seanmonstar Feb 24, 2011

&& and || at the beginning allows for easily commenting out a condition.

field.store('$moo:' + this.getPropName(className), true);
this.options.showError(advice);
this.fireEvent('showAdvice', [field, advice, className]);
View
@@ -55,7 +55,7 @@ var OverText = new Class({
property: 'OverText',
initialize: function(element, options){
- var element = this.element = document.id(element);
+ element = this.element = document.id(element);
if (this.occlude()) return this.occluded;
this.setOptions(options);
@@ -64,7 +64,7 @@ Fx.Accordion = new Class({
if (options.show || this.options.show === 0){
options.display = false;
- previous = this.options.show;
+ this.previous = options.show;

This comment has been minimized.

Show comment Hide comment
@anutron

anutron Feb 24, 2011

Owner

how is this a lint change? This doesn't look right to me.

@anutron

anutron Feb 24, 2011

Owner

how is this a lint change? This doesn't look right to me.

This comment has been minimized.

Show comment Hide comment
@fat

fat Feb 24, 2011

Contributor

hm... it looks right to me actually... not sure how it was working before though.

@fat

fat Feb 24, 2011

Contributor

hm... it looks right to me actually... not sure how it was working before though.

This comment has been minimized.

Show comment Hide comment
@arian

arian Feb 24, 2011

Owner

Yeah, I broke it before :( thanks to jshint I found it.

@arian

arian Feb 24, 2011

Owner

Yeah, I broke it before :( thanks to jshint I found it.

}
if (options.start){
@@ -189,15 +189,15 @@ Fx.Accordion = new Class({
}
this.fireEvent(hide ? 'background' : 'active', [this.togglers[i], el]);
for (var fx in effects) obj[i][fx] = hide ? 0 : el[effects[fx]];
- if (!useFx && !hide && options.resetHeight) obj[i]['height'] = 'auto';
+ if (!useFx && !hide && options.resetHeight) obj[i].height = 'auto';
}, this);
this.internalChain.clearChain();
this.internalChain.chain(function(){
if (options.resetHeight && !this.selfHidden){
var el = elements[index];
if (el) el.setStyle('height', 'auto');
- };
+ }
}.bind(this));
return useFx ? this.start(obj) : this.set(obj);
View
@@ -174,6 +174,6 @@ Fx.Scroll.implement({
function isBody(element){
return (/^(?:body|html)$/i).test(element.tagName);
-};
+}
}).call(this);
View
@@ -88,8 +88,9 @@ Fx.Sort = new Class({
if (newOrder.length > this.elements.length)
newOrder.splice(this.elements.length-1, newOrder.length - this.elements.length);
}
- var margin = top = left = 0;
- newOrder.each(function(item, index){
+ var margin = 0;
+ top = left = 0;

This comment has been minimized.

Show comment Hide comment
@anutron

anutron Feb 24, 2011

Owner

doesn't this make top and left global variables?

@anutron

anutron Feb 24, 2011

Owner

doesn't this make top and left global variables?

This comment has been minimized.

Show comment Hide comment
@fat

fat Feb 24, 2011

Contributor

nope, they're defined above on line ~51

@fat

fat Feb 24, 2011

Contributor

nope, they're defined above on line ~51

+ newOrder.each(function(item){
var newPos = {};
if (vert){
newPos.top = top - current[item].top - margin;
@@ -177,7 +177,8 @@ HtmlTable = Class.refactor(HtmlTable, {
};
row.store('binders', binders);
}
- (this._selectEnabled) ? row.addEvents(binders) : row.removeEvents(binders);
+ if (this._selectEnabled) row.addEvents(binders);
+ else row.removeEvents(binders);
}, this);
},
@@ -164,7 +164,7 @@ HtmlTable = Class.refactor(HtmlTable, {
for (rowIndex = 0; rowIndex < count; rowIndex++) {
if (data[rowIndex].position > position) data[rowIndex].position--;
}
- };
+ }
},
setRowStyle: function(row, i){
@@ -195,8 +195,9 @@ HtmlTable = Class.refactor(HtmlTable, {
var parser = this.getParser();
if (!parser) return;
+ var rel;
if (!Browser.ie){
- var rel = this.body.getParent();
+ rel = this.body.getParent();
this.body.dispose();
}
@@ -327,7 +328,7 @@ HtmlTable.Parsers = new Hash(HtmlTable.Parsers);
HtmlTable.defineParsers = function(parsers){
HtmlTable.Parsers = Object.append(HtmlTable.Parsers, parsers);
- for (parser in parsers){
+ for (var parser in parsers){
HtmlTable.ParserPriority.unshift(parser);
}
};
@@ -37,7 +37,7 @@ Locale.Set.from = function(set, type){
if (!type && typeOf(set) == 'string') type = 'json';
if (parsers[type]) set = parsers[type](set);
- locale = new Locale.Set;
+ var locale = new Locale.Set;
locale.sets = set.sets || {};
@@ -47,6 +47,6 @@ Locale.Set.from = function(set, type){
}
return locale;
-}
+};
}).call(this);
@@ -44,8 +44,9 @@ Request.Queue = new Class({
},
initialize: function(options){
+ var requests;
if (options){
- var requests = options.requests;
+ requests = options.requests;
delete options.requests;
}
this.setOptions(options);
@@ -21,6 +21,9 @@ provides: [Array.Extras]
...
*/
+
+(function(nil){
+

This comment has been minimized.

Show comment Hide comment
@anutron

anutron Feb 24, 2011

Owner

this is a lint fix?

@anutron

anutron Feb 24, 2011

Owner

this is a lint fix?

This comment has been minimized.

Show comment Hide comment
@fat

fat Feb 24, 2011

Contributor

yeah -- it's an optimization for compressing code

@fat

fat Feb 24, 2011

Contributor

yeah -- it's an optimization for compressing code

This comment has been minimized.

Show comment Hide comment
@arian

arian Feb 24, 2011

Owner

var undefined; wasn't allowed, so now I created a undefined variable this way.

@arian

arian Feb 24, 2011

Owner

var undefined; wasn't allowed, so now I created a undefined variable this way.

This comment has been minimized.

Show comment Hide comment
@subtleGradient

subtleGradient Feb 24, 2011

Owner

File another bug with jshint. undefined > nil

@subtleGradient

subtleGradient Feb 24, 2011

Owner

File another bug with jshint. undefined > nil

This comment has been minimized.

Show comment Hide comment
@cpojer

cpojer Feb 24, 2011

Owner

In future versions of ES it might as well be that undefined is not a valid variable name any more. The global undefined is non-writable in ES5. If, in Firefox4, you try to change the value of the global undefined variable, it won't have any effect.

I think its better just to avoid using undefined as a variable name for these reasons. One could argue that it would be better to just use the global undefined in Array.Extras.js, however in the majority of (modern) browsers the global undefined is still writable. So I think using 'nil' as a name (since null and undefined are not available) is perfectly fine and everything is totally cool.

@cpojer

cpojer Feb 24, 2011

Owner

In future versions of ES it might as well be that undefined is not a valid variable name any more. The global undefined is non-writable in ES5. If, in Firefox4, you try to change the value of the global undefined variable, it won't have any effect.

I think its better just to avoid using undefined as a variable name for these reasons. One could argue that it would be better to just use the global undefined in Array.Extras.js, however in the majority of (modern) browsers the global undefined is still writable. So I think using 'nil' as a name (since null and undefined are not available) is perfectly fine and everything is totally cool.

Array.implement({
min: function(){
@@ -57,19 +60,20 @@ Array.implement({
},
reduce: function(fn, value){
- var undefined;
for (var i = 0, l = this.length; i < l; i++){
- if (i in this) value = value === undefined ? this[i] : fn.call(null, value, this[i], i, this);
+ if (i in this) value = value === nil ? this[i] : fn.call(null, value, this[i], i, this);
}
return value;
},
reduceRight: function(fn, value){
- var i = this.length, undefined;
+ var i = this.length;
while (i--){
- if (i in this) value = value === undefined ? this[i] : fn.call(null, value, this[i], i, this);
+ if (i in this) value = value === nil ? this[i] : fn.call(null, value, this[i], i, this);
}
return value;
}
});
+
+}).call(this);

This comment has been minimized.

Show comment Hide comment
@fat

fat Feb 23, 2011

Contributor

why call here?

@fat

fat Feb 23, 2011

Contributor

why call here?

This comment has been minimized.

Show comment Hide comment

This comment has been minimized.

Show comment Hide comment
@fat

fat Feb 24, 2011

Contributor

oh cool, thanks ;)

@fat

fat Feb 24, 2011

Contributor

oh cool, thanks ;)

This comment has been minimized.

Show comment Hide comment
@seanmonstar

seanmonstar Feb 24, 2011

But you never access this inside the function...

@seanmonstar

seanmonstar Feb 24, 2011

But you never access this inside the function...

@@ -95,7 +95,7 @@ Date.implement({
switch(bits[0]){
case 'tom': return d.increment();
case 'yes': return d.decrement();
- default: return d;
+ default: return d;
}
}
},
View
@@ -322,6 +322,9 @@ var parseWord = function(type, word, num){
return (num) ? translated.indexOf(ret) : ret;
};
+var startCentury = 1900,
+ startYear = 70;
+
Date.extend({
getMsg: function(key, args){
@@ -431,9 +434,6 @@ Date.extend({
});
-var startCentury = 1900,
- startYear = 70;
-
var regexOf = function(type){
return new RegExp('(?:' + Date.getMsg(type).map(function(name){
return name.substr(0, 3);
@@ -511,7 +511,7 @@ var build = function(format){
if (year != null) handle.call(date, 'y', year); // need to start in the right year
if ('d' in bits) handle.call(date, 'd', 1);
- if ('m' in bits || bits['b'] || bits['B']) handle.call(date, 'm', 1);
+ if ('m' in bits || bits.b || bits.B) handle.call(date, 'm', 1);
for (var key in bits) handle.call(date, key, bits[key]);
return date;
@@ -54,10 +54,9 @@ Number.implement({
if (precision >= 1 && precision <= 21) value = (+value).toPrecision(precision);
value += '';
-
+ var index;
if (getOption('scientific') === false && value.indexOf('e') > -1){
var match = value.split('e'),
- index,
zeros = +match[1];
value = match[0].replace('.', '');
@@ -42,7 +42,7 @@ Object.extend({
cleanValues: function(object, method){
method = method || defined;
- for (key in object) if (!method(object[key])){
+ for (var key in object) if (!method(object[key])){
delete object[key];
}
return object;
@@ -55,7 +55,7 @@ Object.extend({
run: function(object){
var args = Array.slice(arguments, 1);
- for (key in object) if (object[key].apply){
+ for (var key in object) if (object[key].apply){
object[key].apply(object, args);
}
return object;
@@ -82,15 +82,15 @@ tidy = {
};
var walk = function(string, replacements){
- var result = string;
+ var result = string, key;

This comment has been minimized.

Show comment Hide comment
@anutron

anutron Feb 24, 2011

Owner

I don't really like this; it reads like a tuple.

@anutron

anutron Feb 24, 2011

Owner

I don't really like this; it reads like a tuple.

This comment has been minimized.

Show comment Hide comment
@fat

fat Feb 24, 2011

Contributor

maybe key should be next line?

@fat

fat Feb 24, 2011

Contributor

maybe key should be next line?

This comment has been minimized.

Show comment Hide comment
@arian

arian Feb 24, 2011

Owner

Did this because otherwise I would have done for (var key in replacements){ which would add another var statement. To save this extra chars I simply added it here. Possibly I could've put it on a new line.

@arian

arian Feb 24, 2011

Owner

Did this because otherwise I would have done for (var key in replacements){ which would add another var statement. To save this extra chars I simply added it here. Possibly I could've put it on a new line.

This comment has been minimized.

Show comment Hide comment
@subtleGradient

subtleGradient Feb 24, 2011

Owner

Or you could do

var key, result = string;
@subtleGradient

subtleGradient Feb 24, 2011

Owner

Or you could do

var key, result = string;
for (key in replacements) result = result.replace(replacements[key], key);
return result;
};
var getRegexForTag = function(tag, contents){
tag = tag || '';
- var regstr = contents ? "<" + tag + "(?!\\w)[^>]*>([\\s\\S]*?)<\/" + tag + "(?!\\w)>" : "<\/?" + tag + "([^>]+)?>";
- reg = new RegExp(regstr, "gi");
+ var regstr = contents ? "<" + tag + "(?!\\w)[^>]*>([\\s\\S]*?)<\/" + tag + "(?!\\w)>" : "<\/?" + tag + "([^>]+)?>",
+ reg = new RegExp(regstr, "gi");
return reg;
};
Oops, something went wrong.

4 comments on commit 687d3fc

@anutron

This comment has been minimized.

Show comment Hide comment
@anutron

anutron Feb 24, 2011

Owner

Did this go live onto master w/o a pull request? There's at least one place that doesn't look like a lint change to me. Some of these changes are stylistic in nature and merit discussion before a push (i.e. there should be a pull request).

Owner

anutron replied Feb 24, 2011

Did this go live onto master w/o a pull request? There's at least one place that doesn't look like a lint change to me. Some of these changes are stylistic in nature and merit discussion before a push (i.e. there should be a pull request).

@arian

This comment has been minimized.

Show comment Hide comment
@arian

arian Feb 24, 2011

Owner
Owner

arian replied Feb 24, 2011

@cpojer

This comment has been minimized.

Show comment Hide comment
@cpojer

cpojer Feb 24, 2011

Owner

I reviewed and approved the code. It is a minor change.

Owner

cpojer replied Feb 24, 2011

I reviewed and approved the code. It is a minor change.

@subtleGradient

This comment has been minimized.

Show comment Hide comment
@subtleGradient

subtleGradient Feb 24, 2011

Owner

Overall tons of great little fixes that would have been very slow to find by simply reading the code. Great stuff!
I don't like moving && operators to the end though, that's nasty.

Overall tons of great little fixes that would have been very slow to find by simply reading the code. Great stuff!
I don't like moving && operators to the end though, that's nasty.

Please sign in to comment.