From e811a5d97f2c01dc18f076481932994f96f44a84 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 1 Aug 2016 14:43:20 -0700 Subject: [PATCH] refactor(i18n): misc updates --- .../@angular/compiler/src/i18n/extractor.ts | 64 ++++++++++--------- .../@angular/compiler/src/i18n/i18n_ast.ts | 6 +- .../@angular/compiler/src/i18n/i18n_parser.ts | 20 ++++-- .../compiler/src/i18n/message_bundle.ts | 11 ++-- .../compiler/src/i18n/translation_bundle.ts | 4 ++ .../compiler/test/i18n/extractor_spec.ts | 39 ++++++++++- .../compiler/test/i18n/i18n_parser_spec.ts | 6 +- .../compiler/test/i18n/message_bundle_spec.ts | 4 +- .../test/i18n/serializers/xtb_spec.ts | 4 +- .../test/ml_parser/ast_serializer_spec.ts | 16 ++--- 10 files changed, 115 insertions(+), 59 deletions(-) diff --git a/modules/@angular/compiler/src/i18n/extractor.ts b/modules/@angular/compiler/src/i18n/extractor.ts index 34b6a9a7d6..e5b2774a5d 100644 --- a/modules/@angular/compiler/src/i18n/extractor.ts +++ b/modules/@angular/compiler/src/i18n/extractor.ts @@ -11,9 +11,10 @@ import {I18nError} from './parse_util'; const _I18N_ATTR = 'i18n'; const _I18N_ATTR_PREFIX = 'i18n-'; +const _I18N_COMMENT_PREFIX_REGEXP = /^i18n:?/; /** - * Extract translatable message from an html AST as a list of html AST nodes + * Extract translatable messages from an html AST as a list of html AST nodes */ export function extractAstMessages( sourceAst: html.Node[], implicitTags: string[], @@ -40,19 +41,14 @@ class _ExtractVisitor implements html.Visitor { // {} private _inIcu = false; - private _sectionStartIndex: number; + private _msgCountAtSectionStart: number; private _errors: I18nError[]; constructor(private _implicitTags: string[], private _implicitAttrs: {[k: string]: string[]}) {} extract(nodes: html.Node[]): ExtractionResult { + this._init(); const messages: Message[] = []; - this._inI18nBlock = false; - this._inI18nNode = false; - this._depth = 0; - this._inIcu = false; - this._sectionStartIndex = void 0; - this._errors = []; nodes.forEach(node => node.visit(this, messages)); @@ -105,13 +101,13 @@ class _ExtractVisitor implements html.Visitor { this._inI18nBlock = true; this._blockStartDepth = this._depth; this._blockChildren = []; - this._blockMeaningAndDesc = comment.value.replace(/^i18n:?/, '').trim(); - this._startSection(messages); + this._blockMeaningAndDesc = comment.value.replace(_I18N_COMMENT_PREFIX_REGEXP, '').trim(); + this._openTranslatableSection(comment, messages); } } else { if (isClosing) { if (this._depth == this._blockStartDepth) { - this._endSection(messages, this._blockChildren); + this._closeTranslatableSection(comment, messages, this._blockChildren); this._inI18nBlock = false; this._addMessage(messages, this._blockChildren, this._blockMeaningAndDesc); } else { @@ -129,18 +125,15 @@ class _ExtractVisitor implements html.Visitor { this._mayBeAddBlockChildren(el); this._depth++; const wasInI18nNode = this._inI18nNode; - let useSection = false; // Extract only top level nodes with the (implicit) "i18n" attribute if not in a block or an ICU // message const i18nAttr = _getI18nAttr(el); - const isImplicitI18n = - this._implicitTags.some((tagName: string): boolean => el.name === tagName); + const isImplicitI18n = this._implicitTags.some((tag: string): boolean => el.name === tag); if (!(this._inI18nNode || this._inIcu || this._inI18nBlock)) { if (i18nAttr) { this._inI18nNode = true; this._addMessage(messages, el.children, i18nAttr.value); - useSection = true; } else if (isImplicitI18n) { this._inI18nNode = true; this._addMessage(messages, el.children); @@ -155,10 +148,11 @@ class _ExtractVisitor implements html.Visitor { this._extractFromAttributes(el, messages); - if (useSection) { - this._startSection(messages); + if (i18nAttr || isImplicitI18n) { + // Start a section when the content is translatable + this._openTranslatableSection(el, messages); html.visitAll(this, el.children, messages); - this._endSection(messages, el.children); + this._closeTranslatableSection(el, messages, el.children); } else { html.visitAll(this, el.children, messages); } @@ -171,6 +165,15 @@ class _ExtractVisitor implements html.Visitor { throw new Error('unreachable code'); } + private _init(): void { + this._inI18nBlock = false; + this._inI18nNode = false; + this._depth = 0; + this._inIcu = false; + this._msgCountAtSectionStart = void 0; + this._errors = []; + } + private _extractFromAttributes(el: html.Element, messages: Message[]): void { const explicitAttrNameToValue: {[k: string]: string} = {}; const implicitAttrNames: string[] = this._implicitAttrs[el.name] || []; @@ -214,20 +217,19 @@ class _ExtractVisitor implements html.Visitor { /** * Marks the start of a section, see `_endSection` */ - private _startSection(messages: Message[]): void { - if (this._sectionStartIndex !== void 0) { - throw new Error('Unexpected section start'); + private _openTranslatableSection(node: html.Node, messages: Message[]): void { + if (this._msgCountAtSectionStart !== void 0) { + this._reportError(node, 'Unexpected section start'); + } else { + this._msgCountAtSectionStart = messages.length; } - - this._sectionStartIndex = messages.length; } /** * Terminates a section. * * If a section has only one significant children (comments not significant) then we should not - * keep the message - * from this children: + * keep the message from this children: * * `

{ICU message}

` would produce two messages: * - one for the

content with meaning and description, @@ -239,12 +241,14 @@ class _ExtractVisitor implements html.Visitor { * Note that we should still keep messages extracted from attributes inside the section (ie in the * ICU message here) */ - private _endSection(messages: Message[], directChildren: html.Node[]): void { - if (this._sectionStartIndex === void 0) { - throw new Error('Unexpected section end'); + private _closeTranslatableSection( + node: html.Node, messages: Message[], directChildren: html.Node[]): void { + if (this._msgCountAtSectionStart === void 0) { + this._reportError(node, 'Unexpected section end'); + return; } - const startIndex = this._sectionStartIndex; + const startIndex = this._msgCountAtSectionStart; const significantChildren: number = directChildren.reduce( (count: number, node: html.Node): number => count + (node instanceof html.Comment ? 0 : 1), 0); @@ -259,7 +263,7 @@ class _ExtractVisitor implements html.Visitor { } } - this._sectionStartIndex = void 0; + this._msgCountAtSectionStart = void 0; } private _reportError(node: html.Node, msg: string): void { diff --git a/modules/@angular/compiler/src/i18n/i18n_ast.ts b/modules/@angular/compiler/src/i18n/i18n_ast.ts index d5c3d3021b..1a2806d96e 100644 --- a/modules/@angular/compiler/src/i18n/i18n_ast.ts +++ b/modules/@angular/compiler/src/i18n/i18n_ast.ts @@ -36,7 +36,7 @@ export class Icu implements Node { visit(visitor: Visitor, context?: any): any { return visitor.visitIcu(this, context); } } -export class TagPlaceholder { +export class TagPlaceholder implements Node { constructor( public tag: string, public attrs: {[k: string]: string}, public startName: string, public closeName: string, public children: Node[], public isVoid: boolean, @@ -45,13 +45,13 @@ export class TagPlaceholder { visit(visitor: Visitor, context?: any): any { return visitor.visitTagPlaceholder(this, context); } } -export class Placeholder { +export class Placeholder implements Node { constructor(public value: string, public name: string = '', public sourceSpan: ParseSourceSpan) {} visit(visitor: Visitor, context?: any): any { return visitor.visitPlaceholder(this, context); } } -export class IcuPlaceholder { +export class IcuPlaceholder implements Node { constructor(public value: Icu, public name: string = '', public sourceSpan: ParseSourceSpan) {} visit(visitor: Visitor, context?: any): any { return visitor.visitIcuPlaceholder(this, context); } diff --git a/modules/@angular/compiler/src/i18n/i18n_parser.ts b/modules/@angular/compiler/src/i18n/i18n_parser.ts index 92bbf96066..6774f54733 100644 --- a/modules/@angular/compiler/src/i18n/i18n_parser.ts +++ b/modules/@angular/compiler/src/i18n/i18n_parser.ts @@ -13,10 +13,12 @@ import {getHtmlTagDefinition} from '../ml_parser/html_tags'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseSourceSpan} from '../parse_util'; -import {extractAstMessages} from './extractor'; +import {Message as HtmlMessage, extractAstMessages} from './extractor'; 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. */ @@ -29,11 +31,19 @@ export function extractI18nMessages( return []; } - const expParser = new ExpressionParser(new ExpressionLexer()); - const visitor = new _I18nVisitor(expParser, interpolationConfig); + const htmlToI18n = getHtmlToI18nConverter(interpolationConfig); - return extractionResult.messages.map( - (msg) => visitor.toI18nMessage(msg.nodes, msg.meaning, msg.description)); + return extractionResult.messages.map(htmlToI18n); +} + +/** + * Returns a function converting html Messages to i18n Messages given an interpolationConfig + */ +export function getHtmlToI18nConverter(interpolationConfig: InterpolationConfig): + (msg: HtmlMessage) => i18n.Message { + const visitor = new _I18nVisitor(_expParser, interpolationConfig); + + return (msg: HtmlMessage) => visitor.toI18nMessage(msg.nodes, msg.meaning, msg.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 f9325c1ff6..01c52ab0fa 100644 --- a/modules/@angular/compiler/src/i18n/message_bundle.ts +++ b/modules/@angular/compiler/src/i18n/message_bundle.ts @@ -37,14 +37,17 @@ export class MessageBundle { htmlParserResult.rootNodes, interpolationConfig, this._implicitTags, this._implicitAttrs); messages.forEach((message) => { - const id = strHash(serializeAst(message.nodes).join('') + `[${message.meaning}]`); - this._messageMap[id] = message; + this._messageMap[messageDigest(message.nodes, message.meaning)] = message; }); } write(serializer: Serializer): string { return serializer.write(this._messageMap); } } +export function messageDigest(nodes: i18n.Node[], meaning: string): string { + return strHash(serializeNodes(nodes).join('') + `[${meaning}]`); +} + /** * String hash function similar to java.lang.String.hashCode(). * The hash code for a string is computed as @@ -103,6 +106,6 @@ class _SerializerVisitor implements i18n.Visitor { const serializerVisitor = new _SerializerVisitor(); -export function serializeAst(ast: i18n.Node[]): string[] { - return ast.map(a => a.visit(serializerVisitor, null)); +export function serializeNodes(nodes: i18n.Node[]): string[] { + return nodes.map(a => a.visit(serializerVisitor, null)); } diff --git a/modules/@angular/compiler/src/i18n/translation_bundle.ts b/modules/@angular/compiler/src/i18n/translation_bundle.ts index b4e7e82762..4874ac9657 100644 --- a/modules/@angular/compiler/src/i18n/translation_bundle.ts +++ b/modules/@angular/compiler/src/i18n/translation_bundle.ts @@ -21,4 +21,8 @@ export class TranslationBundle { serializer: Serializer): TranslationBundle { return new TranslationBundle(serializer.load(content, 'url', placeholders)); } + + get(id: string): html.Node[] { return this._messageMap[id]; } + + has(id: string): boolean { return id in this._messageMap; } } \ No newline at end of file diff --git a/modules/@angular/compiler/test/i18n/extractor_spec.ts b/modules/@angular/compiler/test/i18n/extractor_spec.ts index 81b08275c9..d99c80a043 100644 --- a/modules/@angular/compiler/test/i18n/extractor_spec.ts +++ b/modules/@angular/compiler/test/i18n/extractor_spec.ts @@ -10,7 +10,7 @@ import {ExtractionResult, extractAstMessages} from '@angular/compiler/src/i18n/e import {beforeEach, ddescribe, describe, expect, iit, inject, it, xdescribe, xit} from '@angular/core/testing/testing_internal'; import {HtmlParser} from '../../src/ml_parser/html_parser'; -import {serializeAst} from '../html_parser/ast_serializer_spec'; +import {serializeNodes} from '../html_parser/ast_serializer_spec'; export function main() { describe('MessageExtractor', () => { @@ -21,6 +21,24 @@ export function main() { ]); }); + it('should extract from elements', () => { + expect( + extract( + '

nested
')) + .toEqual([ + [ + ['nested'], + 'm', + 'd', + ], + [ + ['title="single child"'], + 'm', + 'd', + ], + ]); + }); + it('should extract from elements', () => { expect( extract( @@ -87,6 +105,11 @@ export function main() { [['{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}}'], '', ''], + ]); + // 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'], @@ -184,18 +207,24 @@ export function main() { it('should report nested translatable elements', () => { expect(extractErrors(`

`)).toEqual([ ['Could not mark an element as translatable inside a translatable section', ''], + ['Unexpected section start', ''], + ['Unexpected section end', '

'], ]); }); it('should report translatable elements in implicit elements', () => { expect(extractErrors(`

`, ['p'])).toEqual([ ['Could not mark an element as translatable inside a translatable section', ''], + ['Unexpected section start', ''], + ['Unexpected section end', '

'], ]); }); it('should report translatable elements in translatable blocks', () => { expect(extractErrors(``)).toEqual([ ['Could not mark an element as translatable inside a translatable section', ''], + ['Unexpected section start', ''], + ['Unexpected section end', '`, ['b'])).toEqual([ ['Could not mark an element as translatable inside a translatable section', ''], + ['Unexpected section start', ''], + ['Unexpected section end', ''; const ast = parser.parse(html, 'url', true); - expect(serializeAst(ast.rootNodes)).toEqual([html]); + expect(serializeNodes(ast.rootNodes)).toEqual([html]); }); it('should support nesting', () => { @@ -55,7 +55,7 @@ export function main() {

`; const ast = parser.parse(html, 'url', true); - expect(serializeAst(ast.rootNodes)).toEqual([html]); + expect(serializeNodes(ast.rootNodes)).toEqual([html]); }); }); } @@ -91,6 +91,6 @@ class _SerializerVisitor implements html.Visitor { const serializerVisitor = new _SerializerVisitor(); -export function serializeAst(nodes: html.Node[]): string[] { +export function serializeNodes(nodes: html.Node[]): string[] { return nodes.map(node => node.visit(serializerVisitor, null)); }