Skip to content

One last addition...#562

Merged
jashkenas merged 1 commit into
jashkenas:masterfrom
colinta:master
Apr 19, 2012
Merged

One last addition...#562
jashkenas merged 1 commit into
jashkenas:masterfrom
colinta:master

Conversation

@colinta

@colinta colinta commented Apr 19, 2012

Copy link
Copy Markdown
Contributor

This uses braddunbar's __t= solution to have <%- foo %> work the same as <%= foo %>. Tests updated.

@jashkenas

Copy link
Copy Markdown
Owner

@braddunbar -- is this patch necessary? I thought escape already handled this perhaps?

@braddunbar

Copy link
Copy Markdown
Collaborator

No, escape does not handle this. It's handled in Backbone.Model#escape but _.escape still returns 'null'. If _.escape was updated to return '' for null values this would be a non-issue. Is there a reason that _.escape should continue to return 'null' and 'undefined' for null values (backwards compatibility may be a good enough reason)?

+1 - I think it's reasonable to expect that null values are handled consistently in escape and interpolate.

@braddunbar

Copy link
Copy Markdown
Collaborator

FYI, this was discussed in jashkenas/backbone#1227 and it was decided that Backbone.Model#escape should continue to return the empty string.

@braddunbar

Copy link
Copy Markdown
Collaborator

For what it's worth this was also inconsistent in 1.3.1.

brad:~/dev/test$ npm install underscore@1.3.1
npm http GET https://registry.npmjs.org/underscore/1.3.1
npm http 200 https://registry.npmjs.org/underscore/1.3.1
npm http GET https://registry.npmjs.org/underscore/-/underscore-1.3.1.tgz
npm http 200 https://registry.npmjs.org/underscore/-/underscore-1.3.1.tgz
underscore@1.3.1 ./node_modules/underscore
brad:~/dev/test$ node
> require('underscore').template('<%-x%>', {x: null});
'null'

@jashkenas

Copy link
Copy Markdown
Owner

FWIW -- it was purposeful. Only strings have a defined "escaped" value. Escaping other objects isn't supported. ... but we can do whatever here.

jashkenas added a commit that referenced this pull request Apr 19, 2012
@jashkenas jashkenas merged commit f10180b into jashkenas:master Apr 19, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants