Rework handling of actions 'replace', fix 476 (#481)

* formatting: rework handling of actions 'replace'

Don't honor 'replace' if it does not match at the end of the previous
text.

plover/formatting.py

@@ -95,6 +95,8 @@ def format(self, undo, do, prev):
         rendered translations. If there is no context then this may be None.
 
         """
+        prev_formatting = prev.formatting if prev else None
+
         for t in do:
             last_action = self._get_last_action(prev.formatting if prev else None)
             if t.english:
@@ -108,17 +110,10 @@ def format(self, undo, do, prev):
         old = [a for t in undo for a in t.formatting]
         new = [a for t in do for a in t.formatting]
 
-        min_length = min(len(old), len(new))
-        for i in xrange(min_length):
-            if old[i] != new[i]:
-                break
-        else:
-            i = min_length
-
         for callback in self._listeners:
             callback(old, new)
 
-        OutputHelper(self._output).render(old[i:], new[i:])
+        OutputHelper(self._output, prev_formatting).render(old, new)
 
     def _get_last_action(self, actions):
         """Return last action in actions if possible or return a default action."""
@@ -134,9 +129,13 @@ class OutputHelper(object):
     optimizes away extra backspaces and typing.
 
     """
-    def __init__(self, output):
-        self.before = ''
-        self.after = ''
+    def __init__(self, output, initial_formatting=None):
+        if initial_formatting is None:
+            self.initial_formatting = []
+        else:
+            self.initial_formatting = initial_formatting
+        self.before = None
+        self.after = None
         self.output = output
 
     def commit(self):
@@ -154,32 +153,39 @@ def commit(self):
         self.before = ''
         self.after = ''
 
-    def render(self, undo, do):
-        for a in undo:
-            if a.replace:
-                if len(a.replace) >= len(self.before):
-                    self.before = ''
-                else:
-                    self.before = self.before[:-len(a.replace)]
+    @staticmethod
+    def _actions_to_text(action_list, text=u''):
+        for a in action_list:
+            if a.replace and text.endswith(a.replace):
+                text = text[:-len(a.replace)]
             if a.text:
-                self.before += a.text
+                text += a.text
+        return text
 
-        self.after = self.before
+    def render(self, undo, do):
 
-        for a in reversed(undo):
-            if a.text:
-                self.after = self.after[:-len(a.text)]
-            if a.replace:
-                self.after += a.replace
+        initial_text = self._actions_to_text(self.initial_formatting)
+
+        min_length = min(len(undo), len(do))
+        for i in range(min_length):
+            if undo[i] != do[i]:
+                break
+        else:
+            i = min_length
+
+        if i > 0:
+            initial_text = self._actions_to_text(undo[:i], initial_text)
+            undo = undo[i:]
+            do = do[i:]
+
+        self.before = initial_text
+        self.after = initial_text[:]
+
+        self.before = self._actions_to_text(undo, self.before)
 
         for a in do:
-            if a.replace:
-                if len(a.replace) > len(self.after):
-                    self.before = a.replace[
-                        :len(a.replace)-len(self.after)] + self.before
-                    self.after = ''
-                else:
-                    self.after = self.after[:-len(a.replace)]
+            if a.replace and self.after.endswith(a.replace):
+                self.after = self.after[:-len(a.replace)]
             if a.text:
                 self.after += a.text
             if a.combo:

test/test_formatting.py

@@ -1090,5 +1090,87 @@ def test_rightmost_word(self):
                  ('word.', 'word.')]
         self.check(formatting._rightmost_word, cases)
 
+    def test_replace(self):
+        for translations, expected_instructions in (
+            # Check that 'replace' does not unconditionally erase
+            # the previous character if it does not match.
+            ([
+                translation(english='{MODE:SET_SPACE:}'),
+                translation(english='foobar'),
+                translation(english='{^}{#Return}{^}{-|}'),
+
+            ], [('s', 'foobar'), ('c', 'Return')]),
+            # Check 'replace' correctly takes into account
+            # the previous translation.
+            ([
+                translation(english='test '),
+                translation(english='{^,}'),
+
+            ], [('s', 'test '), ('b', 1), ('s', ', ')]),
+            # While the previous translation must be taken into account,
+            # any meta-command must not be fired again.
+            ([
+                translation(english='{#Return}'),
+                translation(english='test'),
+
+            ], [('c', 'Return'), ('s', 'test ')]),
+        ):
+            output = CaptureOutput()
+            formatter = formatting.Formatter()
+            formatter.set_output(output)
+            formatter.set_space_placement('After Output')
+            prev = None
+            for t in translations:
+                formatter.format([], [t], prev)
+                prev = t
+            self.assertEqual(output.instructions, expected_instructions)
+
+    def test_undo_replace(self):
+            # Undoing a replace....
+            output = CaptureOutput()
+            formatter = formatting.Formatter()
+            formatter.set_output(output)
+            formatter.set_space_placement('After Output')
+            prev = translation(english='test')
+            formatter.format([], [prev], None)
+            undo = translation(english='{^,}')
+            formatter.format([], [undo], prev)
+            # Undo.
+            formatter.format([undo], [], prev)
+            self.assertEqual(output.instructions, [
+                ('s', 'test '), ('b', 1), ('s', ', '), ('b', 2), ('s', ' '),
+            ])
+
+    def test_output_optimisation(self):
+        for undo, do, expected_instructions in (
+            # No change.
+            ([
+                translation(english='noop'),
+            ], [
+                translation(english='noop'),
+
+            ], [('s', ' noop')]),
+            # Append only.
+            ([
+                translation(english='test'),
+            ], [
+                translation(english='testing'),
+
+            ], [('s', ' test'), ('s', 'ing')]),
+            # Chained meta-commands.
+            ([
+                translation(english='{#a}'),
+            ], [
+                translation(english='{#a}{#b}'),
+
+            ], [('c', 'a'), ('c', 'b')]),
+        ):
+            output = CaptureOutput()
+            formatter = formatting.Formatter()
+            formatter.set_output(output)
+            formatter.format([], undo, None)
+            formatter.format(undo, do, None)
+            self.assertEqual(output.instructions, expected_instructions)
+
 if __name__ == '__main__':
     unittest.main()