From 73a84a70989c418cc2a6fbe612df7bcdb2238eda Mon Sep 17 00:00:00 2001 From: vsavkin Date: Wed, 23 Mar 2016 13:44:45 -0700 Subject: [PATCH] refactor(i18n): remove utility functions into a separate file --- modules/angular2/src/i18n/message.ts | 2 +- .../angular2/src/i18n/message_extractor.ts | 227 ++++++------------ modules/angular2/src/i18n/shared.ts | 169 +++++++++++++ .../test/i18n/message_extractor_spec.ts | 92 ++++--- 4 files changed, 301 insertions(+), 189 deletions(-) create mode 100644 modules/angular2/src/i18n/shared.ts diff --git a/modules/angular2/src/i18n/message.ts b/modules/angular2/src/i18n/message.ts index 2ad922d2ff..2a1798c58d 100644 --- a/modules/angular2/src/i18n/message.ts +++ b/modules/angular2/src/i18n/message.ts @@ -8,7 +8,7 @@ import {isPresent, escape} from 'angular2/src/facade/lang'; * `description` is additional information provided to the translator. */ export class Message { - constructor(public content: string, public meaning: string, public description: string) {} + constructor(public content: string, public meaning: string, public description: string = null) {} } /** diff --git a/modules/angular2/src/i18n/message_extractor.ts b/modules/angular2/src/i18n/message_extractor.ts index 21fabe9592..bae8d5f345 100644 --- a/modules/angular2/src/i18n/message_extractor.ts +++ b/modules/angular2/src/i18n/message_extractor.ts @@ -12,11 +12,17 @@ import { import {isPresent, isBlank} from 'angular2/src/facade/lang'; import {StringMapWrapper} from 'angular2/src/facade/collection'; import {Parser} from 'angular2/src/core/change_detection/parser/parser'; -import {Interpolation} from 'angular2/src/core/change_detection/parser/ast'; import {Message, id} from './message'; - -const I18N_ATTR = "i18n"; -const I18N_ATTR_PREFIX = "i18n-"; +import { + I18nError, + Part, + partition, + meaning, + description, + isI18nAttr, + stringifyNodes, + messageFromAttribute +} from './shared'; /** * All messages extracted from a template. @@ -25,13 +31,6 @@ export class ExtractionResult { constructor(public messages: Message[], public errors: ParseError[]) {} } -/** - * An extraction error. - */ -export class I18nExtractionError extends ParseError { - constructor(span: ParseSourceSpan, msg: string) { super(span, msg); } -} - /** * Removes duplicate messages. * @@ -56,20 +55,61 @@ export function removeDuplicates(messages: Message[]): Message[] { /** * Extracts all messages from a template. * - * It works like this. First, the extractor uses the provided html parser to get - * the html AST of the template. Then it partitions the root nodes into parts. - * Everything between two i18n comments becomes a single part. Every other nodes becomes - * a part too. + * Algorithm: * - * We process every part as follows. Say we have a part A. + * To understand the algorithm, you need to know how partitioning works. + * Partitioning is required as we can use two i18n comments to group node siblings together. + * That is why we cannot just use nodes. * - * If the part has the i18n attribute, it gets converted into a message. - * And we do not recurse into that part, except to extract messages from the attributes. + * Partitioning transforms an array of HtmlAst into an array of Part. + * A part can optionally contain a root element or a root text node. And it can also contain + * children. + * A part can contain i18n property, in which case it needs to be extracted. * - * If the part doesn't have the i18n attribute, we recurse into that part and - * partition its children. + * Example: * - * While walking the AST we also remove i18n attributes from messages. + * The following array of nodes will be split into four parts: + * + * ``` + * A + * B + * + * C + * D + * + * E + * ``` + * + * Part 1 containing the a tag. It should not be translated. + * Part 2 containing the b tag. It should be translated. + * Part 3 containing the c tag and the D text node. It should be translated. + * Part 4 containing the E text node. It should not be translated.. + * + * It is also important to understand how we stringify nodes to create a message. + * + * We walk the tree and replace every element node with a placeholder. We also replace + * all expressions in interpolation with placeholders. We also insert a placeholder element + * to wrap a text node containing interpolation. + * + * Example: + * + * The following tree: + * + * ``` + * A{{I}}B + * ``` + * + * will be stringified into: + * ``` + * AB + * ``` + * + * This is what the algorithm does: + * + * 1. Use the provided html parser to get the html AST of the template. + * 2. Partition the root nodes, and process each part separately. + * 3. If a part does not have the i18n attribute, recurse to process children and attributes. + * 4. If a part has the i18n attribute, stringify the nodes to create a Message. */ export class MessageExtractor { messages: Message[]; @@ -85,16 +125,14 @@ export class MessageExtractor { if (res.errors.length > 0) { return new ExtractionResult([], res.errors); } else { - let ps = this._partition(res.rootNodes); - ps.forEach(p => this._extractMessagesFromPart(p)); + this._recurse(res.rootNodes); return new ExtractionResult(this.messages, this.errors); } } - private _extractMessagesFromPart(p: _Part): void { + private _extractMessagesFromPart(p: Part): void { if (p.hasI18n) { - this.messages.push(new Message(_stringifyNodes(p.children, this._parser), _meaning(p.i18n), - _description(p.i18n))); + this.messages.push(p.createMessage(this._parser)); this._recurseToExtractMessagesFromAttributes(p.children); } else { this._recurse(p.children); @@ -106,8 +144,10 @@ export class MessageExtractor { } private _recurse(nodes: HtmlAst[]): void { - let ps = this._partition(nodes); - ps.forEach(p => this._extractMessagesFromPart(p)); + if (isPresent(nodes)) { + let ps = partition(nodes, this.errors); + ps.forEach(p => this._extractMessagesFromPart(p)); + } } private _recurseToExtractMessagesFromAttributes(nodes: HtmlAst[]): void { @@ -121,130 +161,17 @@ export class MessageExtractor { private _extractMessagesFromAttributes(p: HtmlElementAst): void { p.attrs.forEach(attr => { - if (attr.name.startsWith(I18N_ATTR_PREFIX)) { - let expectedName = attr.name.substring(5); - let matching = p.attrs.filter(a => a.name == expectedName); - - if (matching.length > 0) { - let value = _removeInterpolation(matching[0].value, p.sourceSpan, this._parser); - this.messages.push(new Message(value, _meaning(attr.value), _description(attr.value))); - } else { - this.errors.push( - new I18nExtractionError(p.sourceSpan, `Missing attribute '${expectedName}'.`)); + if (isI18nAttr(attr.name)) { + try { + this.messages.push(messageFromAttribute(this._parser, p, attr)); + } catch (e) { + if (e instanceof I18nError) { + this.errors.push(e); + } else { + throw e; + } } } }); } - - // Man, this is so ugly! - private _partition(nodes: HtmlAst[]): _Part[] { - let res = []; - - for (let i = 0; i < nodes.length; ++i) { - let n = nodes[i]; - let temp = []; - if (_isOpeningComment(n)) { - let i18n = (n).value.substring(5).trim(); - i++; - while (!_isClosingComment(nodes[i])) { - temp.push(nodes[i++]); - if (i === nodes.length) { - this.errors.push( - new I18nExtractionError(n.sourceSpan, "Missing closing 'i18n' comment.")); - break; - } - } - res.push(new _Part(null, temp, i18n, true)); - - } else if (n instanceof HtmlElementAst) { - let i18n = _findI18nAttr(n); - res.push(new _Part(n, n.children, isPresent(i18n) ? i18n.value : null, isPresent(i18n))); - } - } - - return res; - } -} - -class _Part { - constructor(public rootElement: HtmlElementAst, public children: HtmlAst[], public i18n: string, - public hasI18n: boolean) {} -} - -function _isOpeningComment(n: HtmlAst): boolean { - return n instanceof HtmlCommentAst && isPresent(n.value) && n.value.startsWith("i18n:"); -} - -function _isClosingComment(n: HtmlAst): boolean { - return n instanceof HtmlCommentAst && isPresent(n.value) && n.value == "/i18n"; -} - -function _stringifyNodes(nodes: HtmlAst[], parser: Parser) { - let visitor = new _StringifyVisitor(parser); - return htmlVisitAll(visitor, nodes).join(""); -} - -class _StringifyVisitor implements HtmlAstVisitor { - constructor(private _parser: Parser) {} - - visitElement(ast: HtmlElementAst, context: any): any { - let attrs = this._join(htmlVisitAll(this, ast.attrs), " "); - let children = this._join(htmlVisitAll(this, ast.children), ""); - return `<${ast.name} ${attrs}>${children}`; - } - - visitAttr(ast: HtmlAttrAst, context: any): any { - if (ast.name.startsWith(I18N_ATTR_PREFIX)) { - return ""; - } else { - return `${ast.name}="${ast.value}"`; - } - } - - visitText(ast: HtmlTextAst, context: any): any { - return _removeInterpolation(ast.value, ast.sourceSpan, this._parser); - } - - visitComment(ast: HtmlCommentAst, context: any): any { return ""; } - - private _join(strs: string[], str: string): string { - return strs.filter(s => s.length > 0).join(str); - } -} - -function _removeInterpolation(value: string, source: ParseSourceSpan, parser: Parser): string { - try { - let parsed = parser.parseInterpolation(value, source.toString()); - if (isPresent(parsed)) { - let ast: Interpolation = parsed.ast; - let res = ""; - for (let i = 0; i < ast.strings.length; ++i) { - res += ast.strings[i]; - if (i != ast.strings.length - 1) { - res += `{{I${i}}}`; - } - } - return res; - } else { - return value; - } - } catch (e) { - return value; - } -} - -function _findI18nAttr(p: HtmlElementAst): HtmlAttrAst { - let i18n = p.attrs.filter(a => a.name == I18N_ATTR); - return i18n.length == 0 ? null : i18n[0]; -} - -function _meaning(i18n: string): string { - if (isBlank(i18n) || i18n == "") return null; - return i18n.split("|")[0]; -} - -function _description(i18n: string): string { - if (isBlank(i18n) || i18n == "") return null; - let parts = i18n.split("|"); - return parts.length > 1 ? parts[1] : null; } \ No newline at end of file diff --git a/modules/angular2/src/i18n/shared.ts b/modules/angular2/src/i18n/shared.ts new file mode 100644 index 0000000000..9f249f2880 --- /dev/null +++ b/modules/angular2/src/i18n/shared.ts @@ -0,0 +1,169 @@ +import {ParseSourceSpan, ParseError} from 'angular2/src/compiler/parse_util'; +import { + HtmlAst, + HtmlAstVisitor, + HtmlElementAst, + HtmlAttrAst, + HtmlTextAst, + HtmlCommentAst, + htmlVisitAll +} from 'angular2/src/compiler/html_ast'; +import {isPresent, isBlank} from 'angular2/src/facade/lang'; +import {Message} from './message'; +import {Parser} from 'angular2/src/core/change_detection/parser/parser'; + +const I18N_ATTR = "i18n"; +const I18N_ATTR_PREFIX = "i18n-"; + +/** + * An i18n error. + */ +export class I18nError extends ParseError { + constructor(span: ParseSourceSpan, msg: string) { super(span, msg); } +} + + +// Man, this is so ugly! +export function partition(nodes: HtmlAst[], errors: ParseError[]): Part[] { + let res = []; + + for (let i = 0; i < nodes.length; ++i) { + let n = nodes[i]; + let temp = []; + if (_isOpeningComment(n)) { + let i18n = (n).value.substring(5).trim(); + i++; + while (!_isClosingComment(nodes[i])) { + temp.push(nodes[i++]); + if (i === nodes.length) { + errors.push(new I18nError(n.sourceSpan, "Missing closing 'i18n' comment.")); + break; + } + } + res.push(new Part(null, null, temp, i18n, true)); + + } else if (n instanceof HtmlElementAst) { + let i18n = _findI18nAttr(n); + res.push(new Part(n, null, n.children, isPresent(i18n) ? i18n.value : null, isPresent(i18n))); + } else if (n instanceof HtmlTextAst) { + res.push(new Part(null, n, null, null, false)); + } + } + + return res; +} + +export class Part { + constructor(public rootElement: HtmlElementAst, public rootTextNode: HtmlTextAst, + public children: HtmlAst[], public i18n: string, public hasI18n: boolean) {} + + get sourceSpan(): ParseSourceSpan { + if (isPresent(this.rootElement)) + return this.rootElement.sourceSpan; + else if (isPresent(this.rootTextNode)) + return this.rootTextNode.sourceSpan; + else + return this.children[0].sourceSpan; + } + + createMessage(parser: Parser): Message { + return new Message(stringifyNodes(this.children, parser), meaning(this.i18n), + description(this.i18n)); + } +} + +function _isOpeningComment(n: HtmlAst): boolean { + return n instanceof HtmlCommentAst && isPresent(n.value) && n.value.startsWith("i18n:"); +} + +function _isClosingComment(n: HtmlAst): boolean { + return n instanceof HtmlCommentAst && isPresent(n.value) && n.value == "/i18n"; +} + +export function isI18nAttr(n: string): boolean { + return n.startsWith(I18N_ATTR_PREFIX); +} + +function _findI18nAttr(p: HtmlElementAst): HtmlAttrAst { + let i18n = p.attrs.filter(a => a.name == I18N_ATTR); + return i18n.length == 0 ? null : i18n[0]; +} + +export function meaning(i18n: string): string { + if (isBlank(i18n) || i18n == "") return null; + return i18n.split("|")[0]; +} + +export function description(i18n: string): string { + if (isBlank(i18n) || i18n == "") return null; + let parts = i18n.split("|"); + return parts.length > 1 ? parts[1] : null; +} + +export function messageFromAttribute(parser: Parser, p: HtmlElementAst, + attr: HtmlAttrAst): Message { + let expectedName = attr.name.substring(5); + let matching = p.attrs.filter(a => a.name == expectedName); + + if (matching.length > 0) { + let value = removeInterpolation(matching[0].value, matching[0].sourceSpan, parser); + return new Message(value, meaning(attr.value), description(attr.value)); + } else { + throw new I18nError(p.sourceSpan, `Missing attribute '${expectedName}'.`); + } +} + +export function removeInterpolation(value: string, source: ParseSourceSpan, + parser: Parser): string { + try { + let parsed = parser.splitInterpolation(value, source.toString()); + if (isPresent(parsed)) { + let res = ""; + for (let i = 0; i < parsed.strings.length; ++i) { + res += parsed.strings[i]; + if (i != parsed.strings.length - 1) { + res += ``; + } + } + return res; + } else { + return value; + } + } catch (e) { + return value; + } +} + +export function stringifyNodes(nodes: HtmlAst[], parser: Parser) { + let visitor = new _StringifyVisitor(parser); + return htmlVisitAll(visitor, nodes).join(""); +} + +class _StringifyVisitor implements HtmlAstVisitor { + private _index: number = 0; + constructor(private _parser: Parser) {} + + visitElement(ast: HtmlElementAst, context: any): any { + let name = this._index++; + let children = this._join(htmlVisitAll(this, ast.children), ""); + return `${children}`; + } + + visitAttr(ast: HtmlAttrAst, context: any): any { return null; } + + visitText(ast: HtmlTextAst, context: any): any { + let index = this._index++; + let noInterpolation = removeInterpolation(ast.value, ast.sourceSpan, this._parser); + if (noInterpolation != ast.value) { + return `${noInterpolation}`; + } else { + return ast.value; + } + } + + visitComment(ast: HtmlCommentAst, context: any): any { return ""; } + + private _join(strs: string[], str: string): string { + return strs.filter(s => s.length > 0).join(str); + } +} \ No newline at end of file diff --git a/modules/angular2/test/i18n/message_extractor_spec.ts b/modules/angular2/test/i18n/message_extractor_spec.ts index 7b1d1a8b3b..e624ad327e 100644 --- a/modules/angular2/test/i18n/message_extractor_spec.ts +++ b/modules/angular2/test/i18n/message_extractor_spec.ts @@ -1,8 +1,8 @@ import { AsyncTestCompleter, beforeEach, - ddescribe, describe, + ddescribe, expect, iit, inject, @@ -58,18 +58,6 @@ export function main() { ]); }); - it('should error on i18n attributes without matching "real" attributes', () => { - let res = extractor.extract(` -
-
- `, - "someurl"); - - expect(res.errors.length).toEqual(1); - expect(res.errors[0].msg).toEqual("Missing attribute 'title2'."); - }); - it('should extract from partitions', () => { let res = extractor.extract(` message1 @@ -91,40 +79,39 @@ export function main() { expect(res.messages).toEqual([new Message("message1", "meaning1", "desc1")]); }); - it('should error when cannot find a matching desc', () => { - let res = extractor.extract(` - message1`, - "someUrl"); - - expect(res.errors.length).toEqual(1); - expect(res.errors[0].msg).toEqual("Missing closing 'i18n' comment."); - }); - it('should replace interpolation with placeholders (text nodes)', () => { let res = extractor.extract("
Hi {{one}} and {{two}}
", "someurl"); - expect(res.messages).toEqual([new Message("Hi {{I0}} and {{I1}}", null, null)]); + expect(res.messages) + .toEqual( + [new Message('Hi and ', null, null)]); }); it('should replace interpolation with placeholders (attributes)', () => { let res = extractor.extract("
", "someurl"); - expect(res.messages).toEqual([new Message("Hi {{I0}} and {{I1}}", null, null)]); - }); - - it('should ignore errors in interpolation', () => { - let res = extractor.extract("
Hi {{on???.s}}
", "someurl"); - expect(res.messages).toEqual([new Message("Hi {{on???.s}}", null, null)]); - }); - - it("should return parse errors when the template is invalid", () => { - let res = extractor.extract(" and ', null, null)]); }); it("should handle html content", () => { - let res = extractor.extract('
message
', "someurl"); - expect(res.messages).toEqual([new Message('
message
', null, null)]); + let res = extractor.extract( + '
zero
one
two
', "someurl"); + expect(res.messages) + .toEqual([ + new Message('zeroonetwo', null, + null) + ]); + }); + + it("should handle html content with interpolation", () => { + let res = + extractor.extract('
zero{{a}}
{{b}}
', "someurl"); + expect(res.messages) + .toEqual([ + new Message( + 'zero', + null, null) + ]); }); it("should extract from nested elements", () => { @@ -143,7 +130,7 @@ export function main() { '
message
', "someurl"); expect(res.messages) .toEqual([ - new Message('
message
', null, null), + new Message('message', null, null), new Message('value', "meaning", "desc") ]); }); @@ -159,5 +146,34 @@ export function main() { new Message("message", "meaning", "desc1"), ]); }); + + describe("errors", () => { + it('should error on i18n attributes without matching "real" attributes', () => { + let res = extractor.extract(` +
+
+ `, + "someurl"); + + expect(res.errors.length).toEqual(1); + expect(res.errors[0].msg).toEqual("Missing attribute 'title2'."); + }); + + it('should error when cannot find a matching desc', () => { + let res = extractor.extract(` + message1`, + "someUrl"); + + expect(res.errors.length).toEqual(1); + expect(res.errors[0].msg).toEqual("Missing closing 'i18n' comment."); + }); + + it("should return parse errors when the template is invalid", () => { + let res = extractor.extract("