ericf / express-handlebars

A Handlebars view engine for Express which doesn't suck.
BSD 3-Clause "New" or "Revised" License
2.31k stars 384 forks source link

extname option doesn't work #75

Closed knownasilya closed 10 years ago

knownasilya commented 10 years ago

The example works here: https://github.com/ericf/express-handlebars#extnamehandlebars

// doesn't work
app.engine('handlebars', exphbs({extname: '.hbs'}));
app.set('view engine', 'handlebars');

// works
app.engine('.hbs', exphbs()); // extname = '.handlebars'
app.set('view engine', '.hbs');

Seems like the extname option doesn't actually do anything, but it's the name of the engine..

ericf commented 10 years ago

You should make sure they are all the same value in the three places. This package uses its extname options for its Layout feature.

knownasilya commented 10 years ago

@ericf can that be specified in the readme (it's a little vague, since it's not mentioned that the engine name has to match).

Also why can the engine name be handlebars when the extname is .handlebars?

knownasilya commented 10 years ago

Is there no way to make the extname be the single-source of extension truth?

Edit:

So according to http://expressjs.com/api#app.engine, the name is the extension. So maybe extname should be renamed to something more clear, and add a note on actually changing the ext for handlebars and a note about what the layout ext thing is.

ericf commented 10 years ago

If you look at the signature of app.engine() again, it takes ext as the first parameter. Feel free to open a PR adding a note that all three values should match.