Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for Multiple View Folders #2320

Closed
wants to merge 3 commits into from

Conversation

martindale
Copy link

This adds support for supplying an array of view folders in place of a single view folder [string], allowing for a "fallback" folder of views to be overridden by a preferred location, if they exist.

@dougwilson dougwilson self-assigned this Aug 25, 2014
@@ -54,14 +54,25 @@ function View(name, options) {

View.prototype.lookup = function(path){
var ext = this.ext;

if (this.root instanceof Array) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use Array.isArray, please :)

@dougwilson
Copy link
Contributor

In general I think this is useful.

@rlidwka
Copy link
Member

rlidwka commented Aug 26, 2014

I believe it should be directed to expressjs/templation instead, express should eventually use that anyway.

Also it was discussed before (#672, #1667), etc.

@Fishrock123
Copy link
Contributor

express should eventually use that anyway.

Not necessarily..

@dougwilson
Copy link
Contributor

We will accept the PR here and land it where ever it needs to go later. templation is way further down a road map and is not even guaranteed to be anything related to express; it was just thrown out there as a possibility.

@Fishrock123 Fishrock123 added pr and removed Views labels Sep 29, 2014
@dougwilson dougwilson mentioned this pull request Oct 18, 2014
2 tasks
@dougwilson
Copy link
Contributor

This PR still needs to be refactored to remove that fake context thing. I need to get 4.10 out ASAP, so if I don't have time to refactor this tomorrow (and I guess no one updates this PR to address it), then I'll just push it back to 4.11.

@jescalan
Copy link

jescalan commented Dec 4, 2014

Thanks for getting this merged, this feature just saved my life 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants