From b6f9f34f224479dab710ec94687a5a91f9628b5d Mon Sep 17 00:00:00 2001
From: Benoit Pierre <benoit.pierre@gmail.com>
Date: Sat, 12 Mar 2022 01:25:50 +0100
Subject: [PATCH 1/6] steno: change helpers default to be strict

`normalize_stroke`, `normalize_steno`, and `steno_to_sort_key` now
default to strict mode, and don't log errors anymore.
---
 plover/dictionary/json_dict.py     |  4 +--
 plover/dictionary/rtfcre_dict.py   |  2 +-
 plover/gui_qt/dictionary_editor.py |  6 ++---
 plover/gui_qt/steno_validator.py   |  2 +-
 plover/steno.py                    | 11 +++------
 test/test_steno.py                 | 39 +++++++++++++-----------------
 6 files changed, 27 insertions(+), 37 deletions(-)

diff --git a/plover/dictionary/json_dict.py b/plover/dictionary/json_dict.py
index 5f4263e4a..8e27714fb 100644
--- a/plover/dictionary/json_dict.py
+++ b/plover/dictionary/json_dict.py
@@ -29,11 +29,11 @@ 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())
+        self.update((normalize_steno(x[0], strict=False), 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=(',', ': '))
diff --git a/plover/dictionary/rtfcre_dict.py b/plover/dictionary/rtfcre_dict.py
index 833af406f..054663a99 100644
--- a/plover/dictionary/rtfcre_dict.py
+++ b/plover/dictionary/rtfcre_dict.py
@@ -135,7 +135,7 @@ 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))
+        self.update(parse_rtfcre(text, normalize=lambda s: normalize_steno(s, strict=False)))
 
     def _save(self, filename):
         translation_formatter = TranslationFormatter()
diff --git a/plover/gui_qt/dictionary_editor.py b/plover/gui_qt/dictionary_editor.py
index bca7a21ca..3ca64af30 100644
--- a/plover/gui_qt/dictionary_editor.py
+++ b/plover/gui_qt/dictionary_editor.py
@@ -31,7 +31,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):
@@ -206,7 +206,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 +230,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
diff --git a/plover/gui_qt/steno_validator.py b/plover/gui_qt/steno_validator.py
index 687d3c429..7b620178f 100644
--- a/plover/gui_qt/steno_validator.py
+++ b/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
diff --git a/plover/steno.py b/plover/steno.py
index 5d1b05489..7a19b1d29 100644
--- a/plover/steno.py
+++ b/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):
diff --git a/test/test_steno.py b/test/test_steno.py
index 4b73c78b7..fa03cdc04 100644
--- a/test/test_steno.py
+++ b/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
 
 

From dadd1db5e7185f028381e74814415bc1e333a3ac Mon Sep 17 00:00:00 2001
From: Benoit Pierre <benoit.pierre@gmail.com>
Date: Sat, 12 Mar 2022 01:28:20 +0100
Subject: [PATCH 2/6] dictionary: add helpers

Only one helper for now: `StenoNormalizer`; to be used by dictionary
implementation to notify about invalid steno errors during load.  Instead
of (potentially) spamming the notification area with errors, only notify
once in case of errors, with the number of errors and dictionary name.
---
 plover/dictionary/helpers.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 plover/dictionary/helpers.py

diff --git a/plover/dictionary/helpers.py b/plover/dictionary/helpers.py
new file mode 100644
index 000000000..a725788d6
--- /dev/null
+++ b/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)

From 7a32f4eac245410ec52d998ee14431b6774dd72e Mon Sep 17 00:00:00 2001
From: Benoit Pierre <benoit.pierre@gmail.com>
Date: Sat, 12 Mar 2022 01:32:39 +0100
Subject: [PATCH 3/6] dictionary/json: use `StenoNormalizer` helper

---
 plover/dictionary/json_dict.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/plover/dictionary/json_dict.py b/plover/dictionary/json_dict.py
index 8e27714fb..1cf3026eb 100644
--- a/plover/dictionary/json_dict.py
+++ b/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,7 +30,8 @@ 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], strict=False), 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()]

From 8975e21417018ef295e5eb51ccb5a6e4d8aa6b17 Mon Sep 17 00:00:00 2001
From: Benoit Pierre <benoit.pierre@gmail.com>
Date: Sat, 12 Mar 2022 01:33:06 +0100
Subject: [PATCH 4/6] dictionary/rtfcre: use `StenoNormalizer` helper

---
 plover/dictionary/rtfcre_dict.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/plover/dictionary/rtfcre_dict.py b/plover/dictionary/rtfcre_dict.py
index 054663a99..80d793fa9 100644
--- a/plover/dictionary/rtfcre_dict.py
+++ b/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=lambda s: normalize_steno(s, strict=False)))
+        with StenoNormalizer(filename) as normalize_steno:
+            self.update(parse_rtfcre(text, normalize=normalize_steno))
 
     def _save(self, filename):
         translation_formatter = TranslationFormatter()

From c266ea5433a56f568745a17ddff0c932b01a138b Mon Sep 17 00:00:00 2001
From: Benoit Pierre <benoit.pierre@gmail.com>
Date: Sat, 12 Mar 2022 01:36:29 +0100
Subject: [PATCH 5/6] gui_qt/dictionary_editor: make invalid steno entries
 stand out

Add an error icon to those.
---
 plover/gui_qt/dictionary_editor.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/plover/gui_qt/dictionary_editor.py b/plover/gui_qt/dictionary_editor.py
index 3ca64af30..a32a7bfe0 100644
--- a/plover/gui_qt/dictionary_editor.py
+++ b/plover/gui_qt/dictionary_editor.py
@@ -8,6 +8,7 @@
     QModelIndex,
     Qt,
 )
+from PyQt5.QtGui import QIcon
 from PyQt5.QtWidgets import (
     QComboBox,
     QDialog,
@@ -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:

From 901261e888f11421e061b99f81b459ad05afe359 Mon Sep 17 00:00:00 2001
From: Benoit Pierre <benoit.pierre@gmail.com>
Date: Sat, 12 Mar 2022 01:57:43 +0100
Subject: [PATCH 6/6] add/update news entries

---
 news.d/api/1362.break.md  | 1 +
 news.d/api/1501.break.md  | 1 +
 news.d/feature/1362.ui.md | 2 +-
 news.d/feature/1501.ui.md | 3 +++
 4 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 news.d/api/1362.break.md
 create mode 100644 news.d/api/1501.break.md
 create mode 100644 news.d/feature/1501.ui.md

diff --git a/news.d/api/1362.break.md b/news.d/api/1362.break.md
new file mode 100644
index 000000000..e266f994a
--- /dev/null
+++ b/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.
diff --git a/news.d/api/1501.break.md b/news.d/api/1501.break.md
new file mode 100644
index 000000000..e266f994a
--- /dev/null
+++ b/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.
diff --git a/news.d/feature/1362.ui.md b/news.d/feature/1362.ui.md
index dc580279c..5bf137a75 100644
--- a/news.d/feature/1362.ui.md
+++ b/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
diff --git a/news.d/feature/1501.ui.md b/news.d/feature/1501.ui.md
new file mode 100644
index 000000000..5bf137a75
--- /dev/null
+++ b/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