fix($interpolate): always unescape escaped interpolation markers by gkalpak · Pull Request #14199 · angular/angular.js · GitHub
Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($interpolate): always unescape escaped interpolation markers#14199

Open
gkalpak wants to merge 1 commit intoangular:masterfrom
gkalpak:fix-interpolate-always-unescape-escaped-markers
Open

fix($interpolate): always unescape escaped interpolation markers#14199
gkalpak wants to merge 1 commit intoangular:masterfrom
gkalpak:fix-interpolate-always-unescape-escaped-markers

Conversation

@gkalpak
Copy link
Copy Markdown
Member

@gkalpak gkalpak commented Mar 8, 2016

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

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 angular#14196
@gkalpak
Copy link
Copy Markdown
Member Author

gkalpak commented Mar 8, 2016

@lgalfaso
Copy link
Copy Markdown
Contributor

lgalfaso commented Mar 9, 2016

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
Copy link
Copy Markdown
Member Author

gkalpak commented Mar 9, 2016

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
Copy link
Copy Markdown
Contributor

lgalfaso commented Mar 9, 2016

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

@petebacondarwin
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Contributor

lgalfaso commented Mar 9, 2016

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

@petebacondarwin
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Contributor

lgalfaso commented Mar 9, 2016

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
Copy link
Copy Markdown
Contributor

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Escaped interpolation markers are not unescaped if there are no interpolated expressions in the same text

4 participants