Merge pull request #1375 from benoit-pierre/fix_1374

steno_dictionary: drop support for longest key callbacks

Author: benoit-pierre

news.d/api/1375.break.md

@@ -0,0 +1 @@
+The support for `StenoDictionary` and `StenoDictionaryCollection` longest key callbacks is gone, use the `longest_key` properties instead.

news.d/bugfix/1375.core.md

@@ -0,0 +1 @@
+Fixed a memory leak on reloading externally modified dictionaries.

plover/steno_dictionary.py

@@ -30,8 +30,7 @@ class StenoDictionary:
 
     def __init__(self):
         self._dict = {}
-        self._longest_key_length = 0
-        self._longest_listener_callbacks = set()
+        self._longest_key = 0
         self.reverse = collections.defaultdict(list)
         # Case-insensitive reverse dict
         self.casereverse = collections.defaultdict(list)
@@ -165,40 +164,25 @@ def casereverse_lookup(self, value):
         return set(self.casereverse.get(value, ()))
 
     @property
-    def _longest_key(self):
-        return self._longest_key_length
-
-    @_longest_key.setter
-    def _longest_key(self, longest_key):
-        if longest_key == self._longest_key_length:
-            return
-        self._longest_key_length = longest_key
-        for callback in self._longest_listener_callbacks:
-            callback(longest_key)
-
-    def add_longest_key_listener(self, callback):
-        self._longest_listener_callbacks.add(callback)
-
-    def remove_longest_key_listener(self, callback):
-        self._longest_listener_callbacks.remove(callback)
+    def longest_key(self):
+        return self._longest_key
 
 
 class StenoDictionaryCollection:
 
     def __init__(self, dicts=[]):
         self.dicts = []
         self.filters = []
-        self.longest_key = 0
-        self.longest_key_callbacks = set()
         self.set_dicts(dicts)
 
+    @property
+    def longest_key(self):
+        if not self.dicts:
+            return 0
+        return max(d.longest_key for d in self.dicts)
+
     def set_dicts(self, dicts):
-        for d in self.dicts:
-            d.remove_longest_key_listener(self._longest_key_listener)
         self.dicts = dicts[:]
-        for d in self.dicts:
-            d.add_longest_key_listener(self._longest_key_listener)
-        self._longest_key_listener()
 
     def _lookup(self, key, dicts=None, filters=()):
         if dicts is None:
@@ -320,19 +304,3 @@ def add_filter(self, f):
 
     def remove_filter(self, f):
         self.filters.remove(f)
-
-    def add_longest_key_listener(self, callback):
-        self.longest_key_callbacks.add(callback)
-
-    def remove_longest_key_listener(self, callback):
-        self.longest_key_callbacks.remove(callback)
-    
-    def _longest_key_listener(self, ignored=None):
-        if self.dicts:
-            new_longest_key = max(d.longest_key for d in self.dicts)
-        else:
-            new_longest_key = 0
-        if new_longest_key != self.longest_key:
-            self.longest_key = new_longest_key
-            for c in self.longest_key_callbacks:
-                c(new_longest_key)

plover/translation.py

@@ -208,11 +208,7 @@ def translate(self, stroke):
 
     def set_dictionary(self, d):
         """Set the dictionary."""
-        callback = self._dict_callback
-        if self._dictionary:
-            self._dictionary.remove_longest_key_listener(callback)
         self._dictionary = d
-        d.add_longest_key_listener(callback)
 
     def get_dictionary(self):
         return self._dictionary
@@ -275,9 +271,6 @@ def _resize_translations(self):
         self._state.restrict_size(max(self._dictionary.longest_key,
                                       self._undo_length))
 
-    def _dict_callback(self, value):
-        self._resize_translations()
-
     def get_state(self):
         """Get the state of the translator."""
         return self._state
@@ -342,9 +335,10 @@ def _find_translation_helper(self, stroke, suffixes=()):
         # stroke and build the stroke list for translation.
         num_strokes = 1
         translation_count = 0
+        longest_key = self._dictionary.longest_key
         for t in reversed(self._state.translations):
             num_strokes += len(t)
-            if num_strokes > self._dictionary.longest_key:
+            if num_strokes > longest_key:
                 break
             translation_count += 1
         translation_index = len(self._state.translations) - translation_count

plover_build_utils/testing/steno_dictionary.py

@@ -204,31 +204,22 @@ class _WritableDictionaryTests:
 
     def test_longest_key(self):
         '''
-        Check `longest_key` support (including callbacks handling).
+        Check `longest_key` support.
         '''
         assert self.DICT_SUPPORT_SEQUENCE_METHODS
-        notifications = []
-        def listener(longest_key):
-            notifications.append(longest_key)
         d = self.DICT_CLASS()
         assert d.longest_key == 0
-        d.add_longest_key_listener(listener)
         d[('S',)] = 'a'
         assert d.longest_key == 1
-        assert notifications == [1]
         d[('S', 'S', 'S', 'S')] = 'b'
         assert d.longest_key == 4
-        assert notifications == [1, 4]
         d[('S', 'S')] = 'c'
         assert d.longest_key == 4
         assert d[('S', 'S')] == 'c'
-        assert notifications == [1, 4]
         del d[('S', 'S', 'S', 'S')]
         assert d.longest_key == 2
-        assert notifications == [1, 4, 2]
         del d[('S',)]
         assert d.longest_key == 2
-        assert notifications == [1, 4, 2]
         if self.DICT_SUPPORT_REVERSE_LOOKUP:
             assert d.reverse_lookup('c') == {('S', 'S')}
         else:
@@ -239,13 +230,10 @@ def listener(longest_key):
             assert d.casereverse_lookup('c') == set()
         d.clear()
         assert d.longest_key == 0
-        assert notifications == [1, 4, 2, 0]
         assert d.reverse_lookup('c') == set()
         assert d.casereverse_lookup('c') == set()
-        d.remove_longest_key_listener(listener)
         d[('S', 'S')] = 'c'
         assert d.longest_key == 2
-        assert notifications == [1, 4, 2, 0]
 
     def test_casereverse_del(self):
         '''

test/test_translation.py

@@ -9,6 +9,9 @@
 
 from plover.oslayer.config import PLATFORM
 from plover.steno import Stroke, normalize_steno
+
+import pytest
+
 from plover.steno_dictionary import StenoDictionary, StenoDictionaryCollection
 from plover.translation import Translation, Translator, _State
 from plover.translation import escape_translation, unescape_translation
@@ -63,28 +66,34 @@ def setup_method(self):
         self.dc = StenoDictionaryCollection([self.d])
         self.t.set_dictionary(self.dc)
 
-    def test_dictionary_update_grows_size1(self):
-        self.d[('S',)] = '1'
-        self._check_size_call(1)
-
-    def test_dictionary_update_grows_size4(self):
-        self.d[('S', 'PT', '-Z', 'TOP')] = 'hi'
-        self._check_size_call(4)
+    @pytest.mark.parametrize('key', (
+        ('S',),
+        ('S', 'PT', '-Z', 'TOP'),
+    ))
+    def test_dictionary_update_grows_size(self, key):
+        self.d[key] = 'key'
+        self.t.translate(stroke('T-'))
+        self._check_size_call(len(key))
 
     def test_dictionary_update_no_grow(self):
         self.t.set_min_undo_length(4)
         self._check_size_call(4)
         self.clear()
         self.d[('S', 'T')] = 'nothing'
+        self.t.translate(stroke('T-'))
         self._check_size_call(4)
 
     def test_dictionary_update_shrink(self):
         self.d[('S', 'T', 'P', '-Z', '-D')] = '1'
+        self.t.translate(stroke('T-'))
         self._check_size_call(5)
         self.clear()
         self.d[('A', 'P')] = '2'
-        self._check_no_size_call()
+        self.t.translate(stroke('T-'))
+        self._check_size_call(5)
+        self.clear()
         del self.d[('S', 'T', 'P', '-Z', '-D')]
+        self.t.translate(stroke('T-'))
         self._check_size_call(2)
 
     def test_dictionary_update_no_shrink(self):