Merge pull request #85 from martinroob/feature-0.16

Feature 0.16

Changelog.md

@@ -1,3 +1,14 @@
+<a name="0.16.0"></a>
+# [0.16.0](https://github.com/martinroob/ngx-i18nsupport/compare/v0.16.0...v0.15.0) (2018-05-01)
+
+### Bug fixes
+* **xliffmerge** xliffmerge fails when ICU message contains interpolation and/or tags ([#83](https://github.com/martinroob/ngx-i18nsupport/issues/83)).
+
+* **xliffmerge** Placeholder index is invalid ([#84](https://github.com/martinroob/ngx-i18nsupport/issues/84)).
+
+### Features
+* **xliffmerge:** There is a new configuration flag `beautifyOutput`. When set to `true`  now [pretty-data](https://github.com/vkiryukhin/pretty-data) (the library beyond pretty-xml) will be used to format the output. ([#64](https://github.com/martinroob/ngx-i18nsupport/issues/64))
+
 <a name="0.15.0"></a>
 # [0.15.0](https://github.com/martinroob/ngx-i18nsupport/compare/v0.15.0...v0.14.0) (2018-04-22)
 

README.md

@@ -103,6 +103,7 @@ The following list shows all allowed content (every value is optional, there is
     "useSourceAsTarget": true,
     "targetPraefix": "",
     "targetSuffix": "",
+    "beautifyOutput": false,
     "allowIdChange": false,
     "autotranslate": false,
     "apikey": "",
@@ -139,6 +140,10 @@ E.g. `targetPraefix: "%%"` and source contains the string "menu", target will co
 when the flag `useSourceAsTarget` is set and a source is copied to target,
 then the target string will be suffixed by this value.
 E.g. `targetSuffix: "%%"` and source contains the string "menu", target will contain "menu%%" at the end.
+- `beautifyOutput` (since 0.16.0) (boolean, default `false`):
+when set to true, the generated xml output will be send through a beautifier
+([pretty-data](https://github.com/vkiryukhin/pretty-data))
+to get consistent output. See ([xliffmerge #64 Could xlf output be better formatted](https://github.com/martinroob/ngx-i18nsupport/issues/64)) for details.
 - `allowIdChange` (since 0.11.0) (boolean, default `false`):
 flag, wether xliffmerge should merge transunits with changed IDs.
 When there is only a small change in the original message text, e.g. a trailing white space, the Angular extraction tool will change the ID of the unit.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ngx-i18nsupport",
-  "version": "0.15.0",
+  "version": "0.16.0",
   "description": "Some tooling to be used with the Angular 2 i18n workflow",
   "main": "index.js",
   "module": "./src",
@@ -39,21 +39,21 @@
   },
   "homepage": "https://github.com/martinroob/ngx-i18nsupport#readme",
   "devDependencies": {
-    "@types/jasmine": "^2.8.3",
-    "@types/node": "^9.3.0",
-    "@types/request": "^2.0.9",
+    "@types/jasmine": "^2.8.6",
+    "@types/node": "9.3.0",
+    "@types/request": "^2.47.0",
     "coveralls": "^3.0.0",
     "cpx": "^1.5.0",
     "istanbul": "^0.4.5",
     "jasmine-node": "^1.14.5",
-    "typescript": "^2.6.2"
+    "typescript": "^2.8.3"
   },
   "dependencies": {
-    "chalk": "^2.2.0",
-    "commander": "^2.9.0",
-    "ngx-i18nsupport-lib": "^1.8.1",
-    "request": "^2.81.0",
-    "rxjs": "^5.5.6"
+    "chalk": "^2.4.1",
+    "commander": "^2.15.1",
+    "ngx-i18nsupport-lib": "^1.9.1",
+    "request": "^2.85.0",
+    "rxjs": "^6.0.0"
   },
   "xliffmergeOptions": {
     "description": "just for some tests here"

src/autotranslate/auto-translate-service.ts

@@ -1,8 +1,8 @@
 import {format} from 'util';
 import * as request from 'request';
 import {Observable} from 'rxjs';
-import 'rxjs/add/operator/map';
-import 'rxjs/add/operator/catch';
+import {of, forkJoin, throwError} from 'rxjs';
+import {map} from 'rxjs/operators';
 
 /**
  * Created by roobm on 03.07.2017.
@@ -81,26 +81,27 @@ export class AutoTranslateService {
     public translateMultipleStrings(messages: string[], from: string, to: string): Observable<string[]> {
         // empty array needs no translation and always works ... (#78)
         if (messages.length == 0) {
-            return Observable.of([]);
+            return of([]);
         }
         if (!this._apiKey) {
-            return Observable.throw('cannot autotranslate: no api key');
+            return throwError('cannot autotranslate: no api key');
         }
         if (!from || !to) {
-            return Observable.throw('cannot autotranslate: source and target language must be set');
+            return throwError('cannot autotranslate: source and target language must be set');
         }
         from = AutoTranslateService.stripRegioncode(from);
         to = AutoTranslateService.stripRegioncode(to);
         const allRequests: Observable<string[]>[] = this.splitMessagesToGoogleLimit(messages).map((partialMessages: string[]) => {
             return this.limitedTranslateMultipleStrings(partialMessages, from, to);
         });
-        return Observable.forkJoin(allRequests).map((allTranslations: string[][]) => {
-            let all = [];
-            for (let i = 0; i < allTranslations.length; i++) {
-                all = all.concat(allTranslations[i]);
-            }
-            return all;
-        })
+        return forkJoin(allRequests).pipe(
+            map((allTranslations: string[][]) => {
+                let all = [];
+                for (let i = 0; i < allTranslations.length; i++) {
+                    all = all.concat(allTranslations[i]);
+                }
+                return all;
+        }));
     }
 
     private splitMessagesToGoogleLimit(messages: string[]): string[][] {
@@ -146,7 +147,8 @@ export class AutoTranslateService {
             json: true,
 //            proxy: 'http://127.0.0.1:8888' To set a proxy use env var HTTPS_PROXY
         };
-        return this.post(realUrl, options).map((data) => {
+        return this.post(realUrl, options).pipe(
+            map((data) => {
             const body: any = data.body;
             if (!body) {
                 throw new Error('no result received');
@@ -165,7 +167,7 @@ export class AutoTranslateService {
             return result.translations.map((translation: TranslationsResource) => {
                 return translation.translatedText;
             });
-        });
+        }));
     }
 
     /**

src/autotranslate/xliff-merge-auto-translate-service.ts

@@ -1,5 +1,6 @@
 import {isNullOrUndefined} from 'util';
-import {Observable} from 'rxjs/Observable';
+import {Observable, forkJoin, of} from 'rxjs';
+import {map, catchError} from 'rxjs/operators';
 import {
     IICUMessage, IICUMessageTranslation, INormalizedMessage, ITranslationMessagesFile, ITransUnit,
     STATE_NEW
@@ -29,16 +30,17 @@ export class XliffMergeAutoTranslateService {
      * @return a promise with the execution result as a summary report.
      */
     public autoTranslate(from: string, to: string, languageSpecificMessagesFile: ITranslationMessagesFile): Observable<AutoTranslateSummaryReport> {
-        return Observable.forkJoin([
+        return forkJoin([
             this.doAutoTranslateNonICUMessages(from, to, languageSpecificMessagesFile),
             ...this.doAutoTranslateICUMessages(from, to, languageSpecificMessagesFile)])
-            .map((summaries: AutoTranslateSummaryReport[]) => {
-                const summary = summaries[0];
-                for (let i = 1; i < summaries.length; i++) {
-                    summary.merge(summaries[i]);
-                }
-                return summary;
-        });
+            .pipe(
+                map((summaries: AutoTranslateSummaryReport[]) => {
+                    const summary = summaries[0];
+                    for (let i = 1; i < summaries.length; i++) {
+                        summary.merge(summaries[i]);
+                    }
+                    return summary;
+        }));
     }
 
     /**
@@ -64,7 +66,8 @@ export class XliffMergeAutoTranslateService {
             return tu.sourceContentNormalized().asDisplayString();
         });
         return this.autoTranslateService.translateMultipleStrings(allMessages, from, to)
-            .map((translations: string[]) => {
+            .pipe(
+                map((translations: string[]) => {
                 const summary = new AutoTranslateSummaryReport(from, to);
                 summary.setIgnored(allUntranslated.length - allTranslatable.length);
                 for (let i = 0; i < translations.length; i++) {
@@ -74,11 +77,12 @@ export class XliffMergeAutoTranslateService {
                     summary.addSingleResult(tu, result);
                 }
                 return summary;
-            }).catch((err) => {
-                const failSummary = new AutoTranslateSummaryReport(from, to);
-                failSummary.setError(err.message, allMessages.length);
-                return Observable.of(failSummary);
-            });
+                }),
+                catchError((err) => {
+                    const failSummary = new AutoTranslateSummaryReport(from, to);
+                    failSummary.setError(err.message, allMessages.length);
+                    return of(failSummary);
+            }));
     }
 
     private doAutoTranslateICUMessages(from: string, to: string, languageSpecificMessagesFile: ITranslationMessagesFile): Observable<AutoTranslateSummaryReport>[] {
@@ -103,25 +107,26 @@ export class XliffMergeAutoTranslateService {
         if (categories.find((category) => !isNullOrUndefined(category.getMessageNormalized().getICUMessage()))) {
             const summary = new AutoTranslateSummaryReport(from, to);
             summary.setIgnored(1);
-            return Observable.of(summary);
+            return of(summary);
         }
         const allMessages: string[] = categories.map((category) => category.getMessageNormalized().asDisplayString());
         return this.autoTranslateService.translateMultipleStrings(allMessages, from, to)
-            .map((translations: string[]) => {
-                const summary = new AutoTranslateSummaryReport(from, to);
-                const icuTranslation: IICUMessageTranslation = {};
-                for (let i = 0; i < translations.length; i++) {
-                    const translationText = translations[i];
-                    icuTranslation[categories[i].getCategory()] = translationText;
-                }
-                const result = this.autoTranslateICUUnit(tu, icuTranslation);
-                summary.addSingleResult(tu, result);
-                return summary;
-            }).catch((err) => {
-                const failSummary = new AutoTranslateSummaryReport(from, to);
-                failSummary.setError(err.message, allMessages.length);
-                return Observable.of(failSummary);
-            });
+            .pipe(
+                map((translations: string[]) => {
+                    const summary = new AutoTranslateSummaryReport(from, to);
+                    const icuTranslation: IICUMessageTranslation = {};
+                    for (let i = 0; i < translations.length; i++) {
+                        const translationText = translations[i];
+                        icuTranslation[categories[i].getCategory()] = translationText;
+                    }
+                    const result = this.autoTranslateICUUnit(tu, icuTranslation);
+                    summary.addSingleResult(tu, result);
+                    return summary;
+                }), catchError((err) => {
+                    const failSummary = new AutoTranslateSummaryReport(from, to);
+                    failSummary.setError(err.message, allMessages.length);
+                    return of(failSummary);
+            }));
     }
 
     private autoTranslateNonICUUnit(tu: ITransUnit, translatedMessage: string): AutoTranslateResult {

src/xliffmerge/configuration-schema.json

@@ -64,6 +64,10 @@
           "description": "when the flag `useSourceAsTarget` is set and a source is copied to target, then the target string will be praefixed by this value.",
           "type": "string"
         },
+        "beautifyOutput": {
+          "description": "when set to true, the generated xml output will be send through a beautifier to get consistent output.(default \"false\")",
+          "type": "boolean"
+        },
         "targetSuffix": {
           "description": "when the flag `useSourceAsTarget` is set and a source is copied to target, then the target string will be suffixed by this value.",
           "type": "string"

src/xliffmerge/i-xliff-merge-options.ts

@@ -43,6 +43,7 @@ export interface IXliffMergeOptions {
     useSourceAsTarget?: boolean; // Flag, whether source must be used as target for new trans-units
     targetPraefix?: string; // Praefix for target copied from sourced
     targetSuffix?: string; // Suffix for target copied from sourced
+    beautifyOutput?: boolean; // beautify output
     autotranslate?: boolean|string[]; // enable auto translate via Google Translate
         // if it is an array, list of languages to autotranslate
         // if it is true, autotranslate all languages (except source language of course)

src/xliffmerge/translation-messages-file-reader.ts

@@ -63,8 +63,12 @@ export class TranslationMessagesFileReader {
     /**
      * Save edited file.
      * @param messagesFile
+     * @param beautifyOutput Flag whether to use pretty-data to format the output.
+     * XMLSerializer produces some correct but strangely formatted output, which pretty-data can correct.
+     * See issue #64 for details.
+     * Default is false.
      */
-    public static save(messagesFile: ITranslationMessagesFile) {
+    public static save(messagesFile: ITranslationMessagesFile, beautifyOutput?: boolean) {
         FileUtil.replaceContent(messagesFile.filename(), messagesFile.editedContent(), messagesFile.encoding());
     }
 }

src/xliffmerge/xlf12-merge.spec.ts

@@ -259,7 +259,7 @@ describe('XliffMerge XLIFF 1.2 format tests', () => {
             xliffMergeCmd.run();
             expect(ws.writtenData()).not.toContain('ERROR');
             expect(ws.writtenData()).toContain('merged 12 trans-units from master to "en"');
-            expect(ws.writtenData()).toContain('removed 3 unused trans-units in "en"');
+            expect(ws.writtenData()).toContain('removed 5 unused trans-units in "en"');
 
             // look, that the new file contains the old translation
             langFileEnglish = readXliff(xliffMergeCmd.generatedI18nFile('en'));
@@ -456,6 +456,75 @@ describe('XliffMerge XLIFF 1.2 format tests', () => {
             done();
         });
 
+        it('should translate messages with 2 custom tags with different ids (#84)', (done) => {
+            const ID_2_CUSTOM_TAGS = '8856d298b6fa89a339475c5d5cd20f2d2afcfbf7';
+            FileUtil.copy(MASTER1SRC, MASTER);
+            let ws: WriterToString = new WriterToString();
+            let commandOut = new CommandOutput(ws);
+            let profileContent: IConfigFile = {
+                xliffmergeOptions: {
+                    defaultLanguage: 'de',
+                    srcDir: WORKDIR,
+                    genDir: WORKDIR,
+                    i18nFile: MASTERFILE
+                }
+            };
+            let xliffMergeCmd = XliffMerge.createFromOptions(commandOut, {languages: ['de', 'en']}, profileContent);
+            xliffMergeCmd.run();
+            expect(ws.writtenData()).not.toContain('ERROR');
+
+            // now translate some texts in the English version
+            let langFileEnglish: ITranslationMessagesFile = readXliff(xliffMergeCmd.generatedI18nFile('en'));
+            let tu: ITransUnit = langFileEnglish.transUnitWithId(ID_2_CUSTOM_TAGS);
+            expect(tu).toBeTruthy();
+            expect(tu.sourceContentNormalized().asDisplayString()).toBe('Neues <bs-activity-stream-element></bs-activity-stream-element> wurde gemeldet durch <bs-activity-stream-element id="1"></bs-activity-stream-element>');
+            const translation = tu.sourceContentNormalized().translate('New <bs-activity-stream-element></bs-activity-stream-element> was reported by <bs-activity-stream-element id="1"></bs-activity-stream-element>');
+            tu.translate(translation);
+            TranslationMessagesFileReader.save(langFileEnglish);
+
+            // look, that the new file contains the translation
+            langFileEnglish = readXliff(xliffMergeCmd.generatedI18nFile('en'));
+            expect(langFileEnglish.transUnitWithId(ID_2_CUSTOM_TAGS).targetContent()).toBe('New <x id="START_TAG_BS-ACTIVITY-STREAM-ELEMENT" ctype="x-bs-activity-stream-element" equiv-text="&lt;bs-activity-stream-element>"/><x id="CLOSE_TAG_BS-ACTIVITY-STREAM-ELEMENT" ctype="x-bs-activity-stream-element"/> was reported by <x id="START_TAG_BS-ACTIVITY-STREAM-ELEMENT_1" ctype="x-bs-activity-stream-element" equiv-text="&lt;bs-activity-stream-element>"/><x id="CLOSE_TAG_BS-ACTIVITY-STREAM-ELEMENT" ctype="x-bs-activity-stream-element"/>');
+            expect(langFileEnglish.transUnitWithId(ID_2_CUSTOM_TAGS).targetContentNormalized().asDisplayString()).toBe('New <bs-activity-stream-element></bs-activity-stream-element> was reported by <bs-activity-stream-element id="1"></bs-activity-stream-element>');
+
+            done();
+        });
+
+        it('should translate ICU message with placeholder (#83)', (done) => {
+            const ID_ICU_WITH_PLACEHOLDER = '8856d298b6fa89a339475c5d5cd20f2d2afcfbf8';
+            FileUtil.copy(MASTER1SRC, MASTER);
+            let ws: WriterToString = new WriterToString();
+            let commandOut = new CommandOutput(ws);
+            let profileContent: IConfigFile = {
+                xliffmergeOptions: {
+                    defaultLanguage: 'de',
+                    srcDir: WORKDIR,
+                    genDir: WORKDIR,
+                    i18nFile: MASTERFILE
+                }
+            };
+            let xliffMergeCmd = XliffMerge.createFromOptions(commandOut, {languages: ['de', 'en']}, profileContent);
+            xliffMergeCmd.run();
+            expect(ws.writtenData()).not.toContain('ERROR');
+
+            // now translate some texts in the English version
+            let langFileEnglish: ITranslationMessagesFile = readXliff(xliffMergeCmd.generatedI18nFile('en'));
+            let tu: ITransUnit = langFileEnglish.transUnitWithId(ID_ICU_WITH_PLACEHOLDER);
+            expect(tu).toBeTruthy();
+            expect(tu.sourceContentNormalized().asDisplayString()).toBe('<ICU-Message/>');
+            const t = '{VAR_PLURAL, plural, =1 {Crash <x id="INTERPOLATION" equiv-text="{{ a }}"/> was} other {Crashes <x id="INTERPOLATION" equiv-text="{{ a}}"/> were} }';
+            const translation = tu.sourceContentNormalized().translateICUMessage({"=1": 'Crash <x id="INTERPOLATION" equiv-text="{{ a }}"/> was'});
+            tu.translate(translation);
+            TranslationMessagesFileReader.save(langFileEnglish);
+
+            // look, that the new file contains the translation
+            langFileEnglish = readXliff(xliffMergeCmd.generatedI18nFile('en'));
+            expect(langFileEnglish.transUnitWithId(ID_ICU_WITH_PLACEHOLDER).targetContentNormalized().asDisplayString()).toBe('<ICU-Message/>');
+            expect(langFileEnglish.transUnitWithId(ID_ICU_WITH_PLACEHOLDER).targetContentNormalized().getICUMessage().asNativeString()).toContain('Crash');
+
+            done();
+        });
+
         it('allowIdChange feature, should merge only white space changed content with changed ID to already translated files (#65)', (done) => {
             const ID_ORIGINAL = 'originalId';
             const ID_CHANGED = 'changedId';