From 402fd934d09efb31b021508632ab88613e37cc25 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Thu, 30 Jun 2016 14:20:15 -0700 Subject: [PATCH] refactor: code cleanup --- .../compiler/src/i18n/i18n_html_parser.ts | 4 +- modules/@angular/compiler/src/i18n/shared.ts | 64 ++++++++++++------- .../test/i18n/i18n_html_parser_spec.ts | 20 +++--- .../test/i18n/message_extractor_spec.ts | 7 +- 4 files changed, 60 insertions(+), 35 deletions(-) diff --git a/modules/@angular/compiler/src/i18n/i18n_html_parser.ts b/modules/@angular/compiler/src/i18n/i18n_html_parser.ts index e1254776cd..a6d44fc6d7 100644 --- a/modules/@angular/compiler/src/i18n/i18n_html_parser.ts +++ b/modules/@angular/compiler/src/i18n/i18n_html_parser.ts @@ -17,7 +17,7 @@ import {ParseError, ParseSourceSpan} from '../parse_util'; import {expandNodes} from './expander'; import {Message, id} from './message'; -import {I18N_ATTR, I18N_ATTR_PREFIX, I18nError, Part, dedupePhName, getPhNameFromBinding, messageFromAttribute, messageFromI18nAttribute, partition} from './shared'; +import {I18N_ATTR, I18N_ATTR_PREFIX, I18nError, Part, dedupePhName, extractPhNameFromInterpolation, messageFromAttribute, messageFromI18nAttribute, partition} from './shared'; const _PLACEHOLDER_ELEMENT = 'ph'; const _NAME_ATTR = 'name'; @@ -289,7 +289,7 @@ export class I18nHtmlParser implements HtmlParser { let usedNames = new Map(); for (var i = 0; i < exps.length; i++) { - let phName = getPhNameFromBinding(exps[i], i); + let phName = extractPhNameFromInterpolation(exps[i], i); expMap.set(dedupePhName(usedNames, phName), exps[i]); } return expMap; diff --git a/modules/@angular/compiler/src/i18n/shared.ts b/modules/@angular/compiler/src/i18n/shared.ts index cfbe9152bf..2aceee9c08 100644 --- a/modules/@angular/compiler/src/i18n/shared.ts +++ b/modules/@angular/compiler/src/i18n/shared.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Parser} from '../expression_parser/parser'; +import {Parser as ExpressionParser} from '../expression_parser/parser'; import {StringWrapper, isBlank, isPresent} from '../facade/lang'; import {HtmlAst, HtmlAstVisitor, HtmlAttrAst, HtmlCommentAst, HtmlElementAst, HtmlExpansionAst, HtmlExpansionCaseAst, HtmlTextAst, htmlVisitAll} from '../html_ast'; import {InterpolationConfig} from '../interpolation_config'; @@ -15,7 +15,7 @@ import {Message} from './message'; export const I18N_ATTR = 'i18n'; export const I18N_ATTR_PREFIX = 'i18n-'; -var CUSTOM_PH_EXP = /\/\/[\s\S]*i18n[\s\S]*\([\s\S]*ph[\s\S]*=[\s\S]*"([\s\S]*?)"[\s\S]*\)/g; +const _CUSTOM_PH_EXP = /\/\/[\s\S]*i18n[\s\S]*\([\s\S]*ph[\s\S]*=[\s\S]*"([\s\S]*?)"[\s\S]*\)/g; /** * An i18n error. @@ -73,7 +73,7 @@ export class Part { return this.children[0].sourceSpan; } - createMessage(parser: Parser, interpolationConfig: InterpolationConfig): Message { + createMessage(parser: ExpressionParser, interpolationConfig: InterpolationConfig): Message { return new Message( stringifyNodes(this.children, parser, interpolationConfig), meaning(this.i18n), description(this.i18n)); @@ -115,7 +115,7 @@ export function description(i18n: string): string { * @internal */ export function messageFromI18nAttribute( - parser: Parser, interpolationConfig: InterpolationConfig, p: HtmlElementAst, + parser: ExpressionParser, interpolationConfig: InterpolationConfig, p: HtmlElementAst, i18nAttr: HtmlAttrAst): Message { let expectedName = i18nAttr.name.substring(5); let attr = p.attrs.find(a => a.name == expectedName); @@ -129,62 +129,82 @@ export function messageFromI18nAttribute( } export function messageFromAttribute( - parser: Parser, interpolationConfig: InterpolationConfig, attr: HtmlAttrAst, + parser: ExpressionParser, interpolationConfig: InterpolationConfig, attr: HtmlAttrAst, meaning: string = null, description: string = null): Message { let value = removeInterpolation(attr.value, attr.sourceSpan, parser, interpolationConfig); return new Message(value, meaning, description); } +/** + * Replace interpolation in the `value` string with placeholders + */ export function removeInterpolation( - value: string, source: ParseSourceSpan, parser: Parser, + value: string, source: ParseSourceSpan, expressionParser: ExpressionParser, interpolationConfig: InterpolationConfig): string { try { - let parsed = parser.splitInterpolation(value, source.toString(), interpolationConfig); - let usedNames = new Map(); + const parsed = + expressionParser.splitInterpolation(value, source.toString(), interpolationConfig); + const usedNames = new Map(); if (isPresent(parsed)) { let res = ''; for (let i = 0; i < parsed.strings.length; ++i) { res += parsed.strings[i]; if (i != parsed.strings.length - 1) { - let customPhName = getPhNameFromBinding(parsed.expressions[i], i); + let customPhName = extractPhNameFromInterpolation(parsed.expressions[i], i); customPhName = dedupePhName(usedNames, customPhName); res += ``; } } return res; - } else { - return value; } + + return value; } catch (e) { return value; } } -export function getPhNameFromBinding(input: string, index: number): string { - let customPhMatch = StringWrapper.split(input, CUSTOM_PH_EXP); - return customPhMatch.length > 1 ? customPhMatch[1] : `${index}`; +/** + * Extract the placeholder name from the interpolation. + * + * Use a custom name when specified (ie: `{{ //i18n(ph="FIRST")}}`) otherwise generate a + * unique name. + */ +export function extractPhNameFromInterpolation(input: string, index: number): string { + let customPhMatch = StringWrapper.split(input, _CUSTOM_PH_EXP); + return customPhMatch.length > 1 ? customPhMatch[1] : `INTERPOLATION_${index}`; } +/** + * Return a unique placeholder name based on the given name + */ export function dedupePhName(usedNames: Map, name: string): string { - let duplicateNameCount = usedNames.get(name); - if (isPresent(duplicateNameCount)) { + const duplicateNameCount = usedNames.get(name); + + if (duplicateNameCount) { usedNames.set(name, duplicateNameCount + 1); return `${name}_${duplicateNameCount}`; - } else { - usedNames.set(name, 1); - return name; } + + usedNames.set(name, 1); + return name; } +/** + * Convert a list of nodes to a string message. + * + */ export function stringifyNodes( - nodes: HtmlAst[], parser: Parser, interpolationConfig: InterpolationConfig): string { - let visitor = new _StringifyVisitor(parser, interpolationConfig); + nodes: HtmlAst[], expressionParser: ExpressionParser, + interpolationConfig: InterpolationConfig): string { + const visitor = new _StringifyVisitor(expressionParser, interpolationConfig); return htmlVisitAll(visitor, nodes).join(''); } class _StringifyVisitor implements HtmlAstVisitor { private _index: number = 0; - constructor(private _parser: Parser, private _interpolationConfig: InterpolationConfig) {} + constructor( + private _parser: ExpressionParser, private _interpolationConfig: InterpolationConfig) {} visitElement(ast: HtmlElementAst, context: any): any { let name = this._index++; diff --git a/modules/@angular/compiler/test/i18n/i18n_html_parser_spec.ts b/modules/@angular/compiler/test/i18n/i18n_html_parser_spec.ts index de8a199037..98fd9da0bd 100644 --- a/modules/@angular/compiler/test/i18n/i18n_html_parser_spec.ts +++ b/modules/@angular/compiler/test/i18n/i18n_html_parser_spec.ts @@ -67,8 +67,9 @@ export function main() { it('should handle interpolation', () => { let translations: {[key: string]: string} = {}; - translations[id(new Message(' and ', null, null))] = - ' or '; + translations[id(new Message( + ' and ', null, null))] = + ' or '; expect(humanizeDom(parse('
', translations))) .toEqual([[HtmlElementAst, 'div', 0], [HtmlAttrAst, 'value', '{{b}} or {{a}}']]); @@ -76,8 +77,9 @@ export function main() { it('should handle interpolation with config', () => { let translations: {[key: string]: string} = {}; - translations[id(new Message(' and ', null, null))] = - ' or '; + translations[id(new Message( + ' and ', null, null))] = + ' or '; expect(humanizeDom(parse( '
', translations, [], {}, @@ -135,8 +137,9 @@ export function main() { it('should support interpolation', () => { let translations: {[key: string]: string} = {}; translations[id(new Message( - 'ab', null, - null))] = 'BA'; + 'ab', + null, null))] = + 'BA'; expect(humanizeDom(parse('
ab{{i}}
', translations))).toEqual([ [HtmlElementAst, 'div', 0], [HtmlElementAst, 'b', 1], @@ -237,11 +240,12 @@ export function main() { it('should error when the translation refers to an invalid expression', () => { let translations: {[key: string]: string} = {}; - translations[id(new Message('hi ', null, null))] = 'hi '; + translations[id(new Message('hi ', null, null))] = + 'hi '; expect( humanizeErrors(parse('
', translations).errors)) - .toEqual(['Invalid interpolation name \'99\'']); + .toEqual(['Invalid interpolation name \'INTERPOLATION_99\'']); }); }); diff --git a/modules/@angular/compiler/test/i18n/message_extractor_spec.ts b/modules/@angular/compiler/test/i18n/message_extractor_spec.ts index 3b45af10cd..cb1a8048d9 100644 --- a/modules/@angular/compiler/test/i18n/message_extractor_spec.ts +++ b/modules/@angular/compiler/test/i18n/message_extractor_spec.ts @@ -82,14 +82,15 @@ export function main() { it('should replace interpolation with placeholders (text nodes)', () => { let res = extractor.extract('
Hi {{one}} and {{two}}
', 'someurl'); expect(res.messages).toEqual([new Message( - 'Hi and ', null, null)]); + 'Hi and ', + null, null)]); }); it('should replace interpolation with placeholders (attributes)', () => { let res = extractor.extract('
', 'someurl'); expect(res.messages).toEqual([new Message( - 'Hi and ', null, null)]); + 'Hi and ', null, null)]); }); it('should replace interpolation with named placeholders if provided (text nodes)', () => { @@ -142,7 +143,7 @@ export function main() { let res = extractor.extract('
zero{{a}}
{{b}}
', 'someurl'); expect(res.messages).toEqual([new Message( - 'zero', + 'zero', null, null)]); });