Skip to content

Commit

Permalink
fix(localize): serialize all the message locations to XLIFF (#39411)
Browse files Browse the repository at this point in the history 
Previously only the first message, for each id, was serialized
which meant that additional message location information
was lost.

Now all the message locations are included in the serialized
messages.

Fixes #39330

PR Close #39411
  • Loading branch information
@petebacondarwin @alxhub
petebacondarwin authored and alxhub committed Oct 26, 2020
1 parent e8e3771 commit f5710c6
Show file tree
Hide file tree
Showing 5 changed files with 237 additions and 21 deletions.
37 changes: 37 additions & 0 deletions packages/localize/src/tools/src/extract/translation_files/utils.ts
View file
@@ -0,0 +1,37 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {ɵMessageId, ɵParsedMessage, ɵSourceLocation} from '@angular/localize';

/**
* Consolidate an array of messages into a map from message id to an array of messages with that id.
*
* @param messages the messages to consolidate.
* @param getMessageId a function that will compute the message id of a message.
*/
export function consolidateMessages(
messages: ɵParsedMessage[],
getMessageId: (message: ɵParsedMessage) => string): Map<ɵMessageId, ɵParsedMessage[]> {
const consolidateMessages = new Map<ɵMessageId, ɵParsedMessage[]>();
for (const message of messages) {
const id = getMessageId(message);
if (!consolidateMessages.has(id)) {
consolidateMessages.set(id, [message]);
} else {
consolidateMessages.get(id)!.push(message);
}
}
return consolidateMessages;
}

/**
* Does the given message have a location property?
*/
export function hasLocation(message: ɵParsedMessage): message is ɵParsedMessage&
{location: ɵSourceLocation} {
return message.location !== undefined;
}
19 changes: 9 additions & 10 deletions packages/localize/src/tools/src/extract/translation_files/xliff1_translation_serializer.ts
View file
Expand Up @@ -11,6 +11,7 @@ import {ɵParsedMessage, ɵSourceLocation} from '@angular/localize';
import {FormatOptions, validateOptions} from './format_options';
import {extractIcuPlaceholders} from './icu_parsing';
import {TranslationSerializer} from './translation_serializer';
import {consolidateMessages, hasLocation} from './utils';
import {XmlFile} from './xml_file';

/** This is the number of characters that a legacy Xliff 1.2 message id has. */
Expand All @@ -33,7 +34,7 @@ export class Xliff1TranslationSerializer implements TranslationSerializer {
}

serialize(messages: ɵParsedMessage[]): string {
const ids = new Set<string>();
const messageMap = consolidateMessages(messages, message => this.getMessageId(message));
const xml = new XmlFile();
xml.startTag('xliff', {'version': '1.2', 'xmlns': 'urn:oasis:names:tc:xliff:document:1.2'});
// NOTE: the `original` property is set to the legacy `ng2.template` value for backward
Expand All @@ -50,21 +51,19 @@ export class Xliff1TranslationSerializer implements TranslationSerializer {
...this.formatOptions,
});
xml.startTag('body');
for (const message of messages) {
const id = this.getMessageId(message);
if (ids.has(id)) {
// Do not render the same message more than once
continue;
}
ids.add(id);
for (const [id, duplicateMessages] of messageMap.entries()) {
const message = duplicateMessages[0];

xml.startTag('trans-unit', {id, datatype: 'html'});
xml.startTag('source', {}, {preserveWhitespace: true});
this.serializeMessage(xml, message);
xml.endTag('source', {preserveWhitespace: false});
if (message.location) {
this.serializeLocation(xml, message.location);

// Write all the locations
for (const {location} of duplicateMessages.filter(hasLocation)) {
this.serializeLocation(xml, location);
}

if (message.description) {
this.serializeNote(xml, 'description', message.description);
}
Expand Down
22 changes: 11 additions & 11 deletions packages/localize/src/tools/src/extract/translation_files/xliff2_translation_serializer.ts
View file
Expand Up @@ -11,6 +11,7 @@ import {ɵParsedMessage, ɵSourceLocation} from '@angular/localize';
import {FormatOptions, validateOptions} from './format_options';
import {extractIcuPlaceholders} from './icu_parsing';
import {TranslationSerializer} from './translation_serializer';
import {consolidateMessages, hasLocation} from './utils';
import {XmlFile} from './xml_file';

/** This is the maximum number of characters that can appear in a legacy XLIFF 2.0 message id. */
Expand All @@ -33,7 +34,7 @@ export class Xliff2TranslationSerializer implements TranslationSerializer {
}

serialize(messages: ɵParsedMessage[]): string {
const ids = new Set<string>();
const messageMap = consolidateMessages(messages, message => this.getMessageId(message));
const xml = new XmlFile();
xml.startTag('xliff', {
'version': '2.0',
Expand All @@ -48,24 +49,23 @@ export class Xliff2TranslationSerializer implements TranslationSerializer {
// messages that come from a particular original file, and the translation file parsers may
// not
xml.startTag('file', {'id': 'ngi18n', 'original': 'ng.template', ...this.formatOptions});
for (const message of messages) {
const id = this.getMessageId(message);
if (ids.has(id)) {
// Do not render the same message more than once
continue;
}
ids.add(id);
for (const [id, duplicateMessages] of messageMap.entries()) {
const message = duplicateMessages[0];

xml.startTag('unit', {id});
if (message.meaning || message.description || message.location) {
const messagesWithLocations = duplicateMessages.filter(hasLocation);
if (message.meaning || message.description || messagesWithLocations.length) {
xml.startTag('notes');
if (message.location) {
const {file, start, end} = message.location;

// Write all the locations
for (const {location: {file, start, end}} of messagesWithLocations) {
const endLineString =
end !== undefined && end.line !== start.line ? `,${end.line + 1}` : '';
this.serializeNote(
xml, 'location',
`${this.fs.relative(this.basePath, file)}:${start.line + 1}${endLineString}`);
}

if (message.description) {
this.serializeNote(xml, 'description', message.description);
}
Expand Down
95 changes: 95 additions & 0 deletions ...s/localize/src/tools/test/extract/translation_files/xliff1_translation_serializer_spec.ts
View file
Expand Up @@ -135,6 +135,101 @@ runInEachFileSystem(() => {
`</xliff>\n`,
].join('\n'));
});

it('should convert a set of parsed messages into an XML string', () => {
const messageLocation1: ɵSourceLocation = {
start: {line: 0, column: 5},
end: {line: 0, column: 10},
file: absoluteFrom('/project/file-1.ts'),
text: 'message text'
};

const messageLocation2: ɵSourceLocation = {
start: {line: 3, column: 2},
end: {line: 4, column: 7},
file: absoluteFrom('/project/file-2.ts'),
text: 'message text'
};

const messageLocation3: ɵSourceLocation = {
start: {line: 0, column: 5},
end: {line: 0, column: 10},
file: absoluteFrom('/project/file-3.ts'),
text: 'message text'
};

const messageLocation4: ɵSourceLocation = {
start: {line: 3, column: 2},
end: {line: 4, column: 7},
file: absoluteFrom('/project/file-4.ts'),
text: 'message text'
};

const messages: ɵParsedMessage[] = [
mockMessage('1234', ['message text'], [], {location: messageLocation1}),
mockMessage('1234', ['message text'], [], {location: messageLocation2}),
mockMessage('1234', ['message text'], [], {
location: messageLocation3,
legacyIds: ['87654321FEDCBA0987654321FEDCBA0987654321', '563965274788097516']
}),
mockMessage(
'1234', ['message text'], [], {location: messageLocation4, customId: 'other'}),
];
const serializer = new Xliff1TranslationSerializer(
'xx', absoluteFrom('/project'), useLegacyIds, options);
const output = serializer.serialize(messages);

// Note that in this test the third message will match the first two if legacyIds is
// false. Otherwise it will be a separate message on its own.

expect(output).toEqual([
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">`,
` <file source-language="xx" datatype="plaintext" original="ng2.template"${
toAttributes(options)}>`,
` <body>`,
` <trans-unit id="1234" datatype="html">`,
` <source>message text</source>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-1.ts</context>`,
` <context context-type="linenumber">1</context>`,
` </context-group>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-2.ts</context>`,
` <context context-type="linenumber">4,5</context>`,
` </context-group>`,
...useLegacyIds ?
[] :
[
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-3.ts</context>`,
` <context context-type="linenumber">1</context>`,
` </context-group>`,
],
` </trans-unit>`,
...useLegacyIds ?
[
` <trans-unit id="87654321FEDCBA0987654321FEDCBA0987654321" datatype="html">`,
` <source>message text</source>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-3.ts</context>`,
` <context context-type="linenumber">1</context>`,
` </context-group>`,
` </trans-unit>`,
] :
[],
` <trans-unit id="other" datatype="html">`,
` <source>message text</source>`,
` <context-group purpose="location">`,
` <context context-type="sourcefile">file-4.ts</context>`,
` <context context-type="linenumber">4,5</context>`,
` </context-group>`,
` </trans-unit>`,
` </body>`,
` </file>`,
`</xliff>\n`,
].join('\n'));
});
});
});
});
Expand Down
85 changes: 85 additions & 0 deletions ...s/localize/src/tools/test/extract/translation_files/xliff2_translation_serializer_spec.ts
View file
Expand Up @@ -158,6 +158,91 @@ runInEachFileSystem(() => {
`</xliff>\n`,
].join('\n'));
});

it('should convert a set of parsed messages into an XML string', () => {
const messageLocation1: ɵSourceLocation = {
start: {line: 0, column: 5},
end: {line: 0, column: 10},
file: absoluteFrom('/project/file-1.ts'),
text: 'message text'
};

const messageLocation2: ɵSourceLocation = {
start: {line: 3, column: 2},
end: {line: 4, column: 7},
file: absoluteFrom('/project/file-2.ts'),
text: 'message text'
};

const messageLocation3: ɵSourceLocation = {
start: {line: 0, column: 5},
end: {line: 0, column: 10},
file: absoluteFrom('/project/file-3.ts'),
text: 'message text'
};

const messageLocation4: ɵSourceLocation = {
start: {line: 3, column: 2},
end: {line: 4, column: 7},
file: absoluteFrom('/project/file-4.ts'),
text: 'message text'
};

const messages: ɵParsedMessage[] = [
mockMessage('1234', ['message text'], [], {location: messageLocation1}),
mockMessage('1234', ['message text'], [], {location: messageLocation2}),
mockMessage('1234', ['message text'], [], {
location: messageLocation3,
legacyIds: ['87654321FEDCBA0987654321FEDCBA0987654321', '563965274788097516']
}),
mockMessage(
'1234', ['message text'], [], {location: messageLocation4, customId: 'other'}),
];
const serializer = new Xliff2TranslationSerializer(
'xx', absoluteFrom('/project'), useLegacyIds, options);
const output = serializer.serialize(messages);

// Note that in this test the third message will match the first two if legacyIds is
// false. Otherwise it will be a separate message on its own.

expect(output).toEqual([
`<?xml version="1.0" encoding="UTF-8" ?>`,
`<xliff version="2.0" xmlns="urn:oasis:names:tc:xliff:document:2.0" srcLang="xx">`,
` <file id="ngi18n" original="ng.template"${toAttributes(options)}>`,
` <unit id="1234">`,
` <notes>`,
` <note category="location">file-1.ts:1</note>`,
` <note category="location">file-2.ts:4,5</note>`,
...useLegacyIds ? [] : [' <note category="location">file-3.ts:1</note>'],
` </notes>`,
` <segment>`,
` <source>message text</source>`,
` </segment>`,
` </unit>`,
...useLegacyIds ?
[
` <unit id="563965274788097516">`,
` <notes>`,
` <note category="location">file-3.ts:1</note>`,
` </notes>`,
` <segment>`,
` <source>message text</source>`,
` </segment>`,
` </unit>`,
] :
[],
` <unit id="other">`,
` <notes>`,
` <note category="location">file-4.ts:4,5</note>`,
` </notes>`,
` <segment>`,
` <source>message text</source>`,
` </segment>`,
` </unit>`,
` </file>`,
`</xliff>\n`,
].join('\n'));
});
});
});
});
Expand Down

0 comments on commit f5710c6

Please sign in to comment.