Fix a bug where a literal colon in a cell between (#202)

two placeholders breaks detection of both placeholders,
replacing the whole as empty (from beggining of first
placeholder to end of the next one and any literal text in between)

Author: IagoSRL

src/index.js

@@ -807,7 +807,20 @@ class Workbook {
     // is the entirety of the string) and `type` (one of `table` or `cell`)
     extractPlaceholders(string) {
         // Yes, that's right. It's a bunch of brackets and question marks and stuff.
-        var re = /\${(?:(.+?):)?(.+?)(?:\.(.+?))?(?::(.+?))??}/g;
+        // IMPORTANT: Instead of matching 'any character' on every capture (by using a dot),
+        // it captures 'any except colon and brackets' by using [^{}:] to avoid
+        // a bug where the expression will consume the text in a 'greedy' way,
+        // capturing colon characters from outside the placeholder (after a closing
+        // bracket).
+        // Since that characters are already managed in the RegExp as separators
+        // of capture groups, it will still match the whole expression,
+        // but in a safe way (by including brackets and not only the colon from
+        // the exclusion, it avoids other kinds of mistakes and wrong placeholder
+        // syntax, but if compatibility is a concern they could be removed and
+        // left like [^:] ).
+        // (The non-greedy modifier should have done that, but it's more difficult
+        // to get it right than expected)
+        var re = /\${(?:([^{}:]+?):)?([^{}:]+?)(?:\.([^{}:]+?))?(?::([^{}:]+?))??}/g;
 
         var match = null, matches = [];
         while ((match = re.exec(string)) !== null) {
@@ -1061,14 +1074,14 @@ class Workbook {
                     let mergeCell = self.sheet.root.findall("mergeCells/mergeCell")
                         .find(c => self.splitRange(c.attrib.ref).start === cell.attrib.r)
                     let isMergeCell = mergeCell != null
-                    
+
                     if(isMergeCell) {
                         let originalMergeRange = self.splitRange(mergeCell.attrib.ref),
                             originalMergeStart    = self.splitRef(originalMergeRange.start),
                             originalMergeEnd      = self.splitRef(originalMergeRange.end);
-                        
+
                         for (let colnum = self.charToNum(originalMergeStart.col) + 1; colnum <= self.charToNum(originalMergeEnd.col); colnum++) {
-                            const originalRow = self.sheet.root.find('sheetData')._children.find(f=>f.attrib.r == originalMergeStart.row)      
+                            const originalRow = self.sheet.root.find('sheetData')._children.find(f=>f.attrib.r == originalMergeStart.row)
                             let col = self.numToChar(colnum)
                             let originalCell = originalRow._children.find(f=>f.attrib.r.startsWith(col))
 
@@ -1698,7 +1711,7 @@ class Workbook {
     getWidthCell(numCol, sheet) {
         var defaultWidth = sheet.root.find("sheetFormatPr").attrib["defaultColWidth"];
         if (!defaultWidth) {
-            // TODO : Check why defaultColWidth is not set ? 
+            // TODO : Check why defaultColWidth is not set ?
             defaultWidth = 11.42578125;
         }
         var finalWidth = defaultWidth;
@@ -1751,7 +1764,7 @@ class Workbook {
     }
     columnWidthToEMUs(width) {
         // TODO : This is not the true. Change with true calcul
-        // can find help here : 
+        // can find help here :
         // https://docs.microsoft.com/en-us/office/troubleshoot/excel/determine-column-widths
         // https://stackoverflow.com/questions/58021996/how-to-set-the-fixed-column-width-values-in-inches-apache-poi
         // https://poi.apache.org/apidocs/dev/org/apache/poi/ss/usermodel/Sheet.html#setColumnWidth-int-int-
@@ -1766,9 +1779,9 @@ class Workbook {
     }
     /**
      * Find max file id.
-     * @param {RegExp} fileNameRegex 
-     * @param {RegExp} idRegex 
-     * @returns {number} 
+     * @param {RegExp} fileNameRegex
+     * @param {RegExp} idRegex
+     * @returns {number}
      */
     findMaxFileId(fileNameRegex, idRegex) {
         var self = this;

test/crud-test.ts

@@ -279,6 +279,35 @@ describe("CRUD operations", function() {
                 done();
             });
 
+        });
+
+        it("can include a literal colon in between two placeholders in the same cell (with and without newlines in between)", function(done) {
+
+            fs.readFile(path.join(__dirname, "templates", "colon-allowed-between-placeholders.xlsx"), function(err, data) {
+                expect(err).toBeNull();
+
+                var t = new XlsxTemplate(data);
+
+                t.substitute(1, {
+                    foo: "f-value",
+                    bar: "b-value",
+                });
+
+                var newData = t.generate();
+
+                var sharedStrings = etree.parse(t.archive.file("xl/sharedStrings.xml").asText()).getroot(),
+                    sheet1        = etree.parse(t.archive.file("xl/worksheets/sheet1.xml").asText()).getroot();
+
+                expect(sheet1).toBeDefined();
+                expect(getSharedString(sharedStrings, sheet1, "A1")).toEqual("f-value and colon in between: b-value on same line");
+                expect(getSharedString(sharedStrings, sheet1, "A2")).toEqual("f-value\nand colon in between: b-value\non different lines");
+
+                // XXX: For debugging only
+                fs.writeFileSync("test/output/colon-allowed-between-placeholders.xlsx", newData, "binary");
+
+                done();
+            });
+
         });
 
 		it("can substitute values when single item array contains an object and generate a file", function(done) {