Skip to content

Commit

Permalink
Add rmWhitespace mode
Browse files Browse the repository at this point in the history 
It is not a full-fledged HTML minifier, nor will it be. It is designed to
be as "safe" as possible.

Fixes #51.
  • Loading branch information
@TimothyGu
TimothyGu committed Feb 1, 2015
1 parent dc842ec commit a7d733f
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 1 deletion.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ for all the passed options.
- `delimiter` Character to use with angle brackets for open/close
- `debug` Output generated function body
- `_with` Whether or not to use `with() {}` constructs. If `false` then the locals will be stored in the `locals` object.
- `rmWhitespace` Remove all safe-to-remove whitespace, including `\r`, leading and trailing whitespace. This also enables `-%>` new line slurping for all scriptlet tags.

## Tags

Expand Down
12 changes: 11 additions & 1 deletion lib/ejs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var fs = require('fs')
, _TRAILING_SEMCOL = /;\s*$/;

exports.localsName = _DEFAULT_LOCALS_NAME;
exports.rmWhitespace = false;

function resolveInclude(name, filename) {
var path = require('path')
Expand Down Expand Up @@ -223,6 +224,9 @@ function Template(text, opts) {
options.delimiter = opts.delimiter || exports.delimiter || _DEFAULT_DELIMITER;
options._with = typeof opts._with != 'undefined' ? opts._with : true;
options.cache = opts.cache || false;
options.rmWhitespace = opts.rmWhitespace !== undefined

This comment has been minimized.

Sign in to view

Copy link
@mde

mde Feb 6, 2015

Owner

This is generally okay, but I prefer the typeof check (e.g. with _with, above) to see if an option or var has been set. Given all these settings, a global config might have been nicer than hanging all this stuff right on the export. This bunch of massaging of values might be better as its own function too, rather than just inlined here.

This comment has been minimized.

Sign in to view

Copy link
@TimothyGu

TimothyGu Feb 6, 2015

Author Collaborator

This is generally okay, but I prefer the typeof check (e.g. with _with, above) to see if an option or var has been set.

I'm fine with this, but is there any technical reasons for doing so?

Given all these settings, a global config might have been nicer than hanging all this stuff right on the export.

True. EJS v3? ejs.options? See also #43.

This comment has been minimized.

Sign in to view

Copy link
@mde

mde Feb 6, 2015

Owner

The technical reason is because this allows you to do the same sort of check for undeclared vars:

typeof foo == 'undefined'; // true
foo === undefined; // Throws

And of course doing all your "is this variable/property set" checks with the same pattern makes it easy to recognize it as such when you're scanning code.

This comment has been minimized.

Sign in to view

Copy link
@mde

mde Feb 6, 2015

Owner

Oh, and yes, good call, #43. Very similar to the jQuery.ajax API.

This comment has been minimized.

Sign in to view

Copy link
@TimothyGu

TimothyGu Feb 6, 2015

Author Collaborator

opts.rmWhitespace will never throw an error as opts is always defined, and a nonexistent property always returns undefined.

var opts = {}
opts.afdafdsadsfdsfqfjeuiewirf // undefined
opts.afdafdsadsfdsfqfjeuiewirf === undefined // true

This comment has been minimized.

Sign in to view

Copy link
@mde

mde Feb 6, 2015

Owner

I didn't say it would throw on a property access. :) I said there's value in using the same pattern for unset checks everywhere, and it will throw on undeclared vars.
? opts.rmWhitespace
: exports.rmWhitespace;
this.opts = options;

this.regex = this.createRegex();
Expand Down Expand Up @@ -250,6 +254,9 @@ Template.prototype = new function () {
, opts = this.opts
, escape = opts.escapeFunction;

if (opts.rmWhitespace) {
this.templateText = this.templateText.replace(/^\s+|\s+$|\r/gm, '');
}
if (!this.source) {
this.generateSource();
var prepended = 'var __output = "";';
Expand Down Expand Up @@ -392,6 +399,9 @@ Template.prototype = new function () {
function _addOutput() {
if (self.truncate) {
line = line.replace('\n', '');
if (!line) {
return;
}
}

// Preserve literal slashes
Expand Down Expand Up @@ -433,7 +443,7 @@ Template.prototype = new function () {
}

this.mode = null;
this.truncate = line.indexOf('-') === 0;
this.truncate = line.indexOf('-') === 0 || self.opts.rmWhitespace;
break;
default:
// In script mode, depends on type of tag
Expand Down
12 changes: 12 additions & 0 deletions test/ejs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,18 @@ suite('exceptions', function () {
});
});

suite('rmWhitespace', function () {
test('works', function () {
assert.equal(ejs.render(fixture('rmWhitespace.ejs'), {}, {rmWhitespace: true}),
fixture('rmWhitespace.html'));
});

test('works without semicolons', function () {
assert.equal(ejs.render(fixture('no.semicolons.ejs'), {}, {rmWhitespace: true}),
fixture('no.semicolons.rmWhitespace.html'));
});
});

suite('include()', function () {
test('include ejs', function () {
var file = 'test/fixtures/include-simple.ejs';
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/no.semicolons.rmWhitespace.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
This document does not use semicolons in scriptlets.
The value of c is: c
11 changes: 11 additions & 0 deletions test/fixtures/rmWhitespace.ejs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<tag1>
<tag2>
A very long piece of text very long piece of text very long piece of text very
long piece of text very long piece of text very long piece of text very long
piece of text.
<% var a = 'a' %>
Text again.
<% var b = 'b' %>
<% var c = 'c'
var d = 'd' %>
Another text.
7 changes: 7 additions & 0 deletions test/fixtures/rmWhitespace.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<tag1>
<tag2>
A very long piece of text very long piece of text very long piece of text very
long piece of text very long piece of text very long piece of text very long
piece of text.
Text again.
Another text.

0 comments on commit a7d733f

Please sign in to comment.