Skip to content

Commit 50d8903

Browse files
author
benholloway
committed
improvements suggested by @bguiz 
1 parent 95c6c0b commit 50d8903

File tree

5 files changed

+64
-51
lines changed

5 files changed

+64
-51
lines changed

‎lib/build/browserify.js‎

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
'use strict';
22

33
var fs = require('fs'),
4-
path = require('path'),
5-
through = require('through2'),
4+
path = require('path');
5+
6+
var through = require('through2'),
67
merge = require('lodash.merge'),
78
trackFilenames = require('gulp-track-filenames'),
89
transformTools = require('browserify-transform-tools'),
@@ -49,7 +50,7 @@ function isMethodAST(node) {
4950
* @return A jasmine transform
5051
*/
5152
function jasmineTransform(symbol) {
52-
return transformTools.makeFalafelTransform('jasmineTransform', null, function (node, options, done) {
53+
return transformTools.makeFalafelTransform('jasmineTransform', null, function jasmineASTWalker(node, options, done) {
5354
var isValid = isLiteralAST(node, symbol) && isMethodAST(node.parent, 'describe', 'module');
5455
if (isValid) {
5556
node.update('\'' + options.file.replace(/\\/g, '\\\\') + ':0:0\'');
@@ -178,26 +179,33 @@ function compile(bannerWidth, transforms) {
178179
// error handler
179180
var timeout;
180181
function errorHandler(error) {
181-
var text = error.toString();
182-
[
182+
183+
// run a bunch of tests against the error in order to determine the appropriate error message
184+
// there will be at least one truthy value, even if that is the final placeholder
185+
var text = error.toString();
186+
var message = [
187+
183188
// SyntaxError: <file>:<reason>:<line>:<column>
184-
function() {
189+
function testSyntaxError() {
185190
var analysis = /^\s*SyntaxError\:\s*([^:]*)\s*\:\s*([^(]*)\s*\((\d+:\d+)\)\s*\n/.exec(text);
186191
return analysis && ([analysis[1], analysis[3], analysis[2]].join(':') + '\n');
187192
},
193+
188194
// Error: SyntaxError: <reason> while parsing json file <file>
189-
function() {
195+
function testSyntaxErrorJSON() {
190196
var analysis = /^\s*Error: SyntaxError\:\s*(.*)\s*while parsing json file\s*([^]*)/.exec(text);
191197
return analysis && ([analysis[2], '0', '0', ' ' + analysis[1]].join(':') + '\n');
192198
},
199+
193200
// Line <line>: <reason>: <file>
194-
function() {
201+
function testGeneric() {
195202
var analysis = /Line\s*(\d+)\s*\:\s*([^:]*)\s*:\s*(.*)\s*/.exec(text);
196203
return analysis && ([analysis[3], analysis[1], 0, ' ' + analysis[2]].join(':') + '\n');
197204
},
205+
198206
// Error: Cannot find module '<reason>' from '<directory>'
199207
// find the first text match for any text quoted in <reason>
200-
function() {
208+
function testBadImport() {
201209
var analysis = /^\s*Error\: Cannot find module '(.*)\'\s*from\s*\'(.*)\'\s*$/.exec(text);
202210
if (analysis) {
203211
var filename = fs.readdirSync(analysis[2])
@@ -211,20 +219,22 @@ function compile(bannerWidth, transforms) {
211219
return path.join(analysis[2], filename) + ':0:0: Cannot find import ' + analysis[1] + '\n';
212220
}
213221
},
222+
214223
// Unknown
215-
function() {
224+
function otherwise() {
216225
return 'TODO parse this error\n' + text + '\n';
217226
}
218227
]
219-
.map(function(method) {
220-
return method();
228+
.map(function invokeTestMethod(testMethod) {
229+
return testMethod();
221230
})
222231
.filter(Boolean)
223-
.forEach(function addUnique(message) {
224-
if (output.indexOf(message) < 0) {
225-
output.push(message);
226-
}
227-
});
232+
.shift();
233+
234+
// add unique
235+
if (output.indexOf(message) < 0) {
236+
output.push(message);
237+
}
228238

229239
// complete overall only once there are no further errors
230240
clearTimeout(timeout);
@@ -247,7 +257,7 @@ function compile(bannerWidth, transforms) {
247257
// transforms
248258
transforms
249259
.concat(requireTransform(false))
250-
.forEach(function (item, i, list) {
260+
.forEach(function eachItem(item, i, list) {
251261
if (typeof item === 'function') {
252262
var opts = (typeof list[i+1] === 'object') ? merge({ global: true }, list[i+1]) : { global: true };
253263
bundler.transform(item, opts);
@@ -256,7 +266,7 @@ function compile(bannerWidth, transforms) {
256266

257267
// require statements
258268
[].concat(files)
259-
.forEach(function (item) {
269+
.forEach(function eachItem(item) {
260270
bundler.require(item, {entry: true});
261271
});
262272

@@ -278,7 +288,7 @@ function compile(bannerWidth, transforms) {
278288

279289
// when we use minification we will get: error, code, source-map
280290
// when we don't we will get: error, buffer(with embedded source map)
281-
bundler.bundle(function (error, codeOrBuffer, map) {
291+
bundler.bundle(function onComplete(error, codeOrBuffer, map) {
282292
if (!error) {
283293
var code = codeOrBuffer.toString();
284294
var sourceMap = map ? JSON.parse(map) : convert.fromComment(code).toObject();
@@ -305,10 +315,10 @@ function compile(bannerWidth, transforms) {
305315
* @param {string} [sourceMapBase] Base path for source map file
306316
* @returns {stream.Through}
307317
*/
308-
each: function (isMinify, sourceMapBase) {
309-
return through.obj(function (file, encoding, done) {
318+
each: function gulpBundleEach(isMinify, sourceMapBase) {
319+
return through.obj(function transformFn(file, encoding, done) {
310320
bundle(this, [file.path], file.relative, isMinify, sourceMapBase, done);
311-
}, function (done) {
321+
}, function flushFn(done) {
312322
flushErrors();
313323
done();
314324
});
@@ -321,12 +331,12 @@ function compile(bannerWidth, transforms) {
321331
* @param {string} [sourceMapBase] Base path for source map file
322332
* @returns {stream.Through}
323333
*/
324-
all: function (outPath, isMinify, sourceMapBase) {
334+
all: function gulpBundleAll(outPath, isMinify, sourceMapBase) {
325335
var pending = [];
326-
return through.obj(function (file, encoding, done) {
336+
return through.obj(function transformFn(file, encoding, done) {
327337
pending.push(file.path);
328338
done();
329-
}, function (done) {
339+
}, function flushFn(done) {
330340
if (pending.length) {
331341
bundle(this, pending, outPath, isMinify, sourceMapBase, function () {
332342
flushErrors();

‎lib/build/node-sass.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,12 @@ function encodeRelativeURL(startPath, uri) {
6363

6464
// find path to the root, stopping at cwd, package.json or bower.json
6565
var pathToRoot = [];
66-
var isWorking = true;
67-
while (isWorking) {
66+
var isWorking;
67+
do {
6868
pathToRoot.push(absoluteStart);
6969
isWorking = (absoluteStart !== process.cwd()) && notPackage(absoluteStart);
7070
absoluteStart = path.resolve(absoluteStart, '..');
71-
}
71+
} while (isWorking);
7272

7373
// start a queue with the path to the root
7474
var queue = pathToRoot.concat();
@@ -83,9 +83,9 @@ function encodeRelativeURL(startPath, uri) {
8383

8484
// file exists so convert to a dataURI and end
8585
if (fs.existsSync(fullPath)) {
86-
var type = mime.lookup(fullPath);
86+
var type = mime.lookup(fullPath);
8787
var contents = fs.readFileSync(fullPath);
88-
var base64 = new Buffer(contents).toString('base64');
88+
var base64 = new Buffer(contents).toString('base64');
8989
return 'url(data:' + type + ';base64,' + base64 + ')';
9090
}
9191
// enqueue subdirectories that are not packages and are not in the root path

‎lib/inject/adjacent-files.js‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,23 @@ module.exports = function (extension, opts, recurse) {
3232
throw new Error('encountered a file that is outside the given recurse path');
3333
}
3434

35-
// use terms in the relative address, from none to all
36-
var split = fileRelative.split(/[\\\/]/g);
37-
for(var i = 0, glob = []; i < split.length; i++) {
38-
extensions.forEach(addSliceToGlob(glob, 0, i + 1));
39-
}
40-
function addSliceToGlob(glob, start, stop) {
41-
return function (extension) {
42-
var item = [ fileRecurse ]
43-
.concat(split.slice(start, stop))
44-
.concat('*.' + extension)
45-
.join('/');
46-
if (glob.indexOf(item) < 0) {
47-
glob.push(item);
48-
}
49-
};
50-
}
35+
// use elements in the relative address, from none to all
36+
// for each term add a matcher for all files of the given extension
37+
// TODO @bholloway can this be replaced by a single term? i.e. /**/*.<ext>
38+
var glob = [];
39+
fileRelative
40+
.split(/[\\\/]/g)
41+
.forEach(function eachPathElement(value, i, array) {
42+
extensions.forEach(function eachExtension(extension) {
43+
var item = [ fileRecurse ]
44+
.concat(array.slice(0, i + 1))
45+
.concat('*.' + extension)
46+
.join('/');
47+
if (glob.indexOf(item) < 0) {
48+
glob.push(item);
49+
}
50+
});
51+
});
5152
return gulp.src(glob, { read: false })
5253
.pipe(semiflat(file.base))
5354
.pipe(slash());

‎lib/inject/bower-files.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ module.exports = function () {
8484

8585
function greatestCommonBase(absolute) {
8686
if (base) {
87-
for (var i = 0; (base[i] === absolute[i]); i++) {} /* jshint -W035 */
87+
var i = 0;
88+
while(base[i] === absolute[i]) {
89+
i++;
90+
}
8891
base = base.slice(0, i);
8992
} else if (options.base === true) {
9093
base = path.dirname(absolute);

‎lib/test/karma.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,8 @@ function karmaCreateConfig(reporters, configFileName) {
136136
done();
137137
}
138138
function flushFn(done) {
139+
/* jshint validthis:true */
140+
var stream = this;
139141
function requirePlugins(reporter) {
140142
return 'require("' + getKarmaReporterPluginPath(reporter) + '")';
141143
}
@@ -144,9 +146,6 @@ function karmaCreateConfig(reporters, configFileName) {
144146
}
145147
var filePath = path.resolve(configFileName);
146148
if (fs.existsSync(filePath)) {
147-
// Can safely ignore JsHint warnign here because through2 sets the value of
148-
// `this` to the file stream
149-
var stream = this; // jshint ignore:line
150149
var contents = fs.readFileSync(filePath).toString()
151150
.replace(basePathRegex, encode(process.cwd()))
152151
.replace(filesAppendRegex, encode(files.concat(additional)))

0 commit comments

Comments
 (0)