fix($interpolate): always unescape escaped interpolation markers

[gkalpak]

Previously, whenever mustHaveExpression was true (e.g. when compiling a text node),
$interpolate would not unescape the escaped interpolation markers if there were no actual
interpolation expressions in the same string.
This commit fixes the problem, by always unescaping the escaped markers (if any).

Due to always checking for the presence of escaped interpolation markers, there is a slight
performance hit.

Fixes #14196

[gkalpak]

I wonder if this is a BC.

Possible breakages:

1. Even if there are no actual interpolation expressions, interpolating strings with escaped markers will return an interpolation function. Previously it would return undefined (and not unescape the escaped markers).
2. If you relied on the current (documented!) behavior, of not unescaping escaped markers on strings with no actual interpolation expressions.

[lgalfaso]

This PR brings a new level of complexity when people want to have the
literal of value "\{\}" as you would need to write \\{\\{, this is \
should be escaped but only if it would get confused with an interpolation.
This is, it is not possible to write a context free function that takes one
char at a time and returns the escaped char.

This is, we should unescape everything (not only \{) or keep the current
behavior. The former would be a BC.

On Tuesday, March 8, 2016, Georgios Kalpakas notifications@github.com
wrote:

I wonder if this is a BC.

Possible breakages:

1. Even if there are no actual interpolation expressions,
   interpolating strings with escaped markers will return an interpolation
   function. Previously it would return undefined (and not unescape the
   escaped markers).
2. If you relied on the current (documented!) behavior, of not
   unescaping escaped markers on strings with no actual interpolation
   expressions.

—
Reply to this email directly or view it on GitHub
#14199 (comment).

[gkalpak]

Maybe I didn't understand well, but with the current behavior, \{\{ might or might not be unescaped depending on whether an actual intetpolation exists in the same string. Doesn't it mean that a context free function won't work now either ?

[lgalfaso]

@gkalpak agree, but this PR moves the issue from one pattern to another :)

[petebacondarwin]

I think this looks good to me. The issue that @lgalfaso points out is already a problem for text blocks that contain interpolation expressions anyway. This just brings consistency between blocks that do and do not contain interpolation expressions.

[petebacondarwin]

Breaking change? Well at the moment I would argue that the escaping is broken, since it is inconsistent and I doubt that anyone is really relying on these escaped sequences not being unescaped in text blocks that do not contain interpolation expressions.

[lgalfaso]

@petebacondarwin if this PR lands, how would you write \{\{?

[petebacondarwin]

I am not sure. But however you do it after this PR it will be the same as you would do it before the PR, in the case that there was some interpolated expression in the same text block.

One ugly way would be to do...

{{ "\\{\\{" }}

[lgalfaso]

I think everybody agrees that the current is less than ideal. I would not
call it broken, but find it normal that others do think it is broken. Now,
this is what I find strange. We know that there are some limitation and
this PR removes some of them, but at the same time it adds new limitations
that worked fine before.

On Wednesday, March 9, 2016, Pete Bacon Darwin notifications@github.com
wrote:

I am not sure. But however you do it after this PR it will be the same as
you would do it before the PR, in the case that there was some interpolated
expression in the same text block.

One ugly way would be to do...

{{ "{{" }}

—
Reply to this email directly or view it on GitHub
#14199 (comment).

[petebacondarwin]

The new limitation that worked before was that this PR fixes the inconsistency that if a text node did not contain any interpolations then we would not try to unescape our publicly documented escape sequence for curly braces.

I would say that this was such a weird inconsistency that no one could have been using the escaping effectively, since their server-side escaping mechanism must have been able to ascertain whether a set of double curlies, which were supposed to be rendered as-is, appeared in a text block that actually contained a set of double curlies, which were supposed to be interpolated.

I agree that we need a clear way of escaping the escape sequence.