Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(compiler): i18n - trim whitespace from i18n metadata #34154

Closed
wants to merge 3 commits into from
Closed

fix(compiler): i18n - trim whitespace from i18n metadata #34154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f3152e7
fix(compiler): i18n - trim whitespace from i18n custom ids
petebacondarwin Dec 3, 2019
452589d
refactor(compiler): remove redundant `metaFromI18nMessage()` function
petebacondarwin Dec 3, 2019
6818261
fix(ivy): i18n - trim whitespace when parsing metadata
petebacondarwin Dec 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/compiler/src/i18n/extractor_merger.ts
View file
Open in desktop
Expand Up @@ -516,5 +516,5 @@ function _parseMessageMeta(i18n?: string): {meaning: string, description: string
[meaningAndDesc.slice(0, descIndex), meaningAndDesc.slice(descIndex + 1)] :
['', meaningAndDesc];

return {meaning, description, id};
return {meaning, description, id: id.trim()};
}
4 changes: 2 additions & 2 deletions packages/compiler/src/render3/view/i18n/get_msg_utils.ts
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@ import {mapLiteral} from '../../../output/map_util';
import * as o from '../../../output/output_ast';

import {serializeIcuNode} from './icu_serializer';
import {i18nMetaToDocStmt, metaFromI18nMessage} from './meta';
import {i18nMetaToDocStmt} from './meta';
import {formatI18nPlaceholderName} from './util';

/** Closure uses `goog.getMsg(message)` to lookup translations */
Expand All @@ -32,7 +32,7 @@ export function createGoogleGetMsgStatements(
// const MSG_... = goog.getMsg(..);
// I18N_X = MSG_...;
const statements = [];
const jsdocComment = i18nMetaToDocStmt(metaFromI18nMessage(message));
const jsdocComment = i18nMetaToDocStmt(message);
if (jsdocComment !== null) {
statements.push(jsdocComment);
}
Expand Down
6 changes: 2 additions & 4 deletions packages/compiler/src/render3/view/i18n/localize_utils.ts
View file
Open in desktop
Expand Up @@ -9,7 +9,6 @@ import * as i18n from '../../../i18n/i18n_ast';
import * as o from '../../../output/output_ast';

import {serializeIcuNode} from './icu_serializer';
import {metaFromI18nMessage} from './meta';
import {formatI18nPlaceholderName} from './util';

export function createLocalizeStatements(
Expand All @@ -18,9 +17,8 @@ export function createLocalizeStatements(
const statements = [];

const {messageParts, placeHolders} = serializeI18nMessageForLocalize(message);
statements.push(new o.ExpressionStatement(variable.set(o.localizedString(
metaFromI18nMessage(message), messageParts, placeHolders,
placeHolders.map(ph => params[ph])))));
statements.push(new o.ExpressionStatement(variable.set(
o.localizedString(message, messageParts, placeHolders, placeHolders.map(ph => params[ph])))));

return statements;
}
Expand Down
15 changes: 3 additions & 12 deletions packages/compiler/src/render3/view/i18n/meta.ts
View file
Open in desktop
Expand Up @@ -153,7 +153,7 @@ export class I18nMetaVisitor implements html.Visitor {
*/
private _parseMetadata(meta: string|i18n.I18nMeta): I18nMeta {
return typeof meta === 'string' ? parseI18nMeta(meta) :
meta instanceof i18n.Message ? metaFromI18nMessage(meta) : {};
meta instanceof i18n.Message ? meta : {};
}

/**
Expand Down Expand Up @@ -191,16 +191,6 @@ export class I18nMetaVisitor implements html.Visitor {
}
}

export function metaFromI18nMessage(message: i18n.Message, id: string | null = null): I18nMeta {
return {
id: typeof id === 'string' ? id : message.id || '',
customId: message.customId,
legacyId: message.legacyId,
meaning: message.meaning || '',
description: message.description || ''
};
}

/** I18n separators for metadata **/
const I18N_MEANING_SEPARATOR = '|';
const I18N_ID_SEPARATOR = '@@';
Expand All @@ -215,11 +205,12 @@ const I18N_ID_SEPARATOR = '@@';
* @param meta String that represents i18n meta
* @returns Object with id, meaning and description fields
*/
export function parseI18nMeta(meta?: string): I18nMeta {
export function parseI18nMeta(meta: string = ''): I18nMeta {
let customId: string|undefined;
let meaning: string|undefined;
let description: string|undefined;

meta = meta.trim();
if (meta) {
const idIndex = meta.indexOf(I18N_ID_SEPARATOR);
const descIndex = meta.indexOf(I18N_MEANING_SEPARATOR);
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler/test/i18n/extractor_merger_spec.ts
View file
Open in desktop
Expand Up @@ -51,6 +51,12 @@ import {serializeNodes as serializeHtmlNodes} from '../ml_parser/util/util';
]);
});

it('should trim whitespace from custom ids (but not meanings)', () => {
expect(extract('<div i18n="\n m1|d1@@i1\n ">test</div>')).toEqual([
[['test'], '\n m1', 'd1', 'i1'],
]);
});

it('should extract from attributes without meaning and with id', () => {
expect(
extract(
Expand Down
82 changes: 44 additions & 38 deletions packages/compiler/test/i18n/i18n_parser_spec.ts
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
describe('elements', () => {
it('should extract from elements', () => {
expect(_humanizeMessages('<div i18n="m|d">text</div>')).toEqual([
[['text'], 'm', 'd'],
[['text'], 'm', 'd', ''],
]);
});

Expand All @@ -29,7 +29,7 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
'text',
'<ph tag name="START_TAG_SPAN"><ph tag name="START_BOLD_TEXT">nested</ph name="CLOSE_BOLD_TEXT"></ph name="CLOSE_TAG_SPAN">'
],
'm', 'd'
'm', 'd', ''
],
]);
});
Expand All @@ -46,16 +46,22 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
[
'<ph tag name="START_PARAGRAPH"><ph tag name="LINE_BREAK"/></ph name="CLOSE_PARAGRAPH">'
],
'm', 'd'
'm', 'd', ''
],
]);
});

