angular · kara · Apr 12, 2016 · vsavkin · Apr 11, 2016 · kara
diff --git a/modules/angular2/src/core/change_detection/parser/parser.ts b/modules/angular2/src/core/change_detection/parser/parser.ts
@@ -50,6 +50,7 @@ import {
 var _implicitReceiver = new ImplicitReceiver();
 // TODO(tbosch): Cannot make this const/final right now because of the transpiler...
 var INTERPOLATION_REGEXP = /\{\{([\s\S]*?)\}\}/g;
+var COMMENT_REGEX = /\/\//g;
 
 class ParseException extends BaseException {
   constructor(message: string, input: string, errLocation: string, ctxLocation?: any) {
@@ -73,7 +74,7 @@ export class Parser {
 
   parseAction(input: string, location: any): ASTWithSource {
     this._checkNoInterpolation(input, location);
-    var tokens = this._lexer.tokenize(input);
+    var tokens = this._lexer.tokenize(this._stripComments(input));
     var ast = new _ParseAST(input, location, tokens, this._reflector, true).parseChain();
     return new ASTWithSource(ast, input, location);
   }
@@ -102,7 +103,7 @@ export class Parser {
     }
 
     this._checkNoInterpolation(input, location);
-    var tokens = this._lexer.tokenize(input);
+    var tokens = this._lexer.tokenize(this._stripComments(input));
     return new _ParseAST(input, location, tokens, this._reflector, false).parseChain();
   }
 
@@ -128,7 +129,7 @@ export class Parser {
     let expressions = [];
 
     for (let i = 0; i < split.expressions.length; ++i) {
-      var tokens = this._lexer.tokenize(split.expressions[i]);
+      var tokens = this._lexer.tokenize(this._stripComments(split.expressions[i]));
       var ast = new _ParseAST(input, location, tokens, this._reflector, false).parseChain();
       expressions.push(ast);
     }
@@ -164,6 +165,10 @@ export class Parser {
     return new ASTWithSource(new LiteralPrimitive(input), input, location);
   }
 
+  private _stripComments(input: string): string {
+    return StringWrapper.split(input, COMMENT_REGEX)[0].trim();
+  }
+
   private _checkNoInterpolation(input: string, location: any): void {
     var parts = StringWrapper.split(input, INTERPOLATION_REGEXP);
     if (parts.length > 1) {

diff --git a/modules/angular2/src/i18n/i18n_html_parser.ts b/modules/angular2/src/i18n/i18n_html_parser.ts
@@ -22,14 +22,16 @@ import {
   partition,
   Part,
   stringifyNodes,
-  meaning
+  meaning,
+  getPhNameFromBinding,
+  dedupePhName
 } from './shared';
 
 const _I18N_ATTR = "i18n";
 const _PLACEHOLDER_ELEMENT = "ph";
 const _NAME_ATTR = "name";
 const _I18N_ATTR_PREFIX = "i18n-";
-let _PLACEHOLDER_EXPANDED_REGEXP = RegExpWrapper.create(`\\<ph(\\s)+name=("(\\d)+")\\>\\<\\/ph\\>`);
+let _PLACEHOLDER_EXPANDED_REGEXP = RegExpWrapper.create(`\\<ph(\\s)+name=("(\\w)+")\\>\\<\\/ph\\>`);
 
 /**
  * Creates an i18n-ed version of the parsed template.
@@ -313,19 +315,31 @@ export class I18nHtmlParser implements HtmlParser {
 
   private _replacePlaceholdersWithExpressions(message: string, exps: string[],
                                               sourceSpan: ParseSourceSpan): string {
+    let expMap = this._buildExprMap(exps);
     return RegExpWrapper.replaceAll(_PLACEHOLDER_EXPANDED_REGEXP, message, (match) => {
       let nameWithQuotes = match[2];
       let name = nameWithQuotes.substring(1, nameWithQuotes.length - 1);
-      let index = NumberWrapper.parseInt(name, 10);
-      return this._convertIntoExpression(index, exps, sourceSpan);
+      return this._convertIntoExpression(name, expMap, sourceSpan);
     });
   }
 
-  private _convertIntoExpression(index: number, exps: string[], sourceSpan: ParseSourceSpan) {
-    if (index >= 0 && index < exps.length) {
-      return `{{${exps[index]}}}`;
+  private _buildExprMap(exps: string[]): Map<string, string> {
+    let expMap = new Map<string, string>();
+    let usedNames = new Map<string, number>();
+
+    for (var i = 0; i < exps.length; i++) {
+      let phName = getPhNameFromBinding(exps[i], i);
+      expMap.set(dedupePhName(usedNames, phName), exps[i]);
+    }
+    return expMap;
+  }
+
+  private _convertIntoExpression(name: string, expMap: Map<string, string>,
+                                 sourceSpan: ParseSourceSpan) {
+    if (expMap.has(name)) {
+      return `{{${expMap.get(name)}}}`;
     } else {
-      throw new I18nError(sourceSpan, `Invalid interpolation index '${index}'`);
+      throw new I18nError(sourceSpan, `Invalid interpolation name '${name}'`);
     }
   }
 }
@@ -347,4 +361,4 @@ class _CreateNodeMapping implements HtmlAstVisitor {
   }
 
   visitComment(ast: HtmlCommentAst, context: any): any { return ""; }
-}
+}
diff --git a/modules/angular2/src/i18n/shared.ts b/modules/angular2/src/i18n/shared.ts
@@ -8,12 +8,13 @@ import {
   HtmlCommentAst,
   htmlVisitAll
 } from 'angular2/src/compiler/html_ast';
-import {isPresent, isBlank} from 'angular2/src/facade/lang';
+import {isPresent, isBlank, StringWrapper} from 'angular2/src/facade/lang';
 import {Message} from './message';
 import {Parser} from 'angular2/src/core/change_detection/parser/parser';
 
 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;
 
 /**
  * An i18n error.
@@ -113,12 +114,15 @@ export function removeInterpolation(value: string, source: ParseSourceSpan,
                                     parser: Parser): string {
   try {
     let parsed = parser.splitInterpolation(value, source.toString());
+    let usedNames = new Map<string, number>();
     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 += `<ph name="${i}"/>`;
+          let customPhName = getPhNameFromBinding(parsed.expressions[i], i);
+          customPhName = dedupePhName(usedNames, customPhName);
+          res += `<ph name="${customPhName}"/>`;
         }
       }
       return res;
@@ -130,6 +134,22 @@ export function removeInterpolation(value: string, source: ParseSourceSpan,
   }
 }
 
+export function getPhNameFromBinding(input: string, index: number): string {
+  let customPhMatch = StringWrapper.split(input, CUSTOM_PH_EXP);
+  return customPhMatch.length > 1 ? customPhMatch[1] : `${index}`;
+}
+
+export function dedupePhName(usedNames: Map<string, number>, name: string): string {
+  let duplicateNameCount = usedNames.get(name);
+  if (isPresent(duplicateNameCount)) {
+    usedNames.set(name, duplicateNameCount + 1);
+    return `${name}_${duplicateNameCount}`;
+  } else {
+    usedNames.set(name, 1);
+    return name;
+  }
+}
+
 export function stringifyNodes(nodes: HtmlAst[], parser: Parser): string {
   let visitor = new _StringifyVisitor(parser);
   return htmlVisitAll(visitor, nodes).join("");

diff --git a/modules/angular2/test/core/change_detection/parser/parser_spec.ts b/modules/angular2/test/core/change_detection/parser/parser_spec.ts
@@ -104,6 +104,8 @@ export function main() {
 
       it('should parse grouped expressions', () => { checkAction("(1 + 2) * 3", "1 + 2 * 3"); });
 
+      it('should ignore comments in expressions', () => { checkAction('a //comment', 'a'); });
+
       it('should parse an empty string', () => { checkAction(''); });
 
       describe("literals", () => {
@@ -270,6 +272,8 @@ export function main() {
       });
 
       it('should parse conditional expression', () => { checkBinding('a < b ? a : b'); });
+
+      it('should ignore comments in bindings', () => { checkBinding('a //comment', 'a'); });
     });
 
     describe('parseTemplateBindings', () => {
@@ -425,6 +429,9 @@ export function main() {
       it('should parse expression with newline characters', () => {
         checkInterpolation(`{{ 'foo' +\n 'bar' +\r 'baz' }}`, `{{ "foo" + "bar" + "baz" }}`);
       });
+
+      it('should ignore comments in interpolation expressions',
+         () => { checkInterpolation('{{a //comment}}', '{{ a }}'); });
     });
 
     describe("parseSimpleBinding", () => {

diff --git a/modules/angular2/test/i18n/i18n_html_parser_spec.ts b/modules/angular2/test/i18n/i18n_html_parser_spec.ts
@@ -76,6 +76,36 @@ export function main() {
           .toEqual([[HtmlElementAst, 'div', 0], [HtmlAttrAst, 'value', '{{b}} or {{a}}']]);
     });
 
+    it('should handle interpolation with custom placeholder names', () => {
+      let translations: {[key: string]: string} = {};
+      translations[id(new Message('<ph name="FIRST"/> and <ph name="SECOND"/>', null, null))] =
+          '<ph name="SECOND"/> or <ph name="FIRST"/>';
+
+      expect(
+          humanizeDom(parse(
+              `<div value='{{a //i18n(ph="FIRST")}} and {{b //i18n(ph="SECOND")}}' i18n-value></div>`,
+              translations)))
+          .toEqual([
+            [HtmlElementAst, 'div', 0],
+            [HtmlAttrAst, 'value', '{{b //i18n(ph="SECOND")}} or {{a //i18n(ph="FIRST")}}']
+          ]);
+    });
+
+    it('should handle interpolation with duplicate placeholder names', () => {
+      let translations: {[key: string]: string} = {};
+      translations[id(new Message('<ph name="FIRST"/> and <ph name="FIRST_1"/>', null, null))] =
+          '<ph name="FIRST_1"/> or <ph name="FIRST"/>';
+
+      expect(
+          humanizeDom(parse(
+              `<div value='{{a //i18n(ph="FIRST")}} and {{b //i18n(ph="FIRST")}}' i18n-value></div>`,
+              translations)))
+          .toEqual([
+            [HtmlElementAst, 'div', 0],
+            [HtmlAttrAst, 'value', '{{b //i18n(ph="FIRST")}} or {{a //i18n(ph="FIRST")}}']
+          ]);
+    });
+
     it("should handle nested html", () => {
       let translations: {[key: string]: string} = {};
       translations[id(new Message('<ph name="e0">a</ph><ph name="e2">b</ph>', null, null))] =
@@ -198,7 +228,7 @@ export function main() {
 
         expect(
             humanizeErrors(parse("<div value='hi {{a}}' i18n-value></div>", translations).errors))
-            .toEqual(["Invalid interpolation index '99'"]);
+            .toEqual(["Invalid interpolation name '99'"]);
       });
 
     });
@@ -207,4 +237,4 @@ export function main() {
 
 function humanizeErrors(errors: ParseError[]): string[] {
   return errors.map(error => error.msg);
-}
+}
diff --git a/modules/angular2/test/i18n/message_extractor_spec.ts b/modules/angular2/test/i18n/message_extractor_spec.ts
@@ -93,6 +93,47 @@ export function main() {
           .toEqual([new Message('Hi <ph name="0"/> and <ph name="1"/>', null, null)]);
     });
 
+    it('should replace interpolation with named placeholders if provided (text nodes)', () => {
+      let res = extractor.extract(`
+        <div i18n>Hi {{one //i18n(ph="FIRST")}} and {{two //i18n(ph="SECOND")}}</div>`,
+                                  'someurl');
+      expect(res.messages)
+          .toEqual([
+            new Message('<ph name="t0">Hi <ph name="FIRST"/> and <ph name="SECOND"/></ph>', null,
+                        null)
+          ]);
+    });
+
+    it('should replace interpolation with named placeholders if provided (attributes)', () => {
+      let res = extractor.extract(`
+      <div title='Hi {{one //i18n(ph="FIRST")}} and {{two //i18n(ph="SECOND")}}'
+        i18n-title></div>`,
+                                  'someurl');
+      expect(res.messages)
+          .toEqual([new Message('Hi <ph name="FIRST"/> and <ph name="SECOND"/>', null, null)]);
+    });
+
+    it('should match named placeholders with extra spacing', () => {
+      let res = extractor.extract(`
+      <div title='Hi {{one // i18n ( ph = "FIRST" )}} and {{two // i18n ( ph = "SECOND" )}}'
+        i18n-title></div>`,
+                                  'someurl');
+      expect(res.messages)
+          .toEqual([new Message('Hi <ph name="FIRST"/> and <ph name="SECOND"/>', null, null)]);
+    });
+
+    it('should suffix duplicate placeholder names with numbers', () => {
+      let res = extractor.extract(`
+      <div title='Hi {{one //i18n(ph="FIRST")}} and {{two //i18n(ph="FIRST")}} and {{three //i18n(ph="FIRST")}}'
+        i18n-title></div>`,
+                                  'someurl');
+      expect(res.messages)
+          .toEqual([
+            new Message('Hi <ph name="FIRST"/> and <ph name="FIRST_1"/> and <ph name="FIRST_2"/>',
+                        null, null)
+          ]);
+    });
+
     it("should handle html content", () => {
       let res = extractor.extract(
           '<div i18n><div attr="value">zero<div>one</div></div><div>two</div></div>', "someurl");