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

Allow options to override error #41

Merged
merged 2 commits into from Jun 12, 2014
Merged

Allow options to override error #41

merged 2 commits into from Jun 12, 2014

Conversation

RR2DO2
Copy link
Contributor

@RR2DO2 RR2DO2 commented Apr 15, 2014

If an error was missing certain fields, they couldn't be provided by the opt parameter. This resolves that. It also checks showStack for actually being true, 'undefined' was filtering through.

If an error was missing certain fields, they couldn't be provided by the opt parameter. This resolves that. It also checks showStack for actually being true, 'undefined' was filtering through.
@yocontra
Copy link
Member

Yay much cleaner - love it. Can you provide a simple test or two for the behavior?

@sindresorhus
Copy link
Contributor

@RR2DO2 ping :)

@RR2DO2
Copy link
Contributor Author

RR2DO2 commented Jun 4, 2014

This completely slipped my mind - added a quick test.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling a989fd2 on RR2DO2:patch-1 into e7ce36a on gulpjs:master.

@yocontra
Copy link
Member

yocontra commented Jun 4, 2014

FYI I'm working on a rewrite of this as its own module

https://github.com/wearefractal/bettererror

yocontra added a commit that referenced this pull request Jun 12, 2014
Allow options to override error
@yocontra yocontra merged commit 06c3dd4 into gulpjs:master Jun 12, 2014
@yocontra
Copy link
Member

Published as 2.2.17

@mtscout6
Copy link
Contributor

mtscout6 commented Jul 2, 2014

Pull request #53 may be an even better solution for this, because I was running into the same problem when I started work on a mtscout6/gulp-cjsx plugin. Since the fileName property that comes from CoffeeScript syntax errors is 'filename' not 'fileName'. Case sensitivity bites sometimes.

@yocontra
Copy link
Member

yocontra commented Jul 2, 2014

@mtscout6 PR to take either filename or fileName off the error would work

@mtscout6
Copy link
Contributor

mtscout6 commented Jul 2, 2014

My main concern is that there it's entirely possible to other properties as well. I'd hate to keep whitelisting different possibilities for all the different libraries that may or will exist. Hence my reasoning for #53 which will bring over all the properties from any error passed in, but giving you the option to reduce to a finite white list if you feel the need to.

@yocontra
Copy link
Member

yocontra commented Jul 3, 2014

#53 makes sense

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

6 participants