it('should trim whitespace from custom ids (but not meanings)', () => {
expect(_humanizeMessages('<div i18n="\n m|d@@id\n ">text</div>')).toEqual([
[['text'], '\n m', 'd', 'id'],
]);
});
});

describe('attributes', () => {
it('should extract from attributes outside of translatable section', () => {
expect(_humanizeMessages('<div i18n-title="m|d" title="msg"></div>')).toEqual([
[['msg'], 'm', 'd'],
[['msg'], 'm', 'd', ''],
]);
});

Expand All @@ -66,22 +72,22 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
[
'<ph tag name="START_PARAGRAPH"><ph tag name="START_BOLD_TEXT"></ph name="CLOSE_BOLD_TEXT"></ph name="CLOSE_PARAGRAPH">'
],
'', ''
'', '', ''
],
[['msg'], 'm', 'd'],
[['msg'], 'm', 'd', ''],
]);
});

it('should extract from attributes in translatable block', () => {
expect(_humanizeMessages(
'<!-- i18n --><p><b i18n-title="m|d" title="msg"></b></p><!-- /i18n -->'))
.toEqual([
[['msg'], 'm', 'd'],
[['msg'], 'm', 'd', ''],
[
[
'<ph tag name="START_PARAGRAPH"><ph tag name="START_BOLD_TEXT"></ph name="CLOSE_BOLD_TEXT"></ph name="CLOSE_PARAGRAPH">'
],
'', ''
'', '', ''
],
]);
});
Expand All @@ -91,12 +97,12 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
_humanizeMessages(
'<!-- i18n -->{count, plural, =0 {<p><b i18n-title="m|d" title="msg"></b></p>}}<!-- /i18n -->'))
.toEqual([
[['msg'], 'm', 'd'],
[['msg'], 'm', 'd', ''],
[
[
'{count, plural, =0 {[<ph tag name="START_PARAGRAPH"><ph tag name="START_BOLD_TEXT"></ph name="CLOSE_BOLD_TEXT"></ph name="CLOSE_PARAGRAPH">]}}'
],
'', ''
'', '', ''
],
]);
});
Expand All @@ -105,7 +111,7 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
expect(
_humanizeMessages('{count, plural, =0 {<p><b i18n-title="m|d" title="msg"></b></p>}}'))
.toEqual([
[['msg'], 'm', 'd'],
[['msg'], 'm', 'd', ''],
]);
});

