From 4028fcaa513453da806c4b3882b921193fe569c6 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Thu, 4 Aug 2016 17:46:31 -0700 Subject: [PATCH] refactor(i18n): Drop `html.Message` and create `i18n.Message` right away --- .../compiler/src/i18n/extractor_merger.ts | 66 ++++---- .../@angular/compiler/src/i18n/i18n_parser.ts | 25 +-- .../compiler/src/i18n/message_bundle.ts | 11 +- .../test/i18n/extractor_merger_spec.ts | 145 ++++++++++-------- .../compiler/test/i18n/i18n_parser_spec.ts | 7 +- 5 files changed, 127 insertions(+), 127 deletions(-) diff --git a/modules/@angular/compiler/src/i18n/extractor_merger.ts b/modules/@angular/compiler/src/i18n/extractor_merger.ts index ab298ef77e..1795104b63 100644 --- a/modules/@angular/compiler/src/i18n/extractor_merger.ts +++ b/modules/@angular/compiler/src/i18n/extractor_merger.ts @@ -9,7 +9,7 @@ import * as html from '../ml_parser/ast'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; -import {Message as I18nMessage} from './i18n_ast'; +import * as i18n from './i18n_ast'; import * as i18nParser from './i18n_parser'; import * as msgBundle from './message_bundle'; import {I18nError} from './parse_util'; @@ -20,13 +20,13 @@ const _I18N_ATTR_PREFIX = 'i18n-'; const _I18N_COMMENT_PREFIX_REGEXP = /^i18n:?/; /** - * Extract translatable messages from an html AST as a list of html AST nodes + * Extract translatable messages from an html AST */ -export function extractAstMessages( - nodes: html.Node[], implicitTags: string[], +export function extractMessages( + nodes: html.Node[], interpolationConfig: InterpolationConfig, implicitTags: string[], implicitAttrs: {[k: string]: string[]}): ExtractionResult { const visitor = new _Visitor(implicitTags, implicitAttrs); - return visitor.extract(nodes); + return visitor.extract(nodes, interpolationConfig); } export function mergeTranslations( @@ -37,7 +37,7 @@ export function mergeTranslations( } export class ExtractionResult { - constructor(public messages: Message[], public errors: I18nError[]) {} + constructor(public messages: i18n.Message[], public errors: I18nError[]) {} } enum _VisitorMode { @@ -71,11 +71,12 @@ class _Visitor implements html.Visitor { private _mode: _VisitorMode; // _VisitorMode.Extract only - private _messages: Message[]; + private _messages: i18n.Message[]; // _VisitorMode.Merge only private _translations: TranslationBundle; - private _convertHtmlToI18n: (html: Message) => I18nMessage; + private _createI18nMessage: + (msg: html.Node[], meaning: string, description: string) => i18n.Message; constructor(private _implicitTags: string[], private _implicitAttrs: {[k: string]: string[]}) {} @@ -83,8 +84,8 @@ class _Visitor implements html.Visitor { /** * Extracts the messages from the tree */ - extract(nodes: html.Node[]): ExtractionResult { - this._init(_VisitorMode.Extract); + extract(nodes: html.Node[], interpolationConfig: InterpolationConfig): ExtractionResult { + this._init(_VisitorMode.Extract, interpolationConfig); nodes.forEach(node => node.visit(this, null)); @@ -101,8 +102,7 @@ class _Visitor implements html.Visitor { merge( nodes: html.Node[], translations: TranslationBundle, interpolationConfig: InterpolationConfig): html.Node[] { - this._init(_VisitorMode.Merge); - this._convertHtmlToI18n = i18nParser.getHtmlToI18nConverter(interpolationConfig); + this._init(_VisitorMode.Merge, interpolationConfig); this._translations = translations; // Construct a single fake root element @@ -136,7 +136,7 @@ class _Visitor implements html.Visitor { if (!this._inIcu) { // nested ICU messages should not be extracted but top-level translated as a whole if (this._isInTranslatableSection) { - this._addMessage(context, [icu]); + this._addMessage([icu]); } this._inIcu = true; } @@ -182,8 +182,7 @@ class _Visitor implements html.Visitor { if (this._depth == this._blockStartDepth) { this._closeTranslatableSection(comment, this._blockChildren); this._inI18nBlock = false; - const message = - this._addMessage(context, this._blockChildren, this._blockMeaningAndDesc); + const message = this._addMessage(this._blockChildren, this._blockMeaningAndDesc); return this._translateMessage(comment, message); } else { this._reportError(comment, 'I18N blocks should not cross element boundaries'); @@ -216,12 +215,12 @@ class _Visitor implements html.Visitor { if (i18nAttr) { // explicit translation this._inI18nNode = true; - const message = this._addMessage(context, el.children, i18nAttr.value); + const message = this._addMessage(el.children, i18nAttr.value); childNodes = this._translateMessage(el, message); } else if (isImplicitI18n) { // implicit translation this._inI18nNode = true; - const message = this._addMessage(context, el.children); + const message = this._addMessage(el.children); childNodes = this._translateMessage(el, message); } @@ -273,7 +272,7 @@ class _Visitor implements html.Visitor { } } - this._visitAttributesOf(el, context); + this._visitAttributesOf(el); this._depth--; this._inI18nNode = wasInI18nNode; @@ -291,7 +290,7 @@ class _Visitor implements html.Visitor { throw new Error('unreachable code'); } - private _init(mode: _VisitorMode): void { + private _init(mode: _VisitorMode, interpolationConfig: InterpolationConfig): void { this._mode = mode; this._inI18nBlock = false; this._inI18nNode = false; @@ -300,10 +299,11 @@ class _Visitor implements html.Visitor { this._msgCountAtSectionStart = void 0; this._errors = []; this._messages = []; + this._createI18nMessage = i18nParser.createI18nMessageFactory(interpolationConfig); } // looks for translatable attributes - private _visitAttributesOf(el: html.Element, context: any): void { + private _visitAttributesOf(el: html.Element): void { const explicitAttrNameToValue: {[k: string]: string} = {}; const implicitAttrNames: string[] = this._implicitAttrs[el.name] || []; @@ -314,15 +314,15 @@ class _Visitor implements html.Visitor { el.attrs.forEach(attr => { if (attr.name in explicitAttrNameToValue) { - this._addMessage(context, [attr], explicitAttrNameToValue[attr.name]); + this._addMessage([attr], explicitAttrNameToValue[attr.name]); } else if (implicitAttrNames.some(name => attr.name === name)) { - this._addMessage(context, [attr]); + this._addMessage([attr]); } }); } // add a translatable message - private _addMessage(context: any, ast: html.Node[], meaningAndDesc?: string): Message { + private _addMessage(ast: html.Node[], meaningAndDesc?: string): i18n.Message { if (ast.length == 0 || ast.length == 1 && ast[0] instanceof html.Attribute && !(ast[0]).value) { // Do not create empty messages @@ -330,16 +330,15 @@ class _Visitor implements html.Visitor { } const [meaning, description] = _splitMeaningAndDesc(meaningAndDesc); - const message = new Message(ast, meaning, description); + const message = this._createI18nMessage(ast, meaning, description); this._messages.push(message); return message; } // translate the given message given the `TranslationBundle` - private _translateMessage(el: html.Node, message: Message): html.Node[] { + private _translateMessage(el: html.Node, message: i18n.Message): html.Node[] { if (message && this._mode === _VisitorMode.Merge) { - const i18nMessage: I18nMessage = this._convertHtmlToI18n(message); - const id = msgBundle.digestMessage(i18nMessage.nodes, i18nMessage.meaning); + const id = msgBundle.digestMessage(message.nodes, message.meaning); const nodes = this._translations.get(id); if (nodes) { @@ -374,8 +373,8 @@ class _Visitor implements html.Visitor { if (i18nAttributeMeanings.hasOwnProperty(attr.name)) { const meaning = i18nAttributeMeanings[attr.name]; - const i18nMessage: I18nMessage = this._convertHtmlToI18n(new Message([attr], meaning, '')); - const id = msgBundle.digestMessage(i18nMessage.nodes, i18nMessage.meaning); + const message: i18n.Message = this._createI18nMessage([attr], meaning, ''); + const id = msgBundle.digestMessage(message.nodes, message.meaning); const nodes = this._translations.get(id); if (!nodes) { this._reportError( @@ -456,7 +455,7 @@ class _Visitor implements html.Visitor { if (significantChildren == 1) { for (let i = this._messages.length - 1; i >= startIndex; i--) { const ast = this._messages[i].nodes; - if (!(ast.length == 1 && ast[0] instanceof html.Attribute)) { + if (!(ast.length == 1 && ast[0] instanceof i18n.Text)) { this._messages.splice(i, 1); break; } @@ -471,13 +470,6 @@ class _Visitor implements html.Visitor { } } -/** - * A Message contain a fragment (= a subtree) of the source html AST. - */ -export class Message { - constructor(public nodes: html.Node[], public meaning: string, public description: string) {} -} - function _isOpeningComment(n: html.Node): boolean { return n instanceof html.Comment && n.value && n.value.startsWith('i18n'); } diff --git a/modules/@angular/compiler/src/i18n/i18n_parser.ts b/modules/@angular/compiler/src/i18n/i18n_parser.ts index 8eccc15623..7d58dac71b 100644 --- a/modules/@angular/compiler/src/i18n/i18n_parser.ts +++ b/modules/@angular/compiler/src/i18n/i18n_parser.ts @@ -13,37 +13,20 @@ import {getHtmlTagDefinition} from '../ml_parser/html_tags'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseSourceSpan} from '../parse_util'; -import * as extractor from './extractor_merger'; import * as i18n from './i18n_ast'; import {PlaceholderRegistry} from './serializers/placeholder'; const _expParser = new ExpressionParser(new ExpressionLexer()); -/** - * Extract all the i18n messages from a component template. - */ -export function extractI18nMessages( - sourceAst: html.Node[], interpolationConfig: InterpolationConfig, implicitTags: string[], - implicitAttrs: {[k: string]: string[]}): i18n.Message[] { - const extractionResult = extractor.extractAstMessages(sourceAst, implicitTags, implicitAttrs); - - if (extractionResult.errors.length) { - return []; - } - - const htmlToI18n = getHtmlToI18nConverter(interpolationConfig); - - return extractionResult.messages.map(htmlToI18n); -} - /** * Returns a function converting html Messages to i18n Messages given an interpolationConfig */ -export function getHtmlToI18nConverter(interpolationConfig: InterpolationConfig): - (msg: extractor.Message) => i18n.Message { +export function createI18nMessageFactory(interpolationConfig: InterpolationConfig): ( + nodes: html.Node[], meaning: string, description: string) => i18n.Message { const visitor = new _I18nVisitor(_expParser, interpolationConfig); - return (msg: extractor.Message) => visitor.toI18nMessage(msg.nodes, msg.meaning, msg.description); + return (nodes: html.Node[], meaning: string, description: string) => + visitor.toI18nMessage(nodes, meaning, description); } class _I18nVisitor implements html.Visitor { diff --git a/modules/@angular/compiler/src/i18n/message_bundle.ts b/modules/@angular/compiler/src/i18n/message_bundle.ts index bf5f7a673b..035af595b5 100644 --- a/modules/@angular/compiler/src/i18n/message_bundle.ts +++ b/modules/@angular/compiler/src/i18n/message_bundle.ts @@ -10,11 +10,10 @@ import {HtmlParser} from '../ml_parser/html_parser'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseError} from '../parse_util'; +import * as extractor from './extractor_merger'; import * as i18n from './i18n_ast'; -import * as i18nParser from './i18n_parser'; import {Serializer} from './serializers/serializer'; - /** * A container for message extracted from the templates. */ @@ -33,10 +32,14 @@ export class MessageBundle { return htmlParserResult.errors; } - const messages = i18nParser.extractI18nMessages( + const i18nParserResult = extractor.extractMessages( htmlParserResult.rootNodes, interpolationConfig, this._implicitTags, this._implicitAttrs); - messages.forEach((message) => { + if (i18nParserResult.errors.length) { + return i18nParserResult.errors; + } + + i18nParserResult.messages.forEach((message) => { this._messageMap[digestMessage(message.nodes, message.meaning)] = message; }); } diff --git a/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts b/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts index eea2369625..365f0dd9dc 100644 --- a/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts +++ b/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts @@ -8,55 +8,47 @@ import {beforeEach, ddescribe, describe, expect, iit, inject, it, xdescribe, xit} from '@angular/core/testing/testing_internal'; -import {ExtractionResult, Message as HtmlMessage, extractAstMessages, mergeTranslations} from '../../src/i18n/extractor_merger'; -import {getHtmlToI18nConverter} from '../../src/i18n/i18n_parser'; -import {digestMessage} from '../../src/i18n/message_bundle'; +import {extractMessages, mergeTranslations} from '../../src/i18n/extractor_merger'; +import * as i18n from '../../src/i18n/i18n_ast'; +import {digestMessage, serializeNodes as serializeI18nNodes} from '../../src/i18n/message_bundle'; import {TranslationBundle} from '../../src/i18n/translation_bundle'; import * as html from '../../src/ml_parser/ast'; import {HtmlParser} from '../../src/ml_parser/html_parser'; import {DEFAULT_INTERPOLATION_CONFIG} from '../../src/ml_parser/interpolation_config'; -import {serializeNodes} from '../ml_parser/ast_serializer_spec'; +import {serializeNodes as serializeHtmlNodes} from '../ml_parser/ast_serializer_spec'; export function main() { describe('Extractor', () => { describe('elements', () => { it('should extract from elements', () => { expect(extract('
textnested
')).toEqual([ - [['text', 'nested'], 'm', 'd|e'], + [['text', 'nested'], 'm', 'd|e'], ]); }); - it('should extract from elements', () => { + it('should extract from attributes', () => { expect( extract( - '
nested
')) + '
nested
')) .toEqual([ - [ - ['nested'], - 'm', - 'd', - ], - [ - ['title="single child"'], - 'm', - 'd', - ], + [['nested'], 'm1', 'd1'], + [['single child'], 'm2', 'd2'], ]); }); - it('should extract from elements', () => { + it('should extract from ICU messages', () => { expect( extract( '
{count, plural, =0 {

}}
')) .toEqual([ [ [ - '{count, plural, =0 {

}}' + '{count, plural, =0 {[]}}' ], 'm', 'd' ], - [['title="title"'], '', ''], - [['desc="desc"'], '', ''], + [['title'], '', ''], + [['desc'], '', ''], ]); }); @@ -81,11 +73,18 @@ export function main() { extract( `text

htmlnested

{count, plural, =0 {html}}{{interp}}`)) .toEqual([ - [['{count, plural, =0 {html}}'], '', ''], [ [ - 'text', '

htmlnested

', '{count, plural, =0 {html}}', - '{{interp}}' + '{count, plural, =0 {[html]}}' + ], + '', '' + ], + [ + [ + 'text', + 'html, nested', + '{count, plural, =0 {[html]}}', + '[interp]' ], '', '' ], @@ -107,32 +106,35 @@ export function main() { it('should extract ICU messages from translatable elements', () => { // single message when ICU is the only children expect(extract('
{count, plural, =0 {text}}
')).toEqual([ - [['{count, plural, =0 {text}}'], 'm', 'd'], + [['{count, plural, =0 {[text]}}'], 'm', 'd'], ]); // single message when ICU is the only (implicit) children expect(extract('
{count, plural, =0 {text}}
', ['div'])).toEqual([ - [['{count, plural, =0 {text}}'], '', ''], + [['{count, plural, =0 {[text]}}'], '', ''], ]); // one message for the element content and one message for the ICU expect(extract('
before{count, plural, =0 {text}}after
')).toEqual([ - [['before', '{count, plural, =0 {text}}', 'after'], 'm', 'd'], - [['{count, plural, =0 {text}}'], '', ''], + [['before', '{count, plural, =0 {[text]}}', 'after'], 'm', 'd'], + [['{count, plural, =0 {[text]}}'], '', ''], ]); }); it('should extract ICU messages from translatable block', () => { // single message when ICU is the only children expect(extract('{count, plural, =0 {text}}')).toEqual([ - [['{count, plural, =0 {text}}'], 'm', 'd'], + [['{count, plural, =0 {[text]}}'], 'm', 'd'], ]); // one message for the block content and one message for the ICU expect(extract('before{count, plural, =0 {text}}after')) .toEqual([ - [['{count, plural, =0 {text}}'], '', ''], - [['before', '{count, plural, =0 {text}}', 'after'], 'm', 'd'], + [['{count, plural, =0 {[text]}}'], '', ''], + [ + ['before', '{count, plural, =0 {[text]}}', 'after'], 'm', + 'd' + ], ]); }); @@ -142,7 +144,7 @@ export function main() { it('should not extract nested ICU messages', () => { expect(extract('
{count, plural, =0 { {sex, gender, =m {m}} }}
')) .toEqual([ - [['{count, plural, =0 {{sex, gender, =m {m}} }}'], 'm', 'd'], + [['{count, plural, =0 {[{sex, gender, =m {[m]}}, ]}}'], 'm', 'd'], ]); }); }); @@ -150,22 +152,32 @@ export function main() { describe('attributes', () => { it('should extract from attributes outside of translatable sections', () => { expect(extract('
')).toEqual([ - [['title="msg"'], 'm', 'd'], + [['msg'], 'm', 'd'], ]); }); it('should extract from attributes in translatable elements', () => { expect(extract('

')).toEqual([ - [['

'], '', ''], - [['title="msg"'], 'm', 'd'], + [ + [ + '' + ], + '', '' + ], + [['msg'], 'm', 'd'], ]); }); it('should extract from attributes in translatable blocks', () => { expect(extract('

')) .toEqual([ - [['title="msg"'], 'm', 'd'], - [['

'], '', ''], + [['msg'], 'm', 'd'], + [ + [ + '' + ], + '', '' + ], ]); }); @@ -174,15 +186,20 @@ export function main() { extract( '{count, plural, =0 {

}}')) .toEqual([ - [['title="msg"'], 'm', 'd'], - [['{count, plural, =0 {

}}'], '', ''], + [['msg'], 'm', 'd'], + [ + [ + '{count, plural, =0 {[]}}' + ], + '', '' + ], ]); }); it('should extract from attributes in non translatable ICUs', () => { expect(extract('{count, plural, =0 {

}}')) .toEqual([ - [['title="msg"'], 'm', 'd'], + [['msg'], 'm', 'd'], ]); }); @@ -202,7 +219,7 @@ export function main() { it('should extract implicit attributes', () => { expect(extract('bolditalic', [], {'b': ['title']})) .toEqual([ - [['title="bb"'], '', ''], + [['bb'], '', ''], ]); }); }); @@ -296,38 +313,42 @@ export function main() { describe('elements', () => { it('should merge elements', () => { const HTML = `

foo

`; - expect(fakeTranslate(HTML)).toEqual('

-*foo*-

'); + expect(fakeTranslate(HTML)).toEqual('

**foo**

'); }); it('should merge nested elements', () => { const HTML = `
before

foo

`; - expect(fakeTranslate(HTML)).toEqual('
before

-*foo*-

'); + expect(fakeTranslate(HTML)).toEqual('
before

**foo**

'); }); }); describe('blocks', () => { it('should merge blocks', () => { const HTML = `before

foo

barafter`; - expect(fakeTranslate(HTML)).toEqual('before-*

foo

bar*-after'); + expect(fakeTranslate(HTML)) + .toEqual( + 'before**foobar**after'); }); it('should merge nested blocks', () => { const HTML = `
before

foo

barafter
`; expect(fakeTranslate(HTML)) - .toEqual('
before-*

foo

bar*-after
'); + .toEqual( + '
before**foobar**after
'); }); }); describe('attributes', () => { it('should merge attributes', () => { const HTML = `

`; - expect(fakeTranslate(HTML)).toEqual('

'); + expect(fakeTranslate(HTML)).toEqual('

'); }); it('should merge attributes', () => { const HTML = `
{count, plural, =0 {

}}
`; - expect(fakeTranslate(HTML)).toEqual('
{count, plural, =0 {

}}
'); + expect(fakeTranslate(HTML)) + .toEqual('
{count, plural, =0 {

}}
'); }); }); }); @@ -346,20 +367,16 @@ function fakeTranslate( content: string, implicitTags: string[] = [], implicitAttrs: {[k: string]: string[]} = {}): string { const htmlNodes: html.Node[] = parseHtml(content); - const htmlMsgs: HtmlMessage[] = - extractAstMessages(htmlNodes, implicitTags, implicitAttrs).messages; + const messages: i18n.Message[] = + extractMessages(htmlNodes, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs) + .messages; const i18nMsgMap: {[id: string]: html.Node[]} = {}; - const converter = getHtmlToI18nConverter(DEFAULT_INTERPOLATION_CONFIG); - htmlMsgs.forEach(msg => { - const i18nMsg = converter(msg); - - i18nMsgMap[digestMessage(i18nMsg.nodes, i18nMsg.meaning)] = [ - new html.Text('-*', null), - ...msg.nodes, - new html.Text('*-', null), - ]; + messages.forEach(msg => { + const id = digestMessage(msg.nodes, msg.meaning); + const text = serializeI18nNodes(msg.nodes).join(''); + i18nMsgMap[id] = [new html.Text(`**${text}**`, null)]; }); const translations = new TranslationBundle(i18nMsgMap); @@ -367,24 +384,28 @@ function fakeTranslate( const translateNodes = mergeTranslations( htmlNodes, translations, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs); - return serializeNodes(translateNodes).join(''); + return serializeHtmlNodes(translateNodes).join(''); } function extract( html: string, implicitTags: string[] = [], implicitAttrs: {[k: string]: string[]} = {}): [string[], string, string][] { - const messages = extractAstMessages(parseHtml(html), implicitTags, implicitAttrs).messages; + const messages = + extractMessages(parseHtml(html), DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs) + .messages; // clang-format off // https://github.com/angular/clang-format/issues/35 return messages.map( - message => [serializeNodes(message.nodes), message.meaning, message.description, ]) as [string[], string, string][]; + message => [serializeI18nNodes(message.nodes), message.meaning, message.description, ]) as [string[], string, string][]; // clang-format on } function extractErrors( html: string, implicitTags: string[] = [], implicitAttrs: {[k: string]: string[]} = {}): any[] { - const errors = extractAstMessages(parseHtml(html), implicitTags, implicitAttrs).errors; + const errors = + extractMessages(parseHtml(html), DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs) + .errors; return errors.map((e): [string, string] => [e.msg, e.span.toString()]); } diff --git a/modules/@angular/compiler/test/i18n/i18n_parser_spec.ts b/modules/@angular/compiler/test/i18n/i18n_parser_spec.ts index 3caa81418e..8160b175fc 100644 --- a/modules/@angular/compiler/test/i18n/i18n_parser_spec.ts +++ b/modules/@angular/compiler/test/i18n/i18n_parser_spec.ts @@ -6,8 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ +import {extractMessages} from '@angular/compiler/src/i18n/extractor_merger'; import {Message} from '@angular/compiler/src/i18n/i18n_ast'; -import {extractI18nMessages} from '@angular/compiler/src/i18n/i18n_parser'; import {ddescribe, describe, expect, iit, it} from '@angular/core/testing/testing_internal'; import {serializeNodes} from '../../src/i18n/message_bundle'; @@ -312,6 +312,7 @@ function _extractMessages( throw Error(`unexpected parse errors: ${parseResult.errors.join('\n')}`); } - return extractI18nMessages( - parseResult.rootNodes, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs); + return extractMessages( + parseResult.rootNodes, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs) + .messages; }