Fixes from review feedback.

jsha · jsha · commit f3a7c773c7df · 2015-02-04T15:17:31.000-08:00
diff --git a/chromium/background.js b/chromium/background.js
@@ -1,3 +1,6 @@
+/**
+ * Fetch and parse XML to be loaded as RuleSets.
+ */
 function getRuleXml(url) {
   var xhr = new XMLHttpRequest();
   // Use blocking XHR to ensure everything is loaded by the time
diff --git a/chromium/rules.js b/chromium/rules.js
@@ -326,5 +326,6 @@ RuleSets.prototype = {
 };
 
 // Export for HTTPS Rewriter if applicable.
-if (typeof exports != 'undefined')
+if (typeof exports != 'undefined') {
   exports.RuleSets = RuleSets;
+}
diff --git a/rewriter/rewriter.js b/rewriter/rewriter.js
@@ -27,6 +27,11 @@ var path = require("path"),
 
 var ruleSets = null;
 
+/**
+ * For a given directory, recursively edit all files in it that match a filename
+ * pattern representing source code. Replace URLs in those files with rewritten
+ * ones if possible.
+ */
 function processDir(dir) {
   var stream = readdirp({
     root: dir,
@@ -39,7 +44,7 @@ function processDir(dir) {
   stream
   .on('warn', function (err) { 
     console.error('non-fatal error', err); 
-    // optionally call stream.destroy() here in order to abort and cause 'close' to be emitted
+    // Optionally call stream.destroy() here in order to abort and cause 'close' to be emitted
   })
   .on('error', function (err) { console.error('fatal error', err); })
   .pipe(es.mapSync(function (entry) {
@@ -53,13 +58,15 @@ function processDir(dir) {
   }));
 }
 
-// Overwrite the default URI find_uri_expression with a modified one that
-// mitigates a catastrophic backtracking issue common in CSS.
-// The workaround was to insist that URLs start with http, since those are the
-// only ones we want to rewrite anyhow. Note that this may still go exponential
-// on certain inputs. http://www.regular-expressions.info/catastrophic.html
-// Example string that blows up URI.withinString:
-//  image:url(http://img.youtube.com/vi/x7f
+/**
+ * Overwrite the default URI find_uri_expression with a modified one that
+ * mitigates a catastrophic backtracking issue common in CSS.
+ * The workaround was to insist that URLs start with http, since those are the
+ * only ones we want to rewrite anyhow. Note that this may still go exponential
+ * on certain inputs. http://www.regular-expressions.info/catastrophic.html
+ * Example string that blows up URI.withinString:
+ *  image:url(http://img.youtube.com/vi/x7f
+ */
 URI.find_uri_expression = /\b((?:http:(?:\/{1,3}|[a-z0-9%])|www\d{0,3}[.]|[a-z0-9.\-]+[.][a-z]{2,4}\/)(?:[^\s()<>]+)+(?:\(([^\s()<>]+|(\([^\s()<>]+\)))*\)|[^\s`!()\[\]{};:'".,<>?«»“”‘’]))/ig;
 
 function processFile(filename) {
@@ -89,6 +96,9 @@ function processFile(filename) {
   fs.renameSync(filename + ".new", filename);
 }
 
+/**
+ * Load all rulesets for rewriting.
+ */
 function loadRuleSets() {
   console.log("Loading rules...");
   var fileContents = fs.readFileSync(path.join(__dirname, '../pkg/crx/rules/default.rulesets'), 'utf8');
@@ -108,8 +118,9 @@ if (process.argv.length <= 2) {
 
 for (var i = 2; i < process.argv.length; i++) {
   var rewritePath = process.argv[i];
-  if (rewritePath.indexOf('-') == 0)
+  if (rewritePath.indexOf('-') == 0) {
     usage();
+  }
   if (!fs.existsSync(rewritePath)) {
     console.log("Path doesn't exist: " + rewritePath);
     process.exit(1);