Expand All @@ -116,20 +122,20 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
describe('interpolation', () => {
it('should replace interpolation with placeholder', () => {
expect(_humanizeMessages('<div i18n="m|d">before{{ exp }}after</div>')).toEqual([
[['[before, <ph name="INTERPOLATION"> exp </ph>, after]'], 'm', 'd'],
[['[before, <ph name="INTERPOLATION"> exp </ph>, after]'], 'm', 'd', ''],
]);
});

it('should support named interpolation', () => {
expect(_humanizeMessages('<div i18n="m|d">before{{ exp //i18n(ph="teSt") }}after</div>'))
.toEqual([
[['[before, <ph name="TEST"> exp //i18n(ph="teSt") </ph>, after]'], 'm', 'd'],
[['[before, <ph name="TEST"> exp //i18n(ph="teSt") </ph>, after]'], 'm', 'd', ''],
]);

expect(
_humanizeMessages('<div i18n=\'m|d\'>before{{ exp //i18n(ph=\'teSt\') }}after</div>'))
.toEqual([
[[`[before, <ph name="TEST"> exp //i18n(ph='teSt') </ph>, after]`], 'm', 'd'],
[[`[before, <ph name="TEST"> exp //i18n(ph='teSt') </ph>, after]`], 'm', 'd', ''],
]);
});
});
Expand All @@ -140,9 +146,9 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
<!-- i18n: desc2 -->message2<!-- /i18n -->
<!-- i18n -->message3<!-- /i18n -->`))
.toEqual([
[['message1'], 'meaning1', 'desc1'],
[['message2'], '', 'desc2'],
[['message3'], '', ''],
[['message1'], 'meaning1', 'desc1', ''],
[['message2'], '', 'desc2', ''],
[['message3'], '', '', ''],
]);
});

Expand All @@ -153,7 +159,7 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
'text',
'<ph tag name="START_PARAGRAPH">html, <ph tag name="START_BOLD_TEXT">nested</ph name="CLOSE_BOLD_TEXT"></ph name="CLOSE_PARAGRAPH">'
],
'', ''
'', '', ''
],
]);
});
Expand All @@ -162,36 +168,36 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
describe('ICU messages', () => {
it('should extract as ICU when single child of an element', () => {
expect(_humanizeMessages('<div i18n="m|d">{count, plural, =0 {zero}}</div>')).toEqual([
[['{count, plural, =0 {[zero]}}'], 'm', 'd'],
[['{count, plural, =0 {[zero]}}'], 'm', 'd', ''],
]);
});

it('should extract as ICU + ph when not single child of an element', () => {
expect(_humanizeMessages('<div i18n="m|d">b{count, plural, =0 {zero}}a</div>')).toEqual([
[['b', '<ph icu name="ICU">{count, plural, =0 {[zero]}}</ph>', 'a'], 'm', 'd'],
[['{count, plural, =0 {[zero]}}'], '', ''],
[['b', '<ph icu name="ICU">{count, plural, =0 {[zero]}}</ph>', 'a'], 'm', 'd', ''],
[['{count, plural, =0 {[zero]}}'], '', '', ''],
]);
});

it('should extract as ICU + ph when wrapped in whitespace in an element', () => {
expect(_humanizeMessages('<div i18n="m|d"> {count, plural, =0 {zero}} </div>')).toEqual([
[[' ', '<ph icu name="ICU">{count, plural, =0 {[zero]}}</ph>', ' '], 'm', 'd'],
[['{count, plural, =0 {[zero]}}'], '', ''],
[[' ', '<ph icu name="ICU">{count, plural, =0 {[zero]}}</ph>', ' '], 'm', 'd', ''],
[['{count, plural, =0 {[zero]}}'], '', '', ''],
]);
});

it('should extract as ICU when single child of a block', () => {
expect(_humanizeMessages('<!-- i18n:m|d -->{count, plural, =0 {zero}}<!-- /i18n -->'))
.toEqual([
[['{count, plural, =0 {[zero]}}'], 'm', 'd'],
[['{count, plural, =0 {[zero]}}'], 'm', 'd', ''],
]);
});

it('should extract as ICU + ph when not single child of a block', () => {
expect(_humanizeMessages('<!-- i18n:m|d -->b{count, plural, =0 {zero}}a<!-- /i18n -->'))
.toEqual([
[['{count, plural, =0 {[zero]}}'], '', ''],
[['b', '<ph icu name="ICU">{count, plural, =0 {[zero]}}</ph>', 'a'], 'm', 'd'],
[['{count, plural, =0 {[zero]}}'], '', '', ''],
[['b', '<ph icu name="ICU">{count, plural, =0 {[zero]}}</ph>', 'a'], 'm', 'd', ''],
]);
});

Expand All @@ -204,17 +210,17 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
'b', '<ph icu name="ICU">{count, plural, =0 {[{sex, select, male {[m]}}]}}</ph>',
'a'
],
'm', 'd'
'm', 'd', ''
],
[['{count, plural, =0 {[{sex, select, male {[m]}}]}}'], '', ''],
[['{count, plural, =0 {[{sex, select, male {[m]}}]}}'], '', '', ''],
]);
});
});

describe('implicit elements', () => {
it('should extract from implicit elements', () => {
expect(_humanizeMessages('<b>bold</b><i>italic</i>', ['b'])).toEqual([
[['bold'], '', ''],
[['bold'], '', '', ''],
]);
});
});
Expand All @@ -224,7 +230,7 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
expect(_humanizeMessages(
'<b title="bb">bold</b><i title="ii">italic</i>', [], {'b': ['title']}))
.toEqual([
[['bb'], '', ''],
[['bb'], '', '', ''],
]);
});
});
Expand All @@ -239,7 +245,7 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
'<ph tag name="START_PARAGRAPH">two</ph name="CLOSE_PARAGRAPH">',
'<ph tag name="START_PARAGRAPH_1">three</ph name="CLOSE_PARAGRAPH">',
],
'm', 'd'
'm', 'd', ''
],
]);

Expand All @@ -256,7 +262,7 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
[
'[<ph name="INTERPOLATION"> a </ph>, <ph name="INTERPOLATION"> a </ph>, <ph name="INTERPOLATION_1"> b </ph>]'
],
'm', 'd'
'm', 'd', ''
],
]);

Expand All @@ -276,11 +282,11 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte
'<ph icu name="ICU">{count, plural, =0 {[0]}}</ph>',
'<ph icu name="ICU_1">{count, plural, =1 {[1]}}</ph>',
],
'm', 'd'
'm', 'd', ''
],
[['{count, plural, =0 {[0]}}'], '', ''],
[['{count, plural, =0 {[0]}}'], '', ''],
[['{count, plural, =1 {[1]}}'], '', ''],
[['{count, plural, =0 {[0]}}'], '', '', ''],
[['{count, plural, =0 {[0]}}'], '', '', ''],
[['{count, plural, =1 {[1]}}'], '', '', ''],
]);

expect(_humanizePlaceholders(html)).toEqual([
Expand All @@ -304,11 +310,11 @@ import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler/src/ml_parser/inte

export function _humanizeMessages(
html: string, implicitTags: string[] = [],
implicitAttrs: {[k: string]: string[]} = {}): [string[], string, string][] {
implicitAttrs: {[k: string]: string[]} = {}): [string[], string, string, string][] {
// clang-format off
// https://github.com/angular/clang-format/issues/35
return _extractMessages(html, implicitTags, implicitAttrs).map(
message => [serializeNodes(message.nodes), message.meaning, message.description, ]) as [string[], string, string][];
message => [serializeNodes(message.nodes), message.meaning, message.description, message.id]) as [string[], string, string, string][];
// clang-format on
}

Expand Down
7 changes: 7 additions & 0 deletions packages/compiler/test/render3/view/i18n_spec.ts
View file
Open in desktop
Expand Up @@ -207,6 +207,13 @@ describe('Utils', () => {
expect(parseI18nMeta('meaning|desc')).toEqual(meta('', 'meaning', 'desc'));
expect(parseI18nMeta('meaning|desc@@id')).toEqual(meta('id', 'meaning', 'desc'));
expect(parseI18nMeta('@@id')).toEqual(meta('id', '', ''));

expect(parseI18nMeta('\n ')).toEqual(meta());
expect(parseI18nMeta('\n desc\n ')).toEqual(meta('', '', 'desc'));
expect(parseI18nMeta('\n desc@@id\n ')).toEqual(meta('id', '', 'desc'));
expect(parseI18nMeta('\n meaning|desc\n ')).toEqual(meta('', 'meaning', 'desc'));
expect(parseI18nMeta('\n meaning|desc@@id\n ')).toEqual(meta('id', 'meaning', 'desc'));
expect(parseI18nMeta('\n @@id\n ')).toEqual(meta('id', '', ''));
});

it('serializeI18nHead()', () => {
Expand Down