Skip to content

Commit

Permalink
Removing functionnality allowing env to pass a template engine
Browse files Browse the repository at this point in the history 
Rationale: Template engine are only related to the generator itself. The
environment shouldn't have any control on it (it just doesn't make
sense)
  • Loading branch information
@SBoudrias
SBoudrias committed Jan 6, 2015
1 parent 0d4cf09 commit 566ab33
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 56 deletions.
12 changes: 3 additions & 9 deletions lib/actions/actions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var mkdirp = require('mkdirp');
var isBinaryFile = require('isbinaryfile');
var chalk = require('chalk');
var xdgBasedir = require('xdg-basedir');

var engine = require('../util/engines').underscore;
/**
* @mixin
* @alias actions/actions
Expand Down Expand Up @@ -212,14 +212,8 @@ actions.template = function template(source, destination, data, options) {
* @param {Object} options
*/

actions.engine = function engine(body, data, options) {
if (!this._engine) {
throw new Error('Trying to render template without valid engine.');
}

return this._engine.detect && this._engine.detect(body) ?
this._engine(body, data, options) :
body;
actions.engine = function (body, data, options) {
return engine.detect(body) ? engine(body, data, options) : body;
};

// Shared directory method
Expand Down
12 changes: 0 additions & 12 deletions lib/base.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ var userHome = require('user-home');
var GruntfileEditor = require('gruntfile-editor');
var FileEditor = require('mem-fs-editor');

var engines = require('./util/engines');
var Conflicter = require('./util/conflicter');
var Storage = require('./util/storage');
var promptSuggestion = require('./util/prompt-suggestion');
Expand Down Expand Up @@ -108,17 +107,6 @@ var Base = module.exports = function Base(args, options) {
return function () {};
};

_.defaults(this.options, {
engine: engines.underscore
});

this._engine = this.options.engine;

// cleanup options hash from default engine, if users didn't provided one.
if (!options.engine) {
delete this.options.engine;
}

this.conflicter = new Conflicter(this.env.adapter, this.options.force);

// Mirror the adapter log method on the generator.
Expand Down
35 changes: 0 additions & 35 deletions test/actions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,41 +250,6 @@ describe('generators.Base (actions/actions)', function () {
assert.textEqual(body, '<version>bar</version> <%= foo %>;\n');
});
});

describe('with custom tags', function () {
beforeEach(function (done) {
this.src = 'custom-template-setting.xml';
this.dest = 'write/to/custom-template-setting.xml';
this.spy = sinon.spy();

var oldEngineOptions = this.dummy.options.engine.options;

this.dummy.options.engine.options = {
detecter: /\{\{?[^\}]+\}\}/,
matcher: /\{\{\{([^\}]+)\}\}/g,
start: '{{',
end: '}}'
};

this.dummy.template(this.src, this.dest, {
foo: 'bar',
spy: this.spy
}, {
evaluate: /\{\{([\s\S]+?)\}\}/g,
interpolate: /\{\{=([\s\S]+?)\}\}/g,
escape: /\{\{-([\s\S]+?)\}\}/g
});

this.dummy.options.engine.options = oldEngineOptions;
this.dummy._writeFiles(done);
});

it('uses tags specified in option and engine', function () {
var body = fs.readFileSync(this.dest, 'utf8');
assert.textEqual(body, '<version>bar</version>\n');
sinon.assert.calledOnce(this.spy);
});
});
});

describe('#directory()', function () {
Expand Down

4 comments on commit 566ab33

@hurrymaplelad
Copy link
Contributor

Choose a reason for hiding this comment

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

@SBoudrias is there a way to configure the engine from a generator? I don't see how to support a handlebars-engine with v0.18.

@hurrymaplelad
Copy link
Contributor

Choose a reason for hiding this comment

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

This mostly works:

yeoman.generators.Base.extend({
      engine: require('yeoman-handlebars-engine')()
      // ...
})

But each engine would need to wire up its own engine.detect check. Recommended for now?

@SBoudrias
Copy link
Member Author

Choose a reason for hiding this comment

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

First of all: you never could exactly define a different engine. It never worked (maybe under some circumstances, but in most cases it would just broke). This commit only removed code that wasn't working correctly.

Second, here's my take on it. Generators are usually fairly simple. They're not complex web apps. As so, I hardly see the need to define a different templating engine. Lodash templates are not great, but they're good enough for our use cases (and the most flexible).

On the practical side, you can define custom delimiters on the template methods. So you can prevent conflict with the template delimiters symbols.

And on the other side, we're moving away from the monolithics utilities, and we're moving away from this.template and this.copy, etc (these legacy methods are the only ones still using this.engine, and they'll stop doing so in a planned future release - see our deprecation plan). You can learn about our new file utilities on the version 0.18 release notes.

Hope this helps understanding where we're going with yeoman-generator. Feel free to open a new issue to discuss re-allowing custom engines if you feel strongly about this feature.

@hurrymaplelad
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to know. Thanks for clearing things up. I don't feel strongly about custom engines. Glad to hear the core is getting smaller 🌞

Please sign in to comment.