Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

`_.template` doesn't process delimiters correctly that contain newlines. #779

Closed
jdalton opened this Issue · 8 comments

3 participants

@jdalton
Collaborator

Setting a delimiter like

_.templateSettings = {
    'evaluate': /<<(.+?)>>/g
};

incorrectly matches the template below

<script type="text/template" id="template">
    <<
        print('<p>\"item\"' + (added ? ' added' : '') + '</p>');
    >>
</script>

even though

$('#template').html().match(_.templateSettings.evaluate); // is null

Notice the "evaluate" regexp doesn't match newlines so shouldn't match the snippet.

See: http://jsbin.com/ajopar/1/edit

@braddunbar
Collaborator

Hey @jdalton! I see what you mean, but I can only think of contrived cases in which this would be a problem. I suppose you're talking about instances in which you might want to include the template delimiters in the template as is?

I think it's reasonable to expect the user to be mindful of their delimiters and use a separate construct appropriately.

@jdalton
Collaborator

It actually popped up as an invalid Lo-Dash bug report. A user had that in their code. The bug is that Underscore escapes the entire string before parsing out the delimiters.

@braddunbar
Collaborator

Thanks for the link to the issue @jdalton! However, after reading through it, it appears that the user was expecting the current behavior (ignoring newlines). Further, this would mean a breaking change for any code using (.*?). I think it's best to continue matching user expectation in this case.

@braddunbar braddunbar closed this
@jdalton
Collaborator

It's clearly a bug in Underscore. Other contributors have advocated using correct delimiters, but if Underscore doesn't respect the delimiter then what's the point?

@jdalton
Collaborator

If you go back to Underscore v1.3.1 you will see it behave correctly. This is a regression.

Further, this would mean a breaking change for any code using (.*?).

Underscore has already implemented several breaking changes, however this is a bug fix.

@michaelficarra
Collaborator

Agreed with @jdalton: this is a bug.

@braddunbar
Collaborator

I didn't realize that it was a regression and behaved differently in v1.3.1. Thanks for pointing it out. :)

@braddunbar braddunbar reopened this
@braddunbar
Collaborator

Addressed in #782.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.