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 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) diff --git a/plover/dictionary/json_dict.py b/plover/dictionary/json_dict.py index 5f4263e4a..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,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=(',', ': ')) diff --git a/plover/dictionary/rtfcre_dict.py b/plover/dictionary/rtfcre_dict.py index 833af406f..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=normalize_steno)) + with StenoNormalizer(filename) as normalize_steno: + self.update(parse_rtfcre(text, normalize=normalize_steno)) 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..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, @@ -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 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