Better steno errors handling

news.d/api/1362.break.md

@@ -0,0 +1 @@
+The `steno` helpers (`Stroke` class, `normalize_stroke`, …) now raise a `ValueError` exception in case of invalid steno.

news.d/api/1501.break.md

@@ -0,0 +1 @@
+The `steno` helpers (`Stroke` class, `normalize_stroke`, …) now raise a `ValueError` exception in case of invalid steno.

news.d/feature/1362.ui.md

@@ -1,3 +1,3 @@
 Improved steno handling:
 - validate inputs in the "add translation" dialog and dictionary editor
-- sort on steno order in the dictionary editor
+- sort on steno order in the dictionary editor, and signal invalid steno entries

news.d/feature/1501.ui.md

@@ -0,0 +1,3 @@
+Improved steno handling:
+- validate inputs in the "add translation" dialog and dictionary editor
+- sort on steno order in the dictionary editor, and signal invalid steno entries

plover/dictionary/helpers.py

@@ -0,0 +1,25 @@
+from plover import _, log
+from plover.misc import shorten_path
+from plover.steno import normalize_steno
+
+
+class StenoNormalizer:
+
+    def __init__(self, dictionary_path):
+        self._dictionary_path = dictionary_path
+        self._errors_count = 0
+
+    def normalize(self, steno):
+        try:
+            return normalize_steno(steno)
+        except ValueError:
+            self._errors_count += 1
+            return tuple(steno.split('/'))
+
+    def __enter__(self):
+        return self.normalize
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        if exc_type is None and self._errors_count:
+            log.warning(_('dictionary `%s` loaded with %u invalid steno errors'),
+                        shorten_path(self._dictionary_path), self._errors_count)

plover/dictionary/json_dict.py

@@ -10,8 +10,9 @@
 except ImportError:
     import json
 
+from plover.dictionary.helpers import StenoNormalizer
 from plover.steno_dictionary import StenoDictionary
-from plover.steno import normalize_steno, steno_to_sort_key
+from plover.steno import steno_to_sort_key
 
 
 class JsonDictionary(StenoDictionary):
@@ -29,11 +30,12 @@ def _load(self, filename):
         else:
             raise ValueError('\'%s\' encoding could not be determined' % (filename,))
         d = dict(json.loads(contents))
-        self.update((normalize_steno(x[0]), x[1]) for x in d.items())
+        with StenoNormalizer(filename) as normalize_steno:
+            self.update((normalize_steno(x[0]), x[1]) for x in d.items())
 
     def _save(self, filename):
         mappings = [('/'.join(k), v) for k, v in self.items()]
-        mappings.sort(key=lambda i: steno_to_sort_key(i[0]))
+        mappings.sort(key=lambda i: steno_to_sort_key(i[0], strict=False))
         with open(filename, 'w', encoding='utf-8', newline='\n') as fp:
             json.dump(dict(mappings), fp, ensure_ascii=False,
                       indent=0, separators=(',', ': '))

plover/dictionary/rtfcre_dict.py

@@ -17,8 +17,8 @@
 import string
 
 from plover import __version__ as plover_version
+from plover.dictionary.helpers import StenoNormalizer
 from plover.formatting import ATOM_RE
-from plover.steno import normalize_steno
 from plover.steno_dictionary import StenoDictionary
 
 from .rtfcre_parse import parse_rtfcre
@@ -135,7 +135,8 @@ class RtfDictionary(StenoDictionary):
     def _load(self, filename):
         with open(filename, 'rb') as fp:
             text = fp.read().decode('cp1252')
-        self.update(parse_rtfcre(text, normalize=normalize_steno))
+        with StenoNormalizer(filename) as normalize_steno:
+            self.update(parse_rtfcre(text, normalize=normalize_steno))
 
     def _save(self, filename):
         translation_formatter = TranslationFormatter()

plover/gui_qt/dictionary_editor.py

@@ -8,6 +8,7 @@
     QModelIndex,
     Qt,
 )
+from PyQt5.QtGui import QIcon
 from PyQt5.QtWidgets import (
     QComboBox,
     QDialog,
@@ -31,7 +32,7 @@ class DictionaryItem(namedtuple('DictionaryItem', 'steno translation dictionary'
 
     @property
     def strokes(self):
-        return normalize_steno(self.steno)
+        return normalize_steno(self.steno, strict=False)
 
     @property
     def dictionary_path(self):
@@ -64,6 +65,7 @@ class DictionaryItemModel(QAbstractTableModel):
 
     def __init__(self, dictionary_list, sort_column, sort_order):
         super().__init__()
+        self._error_icon = QIcon(':/dictionary_error.svg')
         self._dictionary_list = dictionary_list
         self._operations = []
         self._entries = []
@@ -179,10 +181,17 @@ def headerData(self, section, orientation, role):
             return _('Dictionary')
 
     def data(self, index, role):
-        if not index.isValid() or role not in (Qt.EditRole, Qt.DisplayRole):
+        if not index.isValid() or role not in (Qt.EditRole, Qt.DisplayRole, Qt.DecorationRole):
             return None
         item = self._entries[index.row()]
         column = index.column()
+        if role == Qt.DecorationRole:
+            if column == _COL_STENO:
+                try:
+                    normalize_steno(item.steno)
+                except ValueError:
+                    return self._error_icon
+            return None
         if column == _COL_STENO:
             return item.steno
         if column == _COL_TRANS:
@@ -206,7 +215,7 @@ def filter(self, strokes_filter=None, translation_filter=None):
 
     @staticmethod
     def _item_steno_sort_key(item):
-        return steno_to_sort_key(item[_COL_STENO])
+        return steno_to_sort_key(item[_COL_STENO], strict=False)
 
     def sort(self, column, order):
         self.layoutAboutToBeChanged.emit()
@@ -230,7 +239,7 @@ def setData(self, index, value, role=Qt.EditRole, record=True):
         strokes = old_item.strokes
         steno, translation, dictionary = old_item
         if column == _COL_STENO:
-            strokes = normalize_steno(value.strip())
+            strokes = normalize_steno(value.strip(), strict=False)
             steno = '/'.join(strokes)
             if not steno or steno == old_item.steno:
                 return False

plover/gui_qt/steno_validator.py

@@ -17,7 +17,7 @@ def validate(self, text, pos):
                 state = QValidator.Intermediate
                 steno = prefix
             try:
-                normalize_steno(steno, strict=True)
+                normalize_steno(steno)
             except ValueError:
                 state = QValidator.Invalid
         return state, text, pos

plover/steno.py

@@ -11,8 +11,6 @@
 
 from plover_stroke import BaseStroke
 
-from plover import log
-
 
 class Stroke(BaseStroke):
 
@@ -55,33 +53,30 @@ def from_integer(cls, integer):
         return int.__new__(cls._class, cls._helper.stroke_from_int(integer))
 
     @classmethod
-    def normalize_stroke(cls, steno, strict=False):
+    def normalize_stroke(cls, steno, strict=True):
         try:
             return cls._helper.normalize_stroke(steno)
         except ValueError:
             if strict:
                 raise
-            log.error(exc_info=True)
             return steno
 
     @classmethod
-    def normalize_steno(cls, steno, strict=False):
+    def normalize_steno(cls, steno, strict=True):
         try:
             return cls._helper.normalize_steno(steno)
         except ValueError:
             if strict:
                 raise
-            log.error('', exc_info=True)
             return tuple(steno.split('/'))
 
     @classmethod
-    def steno_to_sort_key(cls, steno, strict=False):
+    def steno_to_sort_key(cls, steno, strict=True):
         try:
             return cls._helper.steno_to_sort_key(steno)
         except ValueError:
             if strict:
                 raise
-            log.error('', exc_info=True)
             return b'\x00\x00' + steno.encode('utf-8')
 
     def __new__(cls, value):

test/test_steno.py

@@ -13,7 +13,6 @@
 
 
 NORMALIZE_TESTS = (
-    # TODO: More cases
     lambda: ('S', ('S',)),
     lambda: ('S-', ('S',)),
     lambda: ('-S', ('-S',)),
@@ -52,30 +51,26 @@
     lambda: ('O', ('O',)),
     lambda: ('O-', ('O',)),
     lambda: ('S*-R', ('S*R',)),
-    # Invalid, no strict error checking.
-    lambda: ('SRALD/invalid', ('SRALD', 'invalid')),
-    lambda: ('SRALD//invalid', ('SRALD', '', 'invalid')),
-    lambda: ('S-*R', ('S-*R',)),
-    lambda: ('-O-', ('-O-',)),
-    lambda: ('-O', ('-O',)),
-    # Invalid, with strick error checking.
-    lambda: ('SRALD/invalid', ValueError),
-    lambda: ('SRALD//invalid', ValueError),
-    lambda: ('S-*R', ValueError),
-    lambda: ('-O-', ValueError),
-    lambda: ('-O', ValueError),
+    # Invalid.
+    lambda: ('SRALD/invalid', (ValueError, ('SRALD', 'invalid'))),
+    lambda: ('SRALD//invalid', (ValueError, ('SRALD', '', 'invalid'))),
+    lambda: ('S-*R', (ValueError, ('S-*R',))),
+    lambda: ('-O-', (ValueError, ('-O-',))),
+    lambda: ('-O', (ValueError, ('-O',))),
 )
 
 @parametrize(NORMALIZE_TESTS)
-def test_normalize_steno(steno, expected):
-    if inspect.isclass(expected):
-        with pytest.raises(expected):
-            normalize_steno(steno, strict=True)
-        return
-    result = normalize_steno(steno)
-    msg = 'normalize_steno(%r)=%r != %r' % (
-        steno, result, expected,
-    )
+@pytest.mark.parametrize('mode', ('strict=False', 'strict=True'))
+def test_normalize_steno(mode, steno, expected):
+    kwargs = eval('dict(' + mode + ')')
+    if inspect.isclass(expected[0]):
+        if kwargs['strict']:
+            with pytest.raises(expected[0]):
+                normalize_steno(steno)
+            return
+        expected = expected[1]
+    result = normalize_steno(steno, **kwargs)
+    msg = 'normalize_steno(%r, %s)=%r != %r' % (steno, mode, result, expected)
     assert result == expected, msg