Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

Code refactoring for gutil.PluginError #75

Merged
merged 8 commits into from
Dec 21, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .jshintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"bitwise": true,
"eqeqeq": true,
"expr": true,
"latedef": true,
"newcap": true,
"quotmark": "single",
"regexp": true,
"undef": true,
"unused": true,
"node": true,
"mocha": true
}
7 changes: 0 additions & 7 deletions .npmignore

This file was deleted.

1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
sudo: false
language: node_js
node_js:
- "0.10"
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ var existingError = new Error('OMG');
var err = new gutil.PluginError('test', existingError, {showStack: true});
```

[npm-url]: https://npmjs.org/package/gulp-util
[npm-url]: https://www.npmjs.com/package/gulp-util
[npm-image]: https://badge.fury.io/js/gulp-util.svg
[travis-url]: https://travis-ci.org/gulpjs/gulp-util
[travis-image]: https://travis-ci.org/gulpjs/gulp-util.svg?branch=master
[travis-image]: https://img.shields.io/travis/gulpjs/gulp-util.svg?branch=master
[coveralls-url]: https://coveralls.io/r/gulpjs/gulp-util
[coveralls-image]: https://coveralls.io/repos/gulpjs/gulp-util/badge.png
[coveralls-image]: https://img.shields.io/coveralls/gulpjs/gulp-util.svg
[depstat-url]: https://david-dm.org/gulpjs/gulp-util
[depstat-image]: https://david-dm.org/gulpjs/gulp-util.svg
44 changes: 27 additions & 17 deletions lib/PluginError.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
var util = require('util');
var arrayDiffer = require('array-differ');
var arrayUniq = require('array-uniq');
var chalk = require('chalk');
var _ = require('lodash');
var objectAssign = require('object-assign');

var nonEnumberableProperties = ['name', 'message', 'stack'];
var propertiesNotToDisplay = nonEnumberableProperties.concat(['plugin', 'showStack', 'showProperties', '__safety', '_stack']);

// wow what a clusterfuck
var parseOptions = function(plugin, message, opt) {
if (!opt) opt = {};
opt = opt || {};
if (typeof plugin === 'object') {
opt = plugin;
} else {
Expand All @@ -21,12 +23,10 @@ var parseOptions = function(plugin, message, opt) {
opt.plugin = plugin;
}

_.defaults(opt, {
return objectAssign({
showStack: false,
showProperties: true
})

return opt;
}, opt);
};

function PluginError(plugin, message, opt) {
Expand All @@ -35,14 +35,15 @@ function PluginError(plugin, message, opt) {
Error.call(this);

var options = parseOptions(plugin, message, opt);
var self = this;

// if options has an error, grab details from it
if (options.error) {
// These properties are not enumerable, so we have to add them explicitly.
_.uniq(Object.keys(options.error).concat(nonEnumberableProperties))
arrayUniq(Object.keys(options.error).concat(nonEnumberableProperties))
.forEach(function(prop) {
this[prop] = options.error[prop];
}, this);
self[prop] = options.error[prop];
});
}

var properties = ['name', 'message', 'fileName', 'lineNumber', 'stack', 'showStack', 'showProperties', 'plugin'];
Expand Down Expand Up @@ -86,14 +87,22 @@ PluginError.prototype._messageWithDetails = function() {
};

PluginError.prototype._messageDetails = function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this whole fn needs some comments about what its doing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this function for furher code refactoring. 272993a

Now it becomes enough readable so we don't need to add any comments here, I think.

var properties;
if (this.showProperties) {
var properties = _(Object.keys(this))
.filter(function(prop) { return propertiesNotToDisplay.indexOf(prop) === -1; })
.map(function(prop) { return '\n ' + prop + ': ' + this[prop]; }, this)
.reduce(function(properties, next) { return properties + next; });
if (!this.showProperties) {
return '';
}

var properties = arrayDiffer(Object.keys(this), propertiesNotToDisplay);

if (properties.length === 0) {
return '';
}
return properties === undefined ? '' : ('Details:' + properties)

var self = this;
properties = properties.map(function stringifyProperty(prop) {
return ' ' + prop + ': ' + self[prop];
});

return 'Details:\n' + properties.join('\n');
};

PluginError.prototype.toString = function () {
Expand All @@ -102,6 +111,7 @@ PluginError.prototype.toString = function () {
return this._messageWithDetails() + '\nStack:\n' + stack;
}.bind(this);

var msg;
if (this.showStack) {
if (this.__safety) { // There is no wrapped error, use the stack captured in the PluginError ctor
msg = this.__safety.stack;
Expand All @@ -114,7 +124,7 @@ PluginError.prototype.toString = function () {
msg = this._messageWithDetails();
}

return sig+'\n'+msg;
return sig + '\n' + msg;
};

module.exports = PluginError;
2 changes: 1 addition & 1 deletion lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ module.exports = function(fn) {
cb();
};
return through.obj(push, end);
};
};
2 changes: 1 addition & 1 deletion lib/env.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var parseArgs = require('minimist');
var argv = parseArgs(process.argv.slice(2));

module.exports = argv;
module.exports = argv;
2 changes: 1 addition & 1 deletion lib/isBuffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ var Buffer = buf.Buffer;
// could use Buffer.isBuffer but this is the same exact thing...
module.exports = function(o) {
return typeof o === 'object' && o instanceof Buffer;
};
};
2 changes: 1 addition & 1 deletion lib/isNull.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module.exports = function(v) {
return v === null;
};
};
2 changes: 1 addition & 1 deletion lib/isStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ var Stream = require('stream').Stream;

module.exports = function(o) {
return !!o && o instanceof Stream;
};
};
35 changes: 21 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,41 @@
"version": "3.0.1",
"repository": "wearefractal/gulp-util",
"author": "Fractal <contact@wearefractal.com> (http://wearefractal.com/)",
"files": [
"index.js",
"lib",
"LECENSE"
],
"dependencies": {
"array-differ": "^1.0.0",
"array-uniq": "^1.0.2",
"beeper": "^1.0.0",
"chalk": "^0.5.1",
"dateformat": "^1.0.7-1.2.3",
"lodash": "^2.4.1",
"dateformat": "^1.0.11",
"lodash._reinterpolate": "^2.4.1",
"lodash.template": "^2.4.1",
"minimist": "^1.1.0",
"multipipe": "^0.1.1",
"multipipe": "^0.1.2",
"object-assign": "^2.0.0",
"replace-ext": "0.0.1",
"through2": "^0.6.1",
"through2": "^0.6.3",
"vinyl": "^0.4.3"
},
"devDependencies": {
"buffer-equal": "^0.0.1",
"coveralls": "^2.7.0",
"event-stream": "^3.1.0",
"istanbul": "^0.3.2",
"jshint": "^2.4.1",
"coveralls": "^2.11.2",
"event-stream": "^3.1.7",
"istanbul": "^0.3.5",
"istanbul-coveralls": "^1.0.1",
"jshint": "^2.5.11",
"lodash.templatesettings": "^2.4.1",
"mocha": "^1.17.0",
"mocha-lcov-reporter": "^0.0.1",
"rimraf": "^2.2.5",
"should": "^4.0.0"
"mocha": "^2.0.1",
"rimraf": "^2.2.8",
"should": "^4.4.1"
},
"scripts": {
"test": "mocha --reporter spec && jshint",
"coveralls": "istanbul cover _mocha --report lcovonly -- -R spec && cat ./coverage/lcov.info | coveralls && rm -rf ./coverage"
"test": "jshint *.js lib/*.js test/*.js && mocha",
"coveralls": "istanbul cover _mocha --report lcovonly && istanbul-coveralls"
},
"engines": {
"node": ">=0.10"
Expand Down
2 changes: 1 addition & 1 deletion test/File.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var util = require('../');
var util = require('..');
var should = require('should');
var path = require('path');
require('mocha');
Expand Down
39 changes: 19 additions & 20 deletions test/PluginError.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
var util = require('../');
var should = require('should');
var path = require('path');
var util = require('..');
require('should');
require('mocha');

describe('PluginError()', function(){
Expand All @@ -17,19 +16,19 @@ describe('PluginError()', function(){

it('should print the plugin name in toString', function(){
var err = new util.PluginError('test', 'something broke');
err.toString().indexOf('test').should.not.equal(-1);
err.toString().should.containEql('test');
});

it('should not include the stack by default in toString', function(){
var err = new util.PluginError('test', 'something broke');
// just check that there are no 'at' lines
err.toString().indexOf('at').should.equal(-1);
err.toString().should.not.containEql('at');
});

it('should include the stack when specified in toString', function(){
var err = new util.PluginError('test', 'something broke', {stack: "at huh", showStack: true});
var err = new util.PluginError('test', 'something broke', {stack: 'at huh', showStack: true});
// just check that there are 'at' lines
err.toString().indexOf('at').should.not.equal(-1);
err.toString().should.containEql('at');
});

it('should take arguments as one object', function(){
Expand Down Expand Up @@ -95,36 +94,36 @@ describe('PluginError()', function(){
var err = new util.PluginError('test', 'it broke', {showProperties: true});
err.fileName = 'original.js';
err.lineNumber = 35;
err.toString().indexOf('it broke').should.not.equal(-1);
err.toString().indexOf('message:').should.equal(-1);
err.toString().indexOf('fileName:').should.not.equal(-1);
err.toString().should.containEql('it broke');
err.toString().should.not.containEql('message:');
err.toString().should.containEql('fileName:');
});

it('should show properties', function() {
var realErr = new Error('something broke');
realErr.fileName = 'original.js';
realErr.lineNumber = 35;
var err = new util.PluginError('test', realErr, {showProperties: true});
err.toString().indexOf('message:').should.equal(-1);
err.toString().indexOf('fileName:').should.not.equal(-1);
err.toString().should.not.containEql('message:');
err.toString().should.containEql('fileName:');
});

it('should not show properties', function() {
var realErr = new Error('something broke');
realErr.fileName = 'original.js';
realErr.lineNumber = 35;
var err = new util.PluginError('test', realErr, {showProperties: false});
err.toString().indexOf('message:').should.equal(-1);
err.toString().indexOf('fileName:').should.equal(-1);
err.toString().should.not.containEql('message:');
err.toString().should.not.containEql('fileName:');
});

it('should not show properties, but should show stack', function() {
var err = new util.PluginError('test', 'it broke', {stack: 'test stack', showStack: true, showProperties: false});
err.fileName = 'original.js';
err.lineNumber = 35;
err.toString().indexOf('message:').should.equal(-1);
err.toString().indexOf('fileName:').should.equal(-1);
err.toString().indexOf('test stack').should.not.equal(-1);
err.toString().should.not.containEql('message:');
err.toString().should.not.containEql('fileName:');
err.toString().should.containEql('test stack');
});

it('should not show properties, but should show stack for real error', function() {
Expand Down Expand Up @@ -171,17 +170,17 @@ describe('PluginError()', function(){
this.timeout(100);

var err = new util.PluginError('test', 'it broke', {showStack: true});
var str = err.toString();
err.toString();

done();
});

it('should toString quickly with original error', function(done) {
this.timeout(100);

var realErr = new Error('it broke')
var realErr = new Error('it broke');
var err = new util.PluginError('test', realErr, {showStack: true});
var str = err.toString();
err.toString();

done();
});
Expand Down
7 changes: 3 additions & 4 deletions test/beep.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
var util = require('../');
var should = require('should');
var path = require('path');
var util = require('..');
require('should');
require('mocha');

describe('beep()', function(){
Expand All @@ -20,4 +19,4 @@ describe('beep()', function(){
process.stdout.write = stdout_write;
done();
});
});
});
3 changes: 1 addition & 2 deletions test/buffer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
var util = require('../');
var util = require('..');
var should = require('should');
var path = require('path');
var es = require('event-stream');
require('mocha');

Expand Down
7 changes: 3 additions & 4 deletions test/colors.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
var util = require('../');
var should = require('should');
var path = require('path');
var util = require('..');
require('should');
require('mocha');

describe('colors', function(){
it('should be a chalk instance', function(done){
util.colors.should.equal(require('chalk'));
done();
});
});
});
3 changes: 1 addition & 2 deletions test/combine.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
var util = require('../');
var util = require('..');
var should = require('should');
var path = require('path');
var es = require('event-stream');
var Stream = require('stream');
require('mocha');
Expand Down
Loading