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

Expose option to override properties included on Plugin Error, copy all properties from original error by default #53

Merged
merged 1 commit into from Jul 11, 2014

Conversation

mtscout6
Copy link
Contributor

@mtscout6 mtscout6 commented Jul 2, 2014

Currently when you create a PluginError with an existing error the properties of the original error are lost unless they are included on an internal list of white listed properties. I propose two enhancements:

  1. Provide a means to override the white listed properties.
  2. If no white list properties are provided, then include all properties from the original error.

@mtscout6
Copy link
Contributor Author

mtscout6 commented Jul 2, 2014

I would think this should require a minor revision bump.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 164b968 on mtscout6:plugin-error-properties into 4c94141 on gulpjs:master.

opt.properties = [];
}

opt.properties = _.uniq(defaultProperties.concat(opt.properties));
Copy link
Member

Choose a reason for hiding this comment

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

if you're using lodash for only one function, use the split out version. the module for this would be lodash.uniq

Copy link
Contributor

Choose a reason for hiding this comment

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

or just https://github.com/sindresorhus/array-uniq which is completely self-contained.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 1e0f24e on mtscout6:plugin-error-properties into 4c94141 on gulpjs:master.

@mtscout6
Copy link
Contributor Author

What's the verdict on this?

@yocontra
Copy link
Member

needs a rebase for merge

…ll properties from original error by default
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 2703722 on mtscout6:plugin-error-properties into a44e450 on gulpjs:master.

@mtscout6
Copy link
Contributor Author

Rebased!

@yocontra
Copy link
Member

My only complaint is the "properties" field name - I think we need something a little more descriptive. What does "properties" mean if I do new PluginError('plugin name', 'error msg', {properties: ?})

@mtscout6
Copy link
Contributor Author

How does errorPropertiesToCopy sound? Or do you have a better alternative?

yocontra added a commit that referenced this pull request Jul 11, 2014
Expose option to override properties included on Plugin Error, copy all properties from original error by default
@yocontra yocontra merged commit 45c55f4 into gulpjs:master Jul 11, 2014
@yocontra
Copy link
Member

I'll take another PR documenting these changes before I publish

@mtscout6 mtscout6 deleted the plugin-error-properties branch July 14, 2014 19:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants