From c3e5ddbe20de3d0cd525c29ffb8b182a5fbf1052 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Thu, 19 Jan 2017 14:42:25 -0800 Subject: [PATCH] refactor(compiler): [i18n] move dedup and placeholder mapping to the `MessageBundle` It makes implementing a `Serializer` simpler as implementations do not have to care any more about message dedup and placeholder mapping. --- .../compiler/src/i18n/message_bundle.ts | 74 +++++++++++++++++-- .../src/i18n/serializers/serializer.ts | 3 + .../compiler/src/i18n/serializers/xliff.ts | 9 +-- .../compiler/src/i18n/serializers/xmb.ts | 62 ++++++---------- .../compiler/test/i18n/message_bundle_spec.ts | 9 +-- 5 files changed, 100 insertions(+), 57 deletions(-) diff --git a/modules/@angular/compiler/src/i18n/message_bundle.ts b/modules/@angular/compiler/src/i18n/message_bundle.ts index a82afebc79..f248bb84e3 100644 --- a/modules/@angular/compiler/src/i18n/message_bundle.ts +++ b/modules/@angular/compiler/src/i18n/message_bundle.ts @@ -11,14 +11,15 @@ import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseError} from '../parse_util'; import {extractMessages} from './extractor_merger'; -import {Message} from './i18n_ast'; -import {Serializer} from './serializers/serializer'; +import * as i18n from './i18n_ast'; +import {PlaceholderMapper, Serializer} from './serializers/serializer'; + /** * A container for message extracted from the templates. */ export class MessageBundle { - private _messages: Message[] = []; + private _messages: i18n.Message[] = []; constructor( private _htmlParser: HtmlParser, private _implicitTags: string[], @@ -42,7 +43,70 @@ export class MessageBundle { this._messages.push(...i18nParserResult.messages); } - getMessages(): Message[] { return this._messages; } + // Return the message in the internal format + // The public (serialized) format might be different, see the `write` method. + getMessages(): i18n.Message[] { return this._messages; } - write(serializer: Serializer): string { return serializer.write(this._messages); } + write(serializer: Serializer): string { + const messages: {[id: string]: i18n.Message} = {}; + const mapperVisitor = new MapPlaceholderNames(); + + // Deduplicate messages based on their ID + this._messages.forEach(message => { + const id = serializer.digest(message); + if (!messages.hasOwnProperty(id)) { + messages[id] = message; + } + }); + + // Transform placeholder names using the serializer mapping + const msgList = Object.keys(messages).map(id => { + const mapper = serializer.createNameMapper(messages[id]); + const src = messages[id]; + const nodes = mapper ? mapperVisitor.convert(src.nodes, mapper) : src.nodes; + return new i18n.Message(nodes, {}, {}, src.meaning, src.description, id); + }); + + return serializer.write(msgList); + } } + +// Transform an i18n AST by renaming the placeholder nodes with the given mapper +class MapPlaceholderNames implements i18n.Visitor { + convert(nodes: i18n.Node[], mapper: PlaceholderMapper): i18n.Node[] { + return mapper ? nodes.map(n => n.visit(this, mapper)) : nodes; + } + + visitText(text: i18n.Text, mapper: PlaceholderMapper): i18n.Text { + return new i18n.Text(text.value, text.sourceSpan); + } + + visitContainer(container: i18n.Container, mapper: PlaceholderMapper): i18n.Container { + const children = container.children.map(n => n.visit(this, mapper)); + return new i18n.Container(children, container.sourceSpan); + } + + visitIcu(icu: i18n.Icu, mapper: PlaceholderMapper): i18n.Icu { + const cases: {[k: string]: i18n.Node} = {}; + Object.keys(icu.cases).forEach(key => cases[key] = icu.cases[key].visit(this, mapper)); + const msg = new i18n.Icu(icu.expression, icu.type, cases, icu.sourceSpan); + msg.expressionPlaceholder = icu.expressionPlaceholder; + return msg; + } + + visitTagPlaceholder(ph: i18n.TagPlaceholder, mapper: PlaceholderMapper): i18n.TagPlaceholder { + const startName = mapper.toPublicName(ph.startName); + const closeName = ph.closeName ? mapper.toPublicName(ph.closeName) : ph.closeName; + const children = ph.children.map(n => n.visit(this, mapper)); + return new i18n.TagPlaceholder( + ph.tag, ph.attrs, startName, closeName, children, ph.isVoid, ph.sourceSpan); + } + + visitPlaceholder(ph: i18n.Placeholder, mapper: PlaceholderMapper): i18n.Placeholder { + return new i18n.Placeholder(ph.value, mapper.toPublicName(ph.name), ph.sourceSpan); + } + + visitIcuPlaceholder(ph: i18n.IcuPlaceholder, mapper: PlaceholderMapper): i18n.IcuPlaceholder { + return new i18n.IcuPlaceholder(ph.value, mapper.toPublicName(ph.name), ph.sourceSpan); + } +} \ No newline at end of file diff --git a/modules/@angular/compiler/src/i18n/serializers/serializer.ts b/modules/@angular/compiler/src/i18n/serializers/serializer.ts index 683d823408..a3476fc8e1 100644 --- a/modules/@angular/compiler/src/i18n/serializers/serializer.ts +++ b/modules/@angular/compiler/src/i18n/serializers/serializer.ts @@ -9,6 +9,9 @@ import * as i18n from '../i18n_ast'; export abstract class Serializer { + // - The `placeholders` and `placeholderToMessage` properties are irrelevant in the input messages + // - The `id` contains the message id that the serializer is expected to use + // - Placeholder names are already map to public names using the provided mapper abstract write(messages: i18n.Message[]): string; abstract load(content: string, url: string): {[msgId: string]: i18n.Node[]}; diff --git a/modules/@angular/compiler/src/i18n/serializers/xliff.ts b/modules/@angular/compiler/src/i18n/serializers/xliff.ts index b4f91c8632..bfad61bb80 100644 --- a/modules/@angular/compiler/src/i18n/serializers/xliff.ts +++ b/modules/@angular/compiler/src/i18n/serializers/xliff.ts @@ -30,17 +30,10 @@ const _UNIT_TAG = 'trans-unit'; export class Xliff extends Serializer { write(messages: i18n.Message[]): string { const visitor = new _WriteVisitor(); - const visited: {[id: string]: boolean} = {}; const transUnits: xml.Node[] = []; messages.forEach(message => { - const id = this.digest(message); - - // deduplicate messages - if (visited[id]) return; - visited[id] = true; - - const transUnit = new xml.Tag(_UNIT_TAG, {id, datatype: 'html'}); + const transUnit = new xml.Tag(_UNIT_TAG, {id: message.id, datatype: 'html'}); transUnit.children.push( new xml.CR(8), new xml.Tag(_SOURCE_TAG, {}, visitor.serialize(message.nodes)), new xml.CR(8), new xml.Tag(_TARGET_TAG)); diff --git a/modules/@angular/compiler/src/i18n/serializers/xmb.ts b/modules/@angular/compiler/src/i18n/serializers/xmb.ts index b947c9cd99..5ae70bd3c6 100644 --- a/modules/@angular/compiler/src/i18n/serializers/xmb.ts +++ b/modules/@angular/compiler/src/i18n/serializers/xmb.ts @@ -41,19 +41,10 @@ export class Xmb extends Serializer { write(messages: i18n.Message[]): string { const exampleVisitor = new ExampleVisitor(); const visitor = new _Visitor(); - const visited: {[id: string]: boolean} = {}; let rootNode = new xml.Tag(_MESSAGES_TAG); messages.forEach(message => { - const id = this.digest(message); - - // deduplicate messages - if (visited[id]) return; - visited[id] = true; - - const mapper = this.createNameMapper(message); - - const attrs: {[k: string]: string} = {id}; + const attrs: {[k: string]: string} = {id: message.id}; if (message.description) { attrs['desc'] = message.description; @@ -64,8 +55,7 @@ export class Xmb extends Serializer { } rootNode.children.push( - new xml.CR(2), - new xml.Tag(_MESSAGE_TAG, attrs, visitor.serialize(message.nodes, {mapper}))); + new xml.CR(2), new xml.Tag(_MESSAGE_TAG, attrs, visitor.serialize(message.nodes))); }); rootNode.children.push(new xml.CR()); @@ -93,21 +83,19 @@ export class Xmb extends Serializer { } class _Visitor implements i18n.Visitor { - visitText(text: i18n.Text, ctx: {mapper: PlaceholderMapper}): xml.Node[] { - return [new xml.Text(text.value)]; - } + visitText(text: i18n.Text, context?: any): xml.Node[] { return [new xml.Text(text.value)]; } - visitContainer(container: i18n.Container, ctx: any): xml.Node[] { + visitContainer(container: i18n.Container, context: any): xml.Node[] { const nodes: xml.Node[] = []; - container.children.forEach((node: i18n.Node) => nodes.push(...node.visit(this, ctx))); + container.children.forEach((node: i18n.Node) => nodes.push(...node.visit(this))); return nodes; } - visitIcu(icu: i18n.Icu, ctx: {mapper: PlaceholderMapper}): xml.Node[] { + visitIcu(icu: i18n.Icu, context?: any): xml.Node[] { const nodes = [new xml.Text(`{${icu.expressionPlaceholder}, ${icu.type}, `)]; Object.keys(icu.cases).forEach((c: string) => { - nodes.push(new xml.Text(`${c} {`), ...icu.cases[c].visit(this, ctx), new xml.Text(`} `)); + nodes.push(new xml.Text(`${c} {`), ...icu.cases[c].visit(this), new xml.Text(`} `)); }); nodes.push(new xml.Text(`}`)); @@ -115,34 +103,30 @@ class _Visitor implements i18n.Visitor { return nodes; } - visitTagPlaceholder(ph: i18n.TagPlaceholder, ctx: {mapper: PlaceholderMapper}): xml.Node[] { + visitTagPlaceholder(ph: i18n.TagPlaceholder, context?: any): xml.Node[] { const startEx = new xml.Tag(_EXEMPLE_TAG, {}, [new xml.Text(`<${ph.tag}>`)]); - let name = ctx.mapper.toPublicName(ph.startName); - const startTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name}, [startEx]); + const startTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name: ph.startName}, [startEx]); if (ph.isVoid) { // void tags have no children nor closing tags return [startTagPh]; } const closeEx = new xml.Tag(_EXEMPLE_TAG, {}, [new xml.Text(``)]); - name = ctx.mapper.toPublicName(ph.closeName); - const closeTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name}, [closeEx]); + const closeTagPh = new xml.Tag(_PLACEHOLDER_TAG, {name: ph.closeName}, [closeEx]); - return [startTagPh, ...this.serialize(ph.children, ctx), closeTagPh]; + return [startTagPh, ...this.serialize(ph.children), closeTagPh]; } - visitPlaceholder(ph: i18n.Placeholder, ctx: {mapper: PlaceholderMapper}): xml.Node[] { - const name = ctx.mapper.toPublicName(ph.name); - return [new xml.Tag(_PLACEHOLDER_TAG, {name})]; + visitPlaceholder(ph: i18n.Placeholder, context?: any): xml.Node[] { + return [new xml.Tag(_PLACEHOLDER_TAG, {name: ph.name})]; } - visitIcuPlaceholder(ph: i18n.IcuPlaceholder, ctx: {mapper: PlaceholderMapper}): xml.Node[] { - const name = ctx.mapper.toPublicName(ph.name); - return [new xml.Tag(_PLACEHOLDER_TAG, {name})]; + visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): xml.Node[] { + return [new xml.Tag(_PLACEHOLDER_TAG, {name: ph.name})]; } - serialize(nodes: i18n.Node[], ctx: {mapper: PlaceholderMapper}): xml.Node[] { - return [].concat(...nodes.map(node => node.visit(this, ctx))); + serialize(nodes: i18n.Node[]): xml.Node[] { + return [].concat(...nodes.map(node => node.visit(this))); } } @@ -197,25 +181,25 @@ export class XmbPlaceholderMapper implements PlaceholderMapper, i18n.Visitor { return this.xmbToInternal.hasOwnProperty(publicName) ? this.xmbToInternal[publicName] : null; } - visitText(text: i18n.Text, ctx?: any): any { return null; } + visitText(text: i18n.Text, context?: any): any { return null; } - visitContainer(container: i18n.Container, ctx?: any): any { + visitContainer(container: i18n.Container, context?: any): any { container.children.forEach(child => child.visit(this)); } - visitIcu(icu: i18n.Icu, ctx?: any): any { + visitIcu(icu: i18n.Icu, context?: any): any { Object.keys(icu.cases).forEach(k => { icu.cases[k].visit(this); }); } - visitTagPlaceholder(ph: i18n.TagPlaceholder, ctx?: any): any { + visitTagPlaceholder(ph: i18n.TagPlaceholder, context?: any): any { this.addPlaceholder(ph.startName); ph.children.forEach(child => child.visit(this)); this.addPlaceholder(ph.closeName); } - visitPlaceholder(ph: i18n.Placeholder, ctx?: any): any { this.addPlaceholder(ph.name); } + visitPlaceholder(ph: i18n.Placeholder, context?: any): any { this.addPlaceholder(ph.name); } - visitIcuPlaceholder(ph: i18n.IcuPlaceholder, ctx?: any): any { this.addPlaceholder(ph.name); } + visitIcuPlaceholder(ph: i18n.IcuPlaceholder, context?: any): any { this.addPlaceholder(ph.name); } // XMB placeholders could only contains A-Z, 0-9 and _ private addPlaceholder(internalName: string): void { diff --git a/modules/@angular/compiler/test/i18n/message_bundle_spec.ts b/modules/@angular/compiler/test/i18n/message_bundle_spec.ts index 5bd1a1a15a..faeb7d6079 100644 --- a/modules/@angular/compiler/test/i18n/message_bundle_spec.ts +++ b/modules/@angular/compiler/test/i18n/message_bundle_spec.ts @@ -28,14 +28,13 @@ export function main(): void { ]); }); - it('should extract the all messages and duplicates', () => { + it('should extract and dedup messages', () => { messages.updateFromTemplate( - '

Translate Me

Translate Me

Translate Me

', 'url', - DEFAULT_INTERPOLATION_CONFIG); + '

Translate Me

Translate Me

Translate Me

', + 'url', DEFAULT_INTERPOLATION_CONFIG); expect(humanizeMessages(messages)).toEqual([ 'Translate Me (m|d)', 'Translate Me (|)', - 'Translate Me (|)', ]); }); }); @@ -50,7 +49,7 @@ class _TestSerializer extends Serializer { load(content: string, url: string): {} { return null; } - digest(msg: i18n.Message): string { return 'unused'; } + digest(msg: i18n.Message): string { return msg.id || `default`; } } function humanizeMessages(catalog: MessageBundle): string[] {