refactor(compiler): introduce `InterpolationConfig` by lacolaco · Pull Request #8906 · angular/angular

lacolaco · 2016-05-29T16:40:18Z

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit-message-format
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior?

Does this PR introduce a breaking change?

Yes
No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

CompilerConfig takes interpolation arguments as InterpolationConfig type.

Other information:

ref. #8865

CompilerConfig has a configuration of interpolation for the parser.
InterpolationConfig has two property; startSymbol and endSymbol.

InterpolationRegexp of the parser will be created with its symbols.

lacolaco · 2016-05-29T16:45:08Z

@vicb I introduced InterpolationConfig as the first step to support i18n parser. Please let me know your opinion on this.

cc/ @mhevery

vicb · 2016-05-31T16:51:53Z

modules/@angular/compiler/src/config.ts

vicb · 2016-05-31T17:00:08Z

@laco0416 LGTM with some minor comments.

Do you plan to amend this PR with i18n / submit a new one ?

lacolaco · 2016-05-31T21:48:51Z

@vicb yes. I'll include i18n paraer changes.
2016/06/01 2:00 "Victor Berchet" [email protected]:

@laco0416 https://github.com/laco0416 LGTM with some minor comments.

Do you plan to amend this PR with i18n / submit a new one ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8906 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABdVXDvUrmUV0vEFysGA1SFwKafonL8sks5qHGkqgaJpZM4IpU61
.

lacolaco · 2016-05-31T22:12:24Z

Introduce InterpolationConfig to Parser
Clean up
Apply this to I18nHTMLParser

lacolaco · 2016-05-31T22:35:42Z

@vicb It's done. Please review again! :)

lacolaco · 2016-05-31T22:43:16Z

Travis red is in saucelabs flaky test.

vicb · 2016-06-01T00:59:43Z

modules/@angular/compiler/src/config.ts

Dowe need to export, if not you'd better create the obj lazily

Totally. :P

lacolaco · 2016-06-01T03:33:48Z

modules/@angular/compiler/src/config.ts

Can I use const here?

vicb · 2016-06-01T16:48:33Z

@laco0416 thanks for your work. I have added a question.

Could you please squash your 2 commits & rebase.

vicb · 2016-06-01T20:55:04Z

thinking more about this, the symbols could not be part of the compiler config but must be part of the @Component to be able to mix & match different symbols.

lacolaco · 2016-06-01T22:58:44Z

@vicb

thinking more about this, the symbols could not be part of the compiler config but must be part of the @Component to be able to mix & match different symbols.

So, the compiler has {{}} as a default symbols, and each component has own symbols for override behavior of the compiler... This approach is similar to encapsulation, right?

`CompilerConfig` has a configuration of interpolation for Parser and I18nHTMLParser. `InterpolationConfig` has two properties; `startSymbol` and `endSymbol`. InterpolationRegexp of the parser will be created with its symbols.

lacolaco · 2016-06-01T23:39:42Z

For that, we must modify the compiler system entirely. I think that is out of this PR... ;(

vicb · 2016-06-02T00:15:44Z

For that, we must modify the compiler system entirely. I think that is out of this PR... ;(

Either we should revert the partial impl that is currently in master or do the full support (component based).

Would you like to work on that ? We should have the feature soon if we want it in 2.0.0. However it could come in a further release.

lacolaco · 2016-06-02T00:19:40Z

Ok, let's revert current impl. you can?
This is not needed as final, I think too.

vicb · 2016-06-02T00:24:12Z

Yes I'll do that.

lacolaco · 2016-06-02T00:29:52Z

Thank you. I close here and create new PR as a feature commit.

angular-automatic-lock-bot · 2019-09-13T19:22:33Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label May 29, 2016

lacolaco force-pushed the laco-parser-config-interpolate branch 3 times, most recently from c9749d4 to 0ad55cf Compare May 29, 2016 16:56

vicb reviewed May 31, 2016
View reviewed changes

modules/@angular/compiler/src/config.ts Outdated

Copy link
Copy Markdown

Contributor

vicb May 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEFAULT

vicb added pr_state: LGTM action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels May 31, 2016

lacolaco changed the title ~~refactor(compiler): introduce InterpolationConfig~~ [WIP] refactor(compiler): introduce InterpolationConfig May 31, 2016

lacolaco force-pushed the laco-parser-config-interpolate branch from 0ad55cf to d53e98e Compare May 31, 2016 22:10

lacolaco force-pushed the laco-parser-config-interpolate branch from d53e98e to 191e2e1 Compare May 31, 2016 22:29

lacolaco changed the title ~~[WIP] refactor(compiler): introduce InterpolationConfig~~ refactor(compiler): introduce InterpolationConfig May 31, 2016

vicb reviewed Jun 1, 2016
View reviewed changes

lacolaco force-pushed the laco-parser-config-interpolate branch from cf6a0c1 to 527d1ff Compare June 1, 2016 01:47

lacolaco mentioned this pull request Jun 1, 2016

CompilerConfig should take options as named params #8933

Closed

1 task

lacolaco reviewed Jun 1, 2016
View reviewed changes

refactor(compiler): introduce InterpolationConfig

3eb2748

`CompilerConfig` has a configuration of interpolation for Parser and I18nHTMLParser. `InterpolationConfig` has two properties; `startSymbol` and `endSymbol`. InterpolationRegexp of the parser will be created with its symbols.

lacolaco force-pushed the laco-parser-config-interpolate branch from 527d1ff to 3eb2748 Compare June 1, 2016 23:13

lacolaco closed this Jun 2, 2016

lacolaco mentioned this pull request Jun 11, 2016

feat(compiler): Introduce InterpolationConfig into component #9158

Closed

5 tasks

lacolaco deleted the laco-parser-config-interpolate branch July 7, 2018 06:43

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Conversation

lacolaco commented May 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lacolaco commented May 29, 2016

Uh oh!

vicb May 31, 2016

Choose a reason for hiding this comment

Uh oh!

vicb commented May 31, 2016

Uh oh!

lacolaco commented May 31, 2016

Uh oh!

lacolaco commented May 31, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lacolaco commented May 31, 2016

Uh oh!

lacolaco commented May 31, 2016

Uh oh!

vicb Jun 1, 2016

Choose a reason for hiding this comment

Uh oh!

lacolaco Jun 1, 2016

Choose a reason for hiding this comment

Uh oh!

lacolaco Jun 1, 2016

Choose a reason for hiding this comment

Uh oh!

vicb Jun 1, 2016

Choose a reason for hiding this comment

Uh oh!

vicb commented Jun 1, 2016

Uh oh!

vicb commented Jun 1, 2016

Uh oh!

lacolaco commented Jun 1, 2016

Uh oh!

lacolaco commented Jun 1, 2016

Uh oh!

vicb commented Jun 2, 2016

Uh oh!

lacolaco commented Jun 2, 2016

Uh oh!

vicb commented Jun 2, 2016

Uh oh!

lacolaco commented Jun 2, 2016

Uh oh!

angular-automatic-lock-bot bot commented Sep 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lacolaco commented May 29, 2016 •

edited

Loading

lacolaco commented May 31, 2016 •

edited

Loading