Merge pull request #54 from martinroob/bugfix-1.9.3

Bugfix 1.10

Changelog.md

@@ -1,3 +1,19 @@
+<a name="1.10.0"></a>
+# [1.10.0](https://github.com/martinroob/ngx-i18nsupport-lib/compare/v1.9.2...v1.10.0) (2018-08-05)
+
+### Bug Fixes
+
+* **beautifier** [#52](https://github.com/martinroob/ngx-i18nsupport-lib/issues/52)
+The pretty data beautifier caused some issues ([xliffmerge #97](https://github.com/martinroob/ngx-i18nsupport/issues/97)).
+It is now replaced by an own implementation based on the serializer that is part of [xmldom](https://www.npmjs.com/package/xmldom).
+This might result in slightly different formatted documents when using `beautifyOutput`
+
+### Features
+
+* **API** [#53](https://github.com/martinroob/ngx-i18nsupport-lib/issues/53)
+`importNewTransunit` now allows to specify the ancestor of the newly imported unit. There is an optional new parameter for this, so the change is not breaking.
+If you do not use this parametes, the behaviour is the same as before (adding at the end).
+
 <a name="1.9.2"></a>
 # [1.9.2](https://github.com/martinroob/ngx-i18nsupport-lib/compare/v1.9.2...v1.9.1) (2018-06-01)
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ngx-i18nsupport-lib",
-  "version": "1.9.2",
+  "version": "1.10.0",
   "description": "A Typescript library to work with Angular generated i18n files (xliff, xmb)",
   "main": "bundles/ngx-i18nsupport-lib.umd.js",
   "module": "./dist/index.js",
@@ -52,7 +52,6 @@
   },
   "dependencies": {
     "@types/xmldom": "^0.1.29",
-    "pretty-data": "^0.40.0",
     "tokenizr": "^1.3.4",
     "xmldom": "^0.1.27"
   }

src/api/i-translation-messages-file.ts

@@ -136,10 +136,14 @@ export interface ITranslationMessagesFile {
      * @param copyContent Flag, wether to copy content or leave it empty.
      * Wben true, content will be copied from source.
      * When false, content will be left empty (if it is not the default language).
+     * @param importAfterElement optional (since 1.10) other transunit (part of this file), that should be used as ancestor.
+     * Newly imported trans unit is then inserted directly after this element.
+     * If not set or not part of this file, new unit will be imported at the end.
+     * If explicity set to null, new unit will be imported at the start.
      * @return the newly imported trans unit (since version 1.7.0)
      * @throws an error if trans-unit with same id already is in the file.
      */
-    importNewTransUnit(foreignTransUnit: ITransUnit, isDefaultLang: boolean, copyContent: boolean): ITransUnit;
+    importNewTransUnit(foreignTransUnit: ITransUnit, isDefaultLang: boolean, copyContent: boolean, importAfterElement?: ITransUnit): ITransUnit;
 
     /**
      * Remove the trans-unit with the given id.

src/impl/abstract-translation-messages-file.ts

@@ -1,9 +1,7 @@
 import {ITranslationMessagesFile, ITransUnit, STATE_NEW, STATE_TRANSLATED} from '../api';
 import {isNullOrUndefined} from 'util';
-import {DOMParser, XMLSerializer} from 'xmldom';
-import * as prettyData from 'pretty-data';
-import {AbstractTransUnit} from './abstract-trans-unit';
-import {XtbFile} from './xtb-file';
+import {DOMParser} from 'xmldom';
+import {XmlSerializer, XmlSerializerOptions} from './xml-serializer';
 /**
  * Created by roobm on 09.05.2017.
  * Abstract superclass for all implementations of ITranslationMessagesFile.
@@ -58,6 +56,13 @@ export abstract class AbstractTranslationMessagesFile implements ITranslationMes
 
     abstract fileType(): string;
 
+    /**
+     * return tag names of all elements that have mixed content.
+     * These elements will not be beautified.
+     * Typical candidates are source and target.
+     */
+    protected abstract elementsWithMixedContent(): string[];
+
     /**
      * Read all trans units from xml content.
      * Puts the found units into transUnits.
@@ -224,17 +229,22 @@ export abstract class AbstractTranslationMessagesFile implements ITranslationMes
      * depending on the values of isDefaultLang and copyContent.
      * So the source can be used as a dummy translation.
      * (used by xliffmerge)
-     * @param transUnit the trans unit to be imported.
+     * @param foreignTransUnit the trans unit to be imported.
      * @param isDefaultLang Flag, wether file contains the default language.
      * Then source and target are just equal.
      * The content will be copied.
      * State will be final.
      * @param copyContent Flag, wether to copy content or leave it empty.
      * Wben true, content will be copied from source.
      * When false, content will be left empty (if it is not the default language).
+     * @param importAfterElement optional (since 1.10) other transunit (part of this file), that should be used as ancestor.
+     * Newly imported trans unit is then inserted directly after this element.
+     * If not set or not part of this file, new unit will be imported at the end.
+     * If explicity set to null, new unit will be imported at the start.
+     * @return the newly imported trans unit (since version 1.7.0)
      * @throws an error if trans-unit with same id already is in the file.
      */
-    abstract importNewTransUnit(transUnit: ITransUnit, isDefaultLang: boolean, copyContent: boolean);
+    abstract importNewTransUnit(foreignTransUnit: ITransUnit, isDefaultLang: boolean, copyContent: boolean, importAfterElement?: ITransUnit): ITransUnit;
 
     /**
      * Remove the trans-unit with the given id.
@@ -272,10 +282,13 @@ export abstract class AbstractTranslationMessagesFile implements ITranslationMes
      * Default is false.
      */
     public editedContent(beautifyOutput?: boolean): string {
-        let result = new XMLSerializer().serializeToString(this._parsedDocument);
+        let options: XmlSerializerOptions = {};
         if (beautifyOutput === true) {
-            result = prettyData.pd.xml(result);
+           options.beautify = true;
+           options.indentString = '  ';
+           options.mixedContentElements = this.elementsWithMixedContent();
         }
+        let result = new XmlSerializer().serializeToString(this._parsedDocument, options);
         if (this._fileEndsWithEOL) {
             // add eol if there was eol in original source
             return result + '\n';

src/impl/dom-utilities.ts

@@ -21,6 +21,62 @@ export class DOMUtilities {
         }
     }
 
+    /**
+     * return an element with the given tag and id attribute.
+     * @param element
+     * @param tagName
+     * @param id
+     * @return {Element} subelement or null, if not existing.
+     */
+    public static getElementByTagNameAndId(element: Element | Document, tagName: string, id: string): Element {
+        let matchingElements = element.getElementsByTagName(tagName);
+        if (matchingElements && matchingElements.length > 0) {
+            for (let i = 0; i < matchingElements.length; i++) {
+                const node: Element = matchingElements.item(i);
+                if (node.getAttribute('id') === id) {
+                    return node;
+                }
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Get next sibling, that is an element.
+     * @param element
+     */
+    public static getElementFollowingSibling(element: Element): Element {
+        if (!element) {
+            return null;
+        }
+        let e = element.nextSibling;
+        while (e) {
+            if (e.nodeType === e.ELEMENT_NODE) {
+                return <Element> e;
+            }
+            e = e.nextSibling;
+        }
+        return null;
+    }
+
+    /**
+     * Get previous sibling, that is an element.
+     * @param element
+     */
+    public static getElementPrecedingSibling(element: Element): Element {
+        if (!element) {
+            return null;
+        }
+        let e = element.previousSibling;
+        while (e) {
+            if (e.nodeType === e.ELEMENT_NODE) {
+                return <Element> e;
+            }
+            e = e.previousSibling;
+        }
+        return null;
+    }
+
     /**
      * return content of element as string, including all markup.
      * @param element
@@ -101,12 +157,31 @@ export class DOMUtilities {
      * @return {Element}
      */
     public static createFollowingSibling(elementNameToCreate: string, previousSibling: Node): Element {
-        const newElement = previousSibling.ownerDocument.createElement('target');
+        const newElement = previousSibling.ownerDocument.createElement(elementNameToCreate);
+        return <Element> DOMUtilities.insertAfter(newElement, previousSibling);
+    }
+
+    /**
+     * Insert newElement directly after previousSibling.
+     * @param newElement
+     * @param previousSibling
+     */
+    public static insertAfter(newElement: Node, previousSibling: Node): Node {
         if (previousSibling.nextSibling !== null) {
             previousSibling.parentNode.insertBefore(newElement, previousSibling.nextSibling);
         } else {
             previousSibling.parentNode.appendChild(newElement);
         }
         return newElement;
     }
+
+    /**
+     * Insert newElement directly before nextSibling.
+     * @param newElement
+     * @param nextSibling
+     */
+    public static insertBefore(newElement: Node, nextSibling: Node): Node {
+        nextSibling.parentNode.insertBefore(newElement, nextSibling);
+        return newElement;
+    }
 }

src/impl/xliff-file.spec.ts

@@ -57,15 +57,21 @@ describe('ngx-i18nsupport-lib xliff 1.2 test spec', () => {
             expect(tu.sourceContent()).toBe('Schließen');
         });
 
-        it('should read xlf file and pretty print it with pretty-data', () => {
+        it('should read xlf file and pretty print it', () => {
             const file: ITranslationMessagesFile = readFile(MASTER1SRC);
             expect(file).toBeTruthy();
             expect(file.editedContent()).toContain('<source>Beschreibung zu <x id="INTERPOLATION"/> (<x id="INTERPOLATION_1"/>)</source>');
-            expect(file.editedContent(true)).toContain(`        <source>Beschreibung zu 
-          <x id="INTERPOLATION"/> (
-          <x id="INTERPOLATION_1"/>)
-        </source>
-`);
+            expect(file.editedContent(true)).toContain(`        <source>Beschreibung zu <x id="INTERPOLATION"/> (<x id="INTERPOLATION_1"/>)</source>`);
+        });
+
+        it('should not add empty lines when beautifying (issue ngx-i18nsupport #97)', () => {
+            const file: ITranslationMessagesFile = readFile(MASTER1SRC);
+            expect(file).toBeTruthy();
+            const editedContentBeautified = file.editedContent(true);
+            const file2: ITranslationMessagesFile = TranslationMessagesFileFactory.fromFileContent('xlf', editedContentBeautified, null, ENCODING);
+            const editedContentBeautifiedAgain = file2.editedContent(true);
+            expect(editedContentBeautifiedAgain).toMatch(/Beschreibung zu <x/);
+            expect(editedContentBeautifiedAgain).not.toMatch(/Beschreibung zu\s*\r\n?/);
         });
 
         it('should emit warnings', () => {
@@ -469,6 +475,46 @@ describe('ngx-i18nsupport-lib xliff 1.2 test spec', () => {
             expect(targetTu.sourceContent()).toBe('Test for merging units');
         });
 
+        it ('should copy a transunit to a specified position (#53)', () => {
+            const file: ITranslationMessagesFile = readFile(MASTER1SRC);
+            const tu: ITransUnit = file.transUnitWithId(ID_TO_MERGE);
+            expect(tu).toBeTruthy();
+            const targetFile: ITranslationMessagesFile = readFile(TRANSLATED_FILE_SRC);
+            expect(targetFile.transUnitWithId(ID_TO_MERGE)).toBeFalsy();
+            const ID_EXISTING = 'f540f05dc71be88e226a3920dbf1140b2658e5ea';
+            const existingTu = targetFile.transUnitWithId(ID_EXISTING);
+            expect(existingTu).toBeTruthy();
+            const newTu = targetFile.importNewTransUnit(tu, false, true, existingTu);
+            expect(targetFile.transUnitWithId(ID_TO_MERGE)).toBeTruthy();
+            expect(targetFile.transUnitWithId(ID_TO_MERGE)).toEqual(newTu);
+            const doc: Document = new DOMParser().parseFromString(targetFile.editedContent());
+            const existingElem = DOMUtilities.getElementByTagNameAndId(doc, 'trans-unit', ID_EXISTING);
+            const newElem = DOMUtilities.getElementByTagNameAndId(doc, 'trans-unit', ID_TO_MERGE);
+            expect(DOMUtilities.getElementFollowingSibling(existingElem)).toEqual(newElem);
+            let changedTargetFile = TranslationMessagesFileFactory.fromUnknownFormatFileContent(targetFile.editedContent(), null, null);
+            let targetTu = changedTargetFile.transUnitWithId(ID_TO_MERGE);
+            expect(targetTu.sourceContent()).toBe('Test for merging units');
+        });
+
+        it ('should copy a transunit to first position (#53)', () => {
+            const file: ITranslationMessagesFile = readFile(MASTER1SRC);
+            const tu: ITransUnit = file.transUnitWithId(ID_TO_MERGE);
+            expect(tu).toBeTruthy();
+            const targetFile: ITranslationMessagesFile = readFile(TRANSLATED_FILE_SRC);
+            expect(targetFile.transUnitWithId(ID_TO_MERGE)).toBeFalsy();
+            // when importNewTransUnit is called with null, new unit will be added at first position
+            const newTu = targetFile.importNewTransUnit(tu, false, true, null);
+            expect(targetFile.transUnitWithId(ID_TO_MERGE)).toBeTruthy();
+            expect(targetFile.transUnitWithId(ID_TO_MERGE)).toEqual(newTu);
+            const doc: Document = new DOMParser().parseFromString(targetFile.editedContent());
+            const newElem = DOMUtilities.getElementByTagNameAndId(doc, 'trans-unit', ID_TO_MERGE);
+            expect(newElem).toBeTruthy();
+            expect(DOMUtilities.getElementPrecedingSibling(newElem)).toBeFalsy();
+            let changedTargetFile = TranslationMessagesFileFactory.fromUnknownFormatFileContent(targetFile.editedContent(), null, null);
+            let targetTu = changedTargetFile.transUnitWithId(ID_TO_MERGE);
+            expect(targetTu.sourceContent()).toBe('Test for merging units');
+        });
+
         it ('should copy source to target and set a praefix and suffix', () => {
             const file: ITranslationMessagesFile = readFile(MASTER1SRC);
             file.setNewTransUnitTargetPraefix('%%');

src/impl/xliff-file.ts

@@ -1,4 +1,3 @@
-import {DOMParser} from "xmldom";
 import {format} from 'util';
 import {ITranslationMessagesFile, ITransUnit, FORMAT_XLIFF12, FILETYPE_XLIFF12} from '../api';
 import {DOMUtilities} from './dom-utilities';
@@ -60,6 +59,15 @@ export class XliffFile extends AbstractTranslationMessagesFile implements ITrans
         return FILETYPE_XLIFF12;
     }
 
+    /**
+     * return tag names of all elements that have mixed content.
+     * These elements will not be beautified.
+     * Typical candidates are source and target.
+     */
+    protected elementsWithMixedContent(): string[] {
+        return ['source', 'target', 'tool', 'seg-source', 'g', 'ph', 'bpt', 'ept', 'it', 'sub', 'mrk'];
+    }
+
     protected initializeTransUnits() {
         this.transUnits = [];
         let transUnitsInFile = this._parsedDocument.getElementsByTagName('trans-unit');
@@ -128,30 +136,66 @@ export class XliffFile extends AbstractTranslationMessagesFile implements ITrans
      * depending on the values of isDefaultLang and copyContent.
      * So the source can be used as a dummy translation.
      * (used by xliffmerge)
-     * @param transUnit the trans unit to be imported.
+     * @param foreignTransUnit the trans unit to be imported.
      * @param isDefaultLang Flag, wether file contains the default language.
      * Then source and target are just equal.
      * The content will be copied.
      * State will be final.
      * @param copyContent Flag, wether to copy content or leave it empty.
      * Wben true, content will be copied from source.
      * When false, content will be left empty (if it is not the default language).
+     * @param importAfterElement optional (since 1.10) other transunit (part of this file), that should be used as ancestor.
+     * Newly imported trans unit is then inserted directly after this element.
+     * If not set or not part of this file, new unit will be imported at the end.
+     * If explicity set to null, new unit will be imported at the start.
      * @return the newly imported trans unit (since version 1.7.0)
      * @throws an error if trans-unit with same id already is in the file.
      */
-    public importNewTransUnit(transUnit: ITransUnit, isDefaultLang: boolean, copyContent: boolean): ITransUnit {
-        if (this.transUnitWithId(transUnit.id)) {
-            throw new Error(format('tu with id %s already exists in file, cannot import it', transUnit.id));
+    importNewTransUnit(foreignTransUnit: ITransUnit, isDefaultLang: boolean, copyContent: boolean, importAfterElement?: ITransUnit): ITransUnit {
+        if (this.transUnitWithId(foreignTransUnit.id)) {
+            throw new Error(format('tu with id %s already exists in file, cannot import it', foreignTransUnit.id));
         }
-        let newTu = (<AbstractTransUnit> transUnit).cloneWithSourceAsTarget(isDefaultLang, copyContent, this);
+        let newTu = (<AbstractTransUnit> foreignTransUnit).cloneWithSourceAsTarget(isDefaultLang, copyContent, this);
         let bodyElement = DOMUtilities.getFirstElementByTagName(this._parsedDocument, 'body');
-        if (bodyElement) {
+        if (!bodyElement) {
+            throw new Error(format('File "%s" seems to be no xliff 1.2 file (should contain a body element)', this._filename));
+        }
+        let inserted = false;
+        let isAfterElementPartOfFile = false;
+        if (!!importAfterElement) {
+            let insertionPoint = this.transUnitWithId(importAfterElement.id);
+            if (!!insertionPoint) {
+                isAfterElementPartOfFile = true;
+            }
+        }
+        if (importAfterElement === undefined || (importAfterElement && !isAfterElementPartOfFile)) {
             bodyElement.appendChild(newTu.asXmlElement());
+            inserted = true;
+        } else if (importAfterElement === null) {
+            let firstUnitElement = DOMUtilities.getFirstElementByTagName(this._parsedDocument, 'trans-unit');
+            if (firstUnitElement) {
+                DOMUtilities.insertBefore(newTu.asXmlElement(), firstUnitElement);
+                inserted = true;
+            } else {
+                // no trans-unit, empty file, so add to body
+                bodyElement.appendChild(newTu.asXmlElement());
+                inserted = true;
+            }
+        } else {
+            let refUnitElement = DOMUtilities.getElementByTagNameAndId(this._parsedDocument, 'trans-unit', importAfterElement.id);
+            if (refUnitElement) {
+                DOMUtilities.insertAfter(newTu.asXmlElement(), refUnitElement);
+                inserted = true;
+            }
+        }
+        if (inserted) {
             this.lazyInitializeTransUnits();
             this.transUnits.push(newTu);
             this.countNumbers();
+            return newTu;
+        } else {
+         return null;
         }
-        return newTu;
     }
 
     /**