From cb3869dce7f2cb179240ac6e0653cbfd94e4d0e8 Mon Sep 17 00:00:00 2001 From: Matěj Cepl Date: Fri, 18 Jun 2010 00:51:50 +0200 Subject: Write HACKING document with coding guidelines and actually fix the code to follow it. --- HACKING | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++ lib/color.js | 58 +++++++++++++++++++++---------------------- lib/logger.js | 40 +++++++++++++++--------------- lib/offline-support.js | 14 +++++------ lib/xmlrpc.js | 44 ++++++++++++++++----------------- 5 files changed, 145 insertions(+), 78 deletions(-) create mode 100644 HACKING diff --git a/HACKING b/HACKING new file mode 100644 index 0000000..0d2cdfe --- /dev/null +++ b/HACKING @@ -0,0 +1,67 @@ +First attempt to create coding guidelines +----------------------------------------- + +1) Generally formatting style should be roughly similar to Java style + (similar to K&R), so that function looks like this: + + function Foo (bar, baz) { + if (bar) { + console.log("bar!"); + return baz / 2; + } else { + console.log("no bar!"); + return baz * 2; + } + } + +2) Indentation is 4 spaces, no TABs + +3) Generally I would like to follow formatting and other advice by jslint + (http://www.jslint.com/). However, I don't want to be bound by sometimes + eccentric opinions of Douglas Crockford, more I care about checking + for missing semicolons, bad spacing, etc. However, in order to make + jslint at least partially work, I need to be quite conservative in + using any non-standard constructs (Douglas strictly refused to add + any non-standard pieces of syntax, particularly let, when I emailed + him about it). + It follows that let and other non-ECMAScript construcsts shouldn't be used + or at least sparingly and only in the time of dire need. + +4) Object literals should be used for pure data objects only. Instead of + + var Foo.prototype = { + bar: function bar () { + console.log("We are in bar! Yuchuuu!!!"); + }, + baz: function baz () { + console.log("We are in baz :("); + } + }; + + write rather two functions separately as in + + Foo.prototype.bar = function bar () { + console.log("We are in bar! Yay!!!"); + }; + + Foo.prototype.baz = function baz () { + console.log("We are in baz :( Shooot."); + }; + + The latter is in my opinion more readable and works better in editors + I use (gedit, scribes, and eclipse). + +5) Use plain Javascript only. This is not multi-platform (meaning mult-browser) + script so no Javascript libraries (e.g., jQuery, Dojo, etc.) should be + used. + On the other hand, we don't have to bound ourselves to compatibility + with anything else than platforms Jetpack runs on ... which is currently + (for -prototype) FF 3.5 and for Jetpack-SDK it will be probably even + higher when it is going to be actually released. So, instead of + hopeless non-standard constructs of jQuery, element.querySelector (with + standard CSS3 selector, which is NOT the same as what's used in + jQuery; see http://www.w3.org/TR/2009/PR-css3-selectors-20091215/) and other + advanced Javascript functions can be used. + +Any other items could be added after discussion and when a need to decide +some pressing issue happens. diff --git a/lib/color.js b/lib/color.js index 2da2fa7..9a09707 100644 --- a/lib/color.js +++ b/lib/color.js @@ -38,9 +38,9 @@ Color.prototype.hs = function(nStr) { }; Color.prototype.toString = function() { - let rH = Number(this.r.toFixed()).toString(16); - let gH = Number(this.g.toFixed()).toString(16); - let bH = Number(this.b.toFixed()).toString(16); + var rH = Number(this.r.toFixed()).toString(16); + var gH = Number(this.g.toFixed()).toString(16); + var bH = Number(this.b.toFixed()).toString(16); return "#" + this.hs(rH) + this.hs(gH) + this.hs(bH); }; @@ -55,16 +55,16 @@ Color.prototype.toString = function() { * @return Array The HSL representation */ Color.prototype.hsl = function() { - let r = this.r / 255; - let g = this.g / 255; - let b = this.b / 255; - let max = Math.max(r, g, b), min = Math.min(r, g, b); - let h, s, l = (max + min) / 2; + var r = this.r / 255; + var g = this.g / 255; + var b = this.b / 255; + var max = Math.max(r, g, b), min = Math.min(r, g, b); + var h, s, l = (max + min) / 2; if (max === min) { h = s = 0; // achromatic } else { - let d = max - min; + var d = max - min; s = l > 0.5 ? d / (2 - max - min) : d / (max + min); switch (max) { case r: @@ -113,13 +113,13 @@ Color.prototype.hslToRgb = function(h, s, l) { return p; } - let r, g, b; + var r, g, b; if (s === 0) { r = g = b = l; // achromatic } else { - let q = l < 0.5 ? l * (1 + s) : l + s - l * s; - let p = 2 * l - q; + var q = l < 0.5 ? l * (1 + s) : l + s - l * s; + var p = 2 * l - q; r = hue2rgb(p, q, h + 1 / 3); g = hue2rgb(p, q, h); b = hue2rgb(p, q, h - 1 / 3); @@ -139,13 +139,13 @@ Color.prototype.hslToRgb = function(h, s, l) { * @return Array The HSV representation */ Color.prototype.hsv = function() { - let r = this.r / 255; - let g = this.g / 255; - let b = this.b / 255; - let max = Math.max(r, g, b), min = Math.min(r, g, b); - let h, s, v = max; + var r = this.r / 255; + var g = this.g / 255; + var b = this.b / 255; + var max = Math.max(r, g, b), min = Math.min(r, g, b); + var h, s, v = max; - let d = max - min; + var d = max - min; s = max === 0 ? 0 : d / max; if (max === min) { @@ -179,13 +179,13 @@ Color.prototype.hsv = function() { * @return Array The RGB representation */ Color.prototype.hsvToRgb = function(h, s, v) { - let r, g, b; + var r, g, b; - let i = Math.floor(h * 6); - let f = h * 6 - i; - let p = v * (1 - s); - let q = v * (1 - f * s); - let t = v * (1 - (1 - f) * s); + var i = Math.floor(h * 6); + var f = h * 6 - i; + var p = v * (1 - s); + var q = v * (1 - f * s); + var t = v * (1 - (1 - f) * s); switch (i % 6) { case 0: @@ -227,10 +227,10 @@ Color.prototype.hsvToRgb = function(h, s, v) { * Provide */ Color.prototype.lightColor = function() { - let hslArray = this.hsl(); - let h = Number(hslArray[0]); - let s = Number(hslArray[1]) * this.Desaturated; - let l = this.Luminosity; - let desA = this.hslToRgb(h, s, l); + var hslArray = this.hsl(); + var h = Number(hslArray[0]); + var s = Number(hslArray[1]) * this.Desaturated; + var l = this.Luminosity; + var desA = this.hslToRgb(h, s, l); return new Color(desA[0], desA[1], desA[2]); }; diff --git a/lib/logger.js b/lib/logger.js index b817056..ad7f4e9 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -14,21 +14,21 @@ var Logger = exports.Logger = function Logger(store, abbsMap) { }; Logger.prototype.addLogRecord = function(that) { - let rec = {}; + var rec = {}; rec.date = new Date(); rec.url = that.doc.location.toString(); rec.title = that.title; - let comment = jetpack.tabs.focused.contentWindow.prompt( + var comment = jetpack.tabs.focused.contentWindow.prompt( "Enter comments for this comment"); if (comment && comment.length > 0) { comment = comment.trim(); rec.comment = comment; - let recKey = utilMod.getISODate(rec.date) + "+" + var recKey = utilMod.getISODate(rec.date) + "+" + urlMod.parse(rec.url).host + "+" + that.bugNo; console.log("rec = " + rec.toSource()); - let clearLogAElem = that.doc.getElementById("clearLogs"); + var clearLogAElem = that.doc.getElementById("clearLogs"); if (clearLogAElem.style.color != this.FullLogsColor) { clearLogAElem.style.color = this.FullLogsColor; clearLogAElem.style.fontWeight = "bolder"; @@ -51,12 +51,12 @@ Logger.prototype.getBugzillaAbbr = function(url) { } Logger.prototype.timeSheetRecordsPrinter = function(body, records) { - let that = this; - let commentBugRE = new RegExp("[bB]ug\\s+([0-9]+)","g"); + var that = this; + var commentBugRE = new RegExp("[bB]ug\\s+([0-9]+)","g"); // sort the records into temporary array - let tmpArr = []; + var tmpArr = []; - for ( let i in records) { + for ( var i in records) { if (records.hasOwnProperty(i)) { tmpArr.push( [ i, records[i] ]); } @@ -65,21 +65,21 @@ Logger.prototype.timeSheetRecordsPrinter = function(body, records) { return a[0] > b[0] ? 1 : -1; }); - let currentDay = ""; + var currentDay = ""; // now print the array tmpArr.forEach(function(rec) { - let x = rec[1]; - let dayStr = utilMod.getISODate(x.date); - let host = urlMod.parse(x.url).host; - let BZName = that.getBugzillaAbbr(x.url); - let bugNo = utilMod.getBugNo(x.url); + var x = rec[1]; + var dayStr = utilMod.getISODate(x.date); + var host = urlMod.parse(x.url).host; + var BZName = that.getBugzillaAbbr(x.url); + var bugNo = utilMod.getBugNo(x.url); if (dayStr != currentDay) { currentDay = dayStr; body.innerHTML += "

" + currentDay + "

"; } // replace "bug ####" with a hyperlink to the current bugzilla - let comment = x.comment.replace(commentBugRE, + var comment = x.comment.replace(commentBugRE, "$&"); body.innerHTML += "

\n"; xml += "\n"; @@ -44,8 +44,8 @@ XMLRPCMessage.prototype.xml = function() { xml += "\n"; // do individual parameters - for ( let i = 0; i < this.params.length; i++) { - let data = this.params[i]; + for ( var i = 0; i < this.params.length; i++) { + var data = this.params[i]; xml += "\n"; xml += "" + this.getParamXML(this.dataTypeOf(data), @@ -61,7 +61,7 @@ XMLRPCMessage.prototype.xml = function() { XMLRPCMessage.prototype.dataTypeOf = function(o) { // identifies the data type - let type = typeof (o); + var type = typeof (o); type = type.toLowerCase(); switch (type) { case "number": @@ -71,7 +71,7 @@ XMLRPCMessage.prototype.dataTypeOf = function(o) { type = "double"; break; case "object": - let con = o.constructor; + var con = o.constructor; if (con == Date) type = "date"; else if (con == Array) @@ -84,45 +84,45 @@ XMLRPCMessage.prototype.dataTypeOf = function(o) { }; XMLRPCMessage.prototype.doValueXML = function(type, data) { - let xml = "<" + type + ">" + data + ""; + var xml = "<" + type + ">" + data + ""; return xml; }; XMLRPCMessage.prototype.doBooleanXML = function(data) { - let value = (data == true) ? 1 : 0; - let xml = "" + value + ""; + var value = (data == true) ? 1 : 0; + var xml = "" + value + ""; return xml; }; XMLRPCMessage.prototype.doDateXML = function(data) { - let leadingZero = function (n) { + var leadingZero = function (n) { // pads a single number with a leading zero. Heh. if (n.length == 1) n = "0" + n; return n; }; - let dateToISO8601 = function(date) { + var dateToISO8601 = function(date) { // wow I hate working with the Date object - let year = new String(date.getYear()); - let month = this.leadingZero(new String(date.getMonth())); - let day = this.leadingZero(new String(date.getDate())); - let time = this.leadingZero(new String(date.getHours())) + ":" + var year = new String(date.getYear()); + var month = this.leadingZero(new String(date.getMonth())); + var day = this.leadingZero(new String(date.getDate())); + var time = this.leadingZero(new String(date.getHours())) + ":" + this.leadingZero(new String(date.getMinutes())) + ":" + this.leadingZero(new String(date.getSeconds())); - let converted = year + month + day + "T" + time; + var converted = year + month + day + "T" + time; return converted; }; - let xml = ""; + var xml = ""; xml += dateToISO8601(data); xml += ""; return xml; }; XMLRPCMessage.prototype.doArrayXML = function(data) { - let xml = "\n"; - for ( let i = 0; i < data.length; i++) { + var xml = "\n"; + for ( var i = 0; i < data.length; i++) { xml += "" + this.getParamXML(this.dataTypeOf(data[i]), data[i]) + "\n"; @@ -132,8 +132,8 @@ XMLRPCMessage.prototype.doArrayXML = function(data) { }; XMLRPCMessage.prototype.doStructXML = function(data) { - let xml = "\n"; - for ( let i in data) { + var xml = "\n"; + for ( var i in data) { xml += "\n"; xml += "" + i + "\n"; xml += "" @@ -146,7 +146,7 @@ XMLRPCMessage.prototype.doStructXML = function(data) { }; XMLRPCMessage.prototype.getParamXML = function(type, data) { - let xml; + var xml; switch (type) { case "date": xml = this.doDateXML(data); -- cgit