Perform substitution on all parts, not just the last one

Signed-off-by: Stefan Parviainen <pafcu@iki.fi>

This way the substitutions don't need to appear in the same order in the
translated string.

src/languageHandler.js

@@ -126,6 +126,8 @@ export function substitute(text, variables, tags) {
  * @return a React <span> component if any non-strings were used in substitutions, otherwise a string
  */
 export function replaceByRegexes(text, mapping) {
+    // We initially store our output as an array of strings and objects (e.g. React components).
+    // This will then be converted to a string or a <span> at the end
     const output = [text];
 
     // If we insert any components we need to wrap the output in a span. React doesn't like just an array of components.
@@ -135,13 +137,18 @@ export function replaceByRegexes(text, mapping) {
         // TODO: Cache regexps
         const regexp = new RegExp(regexpString);
 
-        // convert the last element in 'output' into 3 elements (pre-text, sub function, post-text).
+        // Loop over what output we have so far and perform replacements
-        // Rinse and repeat for other patterns (using post-text).
+        // We look for matches: if we find one, we get three parts: everything before the match, the replaced part,
-        const inputText = output.pop();
+        // and everything after the match. Insert all three into the output. We need to do this because we can insert objects.
+        // Otherwise there would be no need for the splitting and we could do simple replcement.
+        for (const outputIndex in output) {
+            const inputText = output[outputIndex];
+            if (typeof inputText !== 'string') { // We might have inserted objects earlier, don't try to replace them
+                continue;
+            }
+
             const match = inputText.match(regexp);
             if (!match) {
-            output.push(inputText); // Push back input
-
                 // Missing matches is entirely possible because you might choose to show some variables only in the case
                 // of e.g. plurals. It's still a bit suspicious, and could be due to an error, so log it.
                 // However, not showing count is so common that it's not worth logging. And other commonly unused variables
@@ -151,15 +158,14 @@ export function replaceByRegexes(text, mapping) {
                 }
                 continue;
             }
+
             const capturedGroups = match.slice(2);
 
-        // Return the raw translation before the *match* followed by the return value of sub() followed
+            // The textual part before the match
-        // by the raw translation after the *match* (not captured group).
-
             const head = inputText.substr(0, match.index);
-        if (head !== '') { // Don't push empty nodes, they are of no use
+
-            output.push(head);
+            // The textual part after the match
-        }
+            const tail = inputText.substr(match.index + match[0].length);
 
             let replaced;
             // If substitution is a function, call it
@@ -169,19 +175,26 @@ export function replaceByRegexes(text, mapping) {
                 replaced = mapping[regexpString];
             }
 
-        // Here we also need to check that it actually is a string before comparing against one
-        // The head and tail are always strings
-        if (typeof replaced !== 'string' || replaced !== '') {
-            output.push(replaced);
-        }
-
             if (typeof replaced === 'object') {
                 shouldWrapInSpan = true;
             }
 
-        const tail = inputText.substr(match.index + match[0].length);
+            output.splice(outputIndex, 1); // Remove old element
+
+            // Insert in reverse order as splice does insert-before and this way we get the final order correct
             if (tail !== '') {
-            output.push(tail);
+                output.splice(outputIndex, 0, tail);
+            }
+
+            // Here we also need to check that it actually is a string before comparing against one
+            // The head and tail are always strings
+            if (typeof replaced !== 'string' || replaced !== '') {
+                output.splice(outputIndex, 0, replaced);
+            }
+
+            if (head !== '') { // Don't push empty nodes, they are of no use
+                output.splice(outputIndex, 0, head);
+            }
         }
     }
 

test/i18n-test/languageHandler-test.js

@@ -65,4 +65,9 @@ describe('languageHandler', function() {
         expect(languageHandler._t(text, {}, { 'StartChatButton': () => <i>foo</i> }))
             .toEqual(<span>Press <i>foo</i> to start a chat with someone</span>);
     });
+
+    it('replacements in the wrong order', function() {
+        const text = '%(var1)s %(var2)s';
+        expect(languageHandler._t(text, { var2: 'val2', var1: 'val1' })).toBe('val1 val2');
+    });
 });