Skip to content
This repository has been archived by the owner on Nov 21, 2021. It is now read-only.

Extending options object in renderPartial #5

Merged
merged 1 commit into from
Mar 13, 2014

Conversation

pwalczyszyn
Copy link
Contributor

In the renderPartial function I changed _.defaults to _.extend. This fixes a problem of not being able to have different options in multiple file rendering setup.

In the `renderPartial` function I changed _.defaults to _.extend. This fixes a problem of not being able to have different options in multiple file rendering setup.
@dwightjack
Copy link
Owner

Was thinking the same thing, but i noticed that that way you're extending the main option object.

Wouldn't it be better to use an empty object as first argument? Also providing a default when data is not set would prevent errors:

_.extend({}, options, {filename: fpath}, data || {})

@pwalczyszyn
Copy link
Contributor Author

Yeah, you are right it would be even better this way.

On Wednesday, March 12, 2014, Marco Solazzi [email protected]
wrote:

Was thinking the same thing, but i noticed that that way you're extending
the main option object.

Wouldn't it be better to use an empty object as first argument? Also
providing a default when data is not set would prevent errors:

_.extend({}, options, {filename: fpath}, data || {})

Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-37465654
.

dwightjack added a commit that referenced this pull request Mar 13, 2014
Extending options object in renderPartial
@dwightjack dwightjack merged commit 069fffc into dwightjack:master Mar 13, 2014
@dwightjack
Copy link
Owner

Ok merged and added the fix. Pushed a new versione 0.2.5

@pwalczyszyn
Copy link
Contributor Author

Awesome!
Thx
P.

On Thursday, March 13, 2014, Marco Solazzi [email protected] wrote:

Ok merged and added the fix. Pushed a new versione 0.2.5

Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-37510208
.

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.

2 participants