Check context file without blocking GUI thread

damnit/gui/editor.py

@@ -4,8 +4,8 @@
 from pathlib import Path
 from tempfile import NamedTemporaryFile
 
-from PyQt5.QtCore import Qt
-from PyQt5.QtGui import QColor, QFont, QGuiApplication, QCursor
+from PyQt5.QtCore import Qt, QThread, pyqtSignal
+from PyQt5.QtGui import QColor, QFont
 from PyQt5.Qsci import QsciScintilla, QsciLexerPython, QsciCommand
 
 from pyflakes.reporter import Reporter
@@ -21,7 +21,64 @@ class ContextTestResult(Enum):
     WARNING = 1
     ERROR = 2
 
+
+class ContextFileCheckerThread(QThread):
+    # ContextTestResult, traceback, lineno, offset, checked_code
+    check_result = pyqtSignal(object, str, int, int, str)
+
+    def __init__(self, code, context_python, parent=None):
+        super().__init__(parent)
+        self.code = code
+        self.context_python = context_python
+
+    def run(self):
+        error_info = None
+
+        # If a different environment is not specified, we can evaluate the
+        # context file directly.
+        if self.context_python is None:
+            try:
+                ContextFile.from_str(self.code)
+            except:
+                # Extract the error information
+                error_info = extract_error_info(*sys.exc_info())
+
+        # Otherwise, write it to a temporary file to evaluate it from another
+        # process.
+        else:
+            with NamedTemporaryFile(prefix=".tmp_ctx") as ctx_file:
+                ctx_path = Path(ctx_file.name)
+                ctx_path.write_text(self.code)
+
+                ctx, error_info = get_context_file(ctx_path, self.context_python)
+
+        if error_info is not None:
+            stacktrace, lineno, offset = error_info
+            self.check_result.emit(ContextTestResult.ERROR, stacktrace, lineno, offset, self.code)
+            return
+
+        # If that worked, try pyflakes
+        out_buffer = StringIO()
+        reporter = Reporter(out_buffer, out_buffer)
+        pyflakes_check(self.code, "<ctx>", reporter)
+        # Disgusting hack to avoid getting warnings for "var#foo", "meta#foo",
+        # and "mymdc#foo" type annotations. This needs some tweaking to avoid
+        # missing real errors.
+        pyflakes_output = "\n".join([line for line in out_buffer.getvalue().split("\n")
+                                     if not line.endswith("undefined name 'var'") \
+                                     and not line.endswith("undefined name 'meta'") \
+                                     and not line.endswith("undefined name 'mymdc'")])
+
+        if len(pyflakes_output) > 0:
+            res, info = ContextTestResult.WARNING, pyflakes_output
+        else:
+            res, info = ContextTestResult.OK, None
+        self.check_result.emit(res, info, -1, -1, self.code)
+
+
 class Editor(QsciScintilla):
+    check_result = pyqtSignal(object, str, str)  # result, info, checked_code
+
     def __init__(self):
         super().__init__()
 
@@ -50,38 +107,15 @@ def __init__(self):
         line_del = commands.find(QsciCommand.LineDelete)
         line_del.setKey(Qt.ControlModifier | Qt.Key_D)
 
-    def test_context(self, db, db_dir):
-        """
-        Check if the current context file is valid.
-
-        Returns a tuple of (result, output_msg).
-        """
-        error_info = None
+    def launch_test_context(self, db):
         context_python = db.metameta.get("context_python")
+        thread = ContextFileCheckerThread(self.text(), context_python, parent=self)
+        thread.check_result.connect(self.on_test_result)
+        thread.finished.connect(thread.deleteLater)
+        thread.start()
 
-        # If a different environment is not specified, we can evaluate the
-        # context file directly.
-        if context_python is None:
-            try:
-                ContextFile.from_str(self.text())
-            except:
-                # Extract the error information
-                error_info = extract_error_info(*sys.exc_info())
-
-        # Otherwise, write it to a temporary file to evaluate it from another
-        # process.
-        else:
-            with NamedTemporaryFile(prefix=".tmp_ctx", dir=db_dir) as ctx_file:
-                ctx_path = Path(ctx_file.name)
-                ctx_path.write_text(self.text())
-
-                QGuiApplication.setOverrideCursor(QCursor(Qt.WaitCursor))
-                ctx, error_info = get_context_file(ctx_path, context_python)
-                QGuiApplication.restoreOverrideCursor()
-
-        if error_info is not None:
-            stacktrace, lineno, offset = error_info
-
+    def on_test_result(self, res, info, lineno, offset, checked_code):
+        if res is ContextTestResult.ERROR:
             if lineno != -1:
                 # The line numbers reported by Python are 1-indexed so we
                 # decrement before passing them to scintilla.
@@ -93,21 +127,4 @@ def test_context(self, db, db_dir):
                 if lineno != self.getCursorPosition()[0]:
                     self.setCursorPosition(lineno, offset)
 
-            return ContextTestResult.ERROR, stacktrace
-
-        # If that worked, try pyflakes
-        out_buffer = StringIO()
-        reporter = Reporter(out_buffer, out_buffer)
-        pyflakes_check(self.text(), "<ctx>", reporter)
-        # Disgusting hack to avoid getting warnings for "var#foo", "meta#foo",
-        # and "mymdc#foo" type annotations. This needs some tweaking to avoid
-        # missing real errors.
-        pyflakes_output = "\n".join([line for line in out_buffer.getvalue().split("\n")
-                                     if not line.endswith("undefined name 'var'") \
-                                     and not line.endswith("undefined name 'meta'") \
-                                     and not line.endswith("undefined name 'mymdc'")])
-
-        if len(pyflakes_output) > 0:
-            return ContextTestResult.WARNING, pyflakes_output
-        else:
-            return ContextTestResult.OK, None
+        self.check_result.emit(res, info, checked_code)

damnit/gui/main_window.py

@@ -49,6 +49,7 @@ class Settings(Enum):
 class MainWindow(QtWidgets.QMainWindow):
 
     context_dir_changed = QtCore.pyqtSignal(str)
+    save_context_finished = QtCore.pyqtSignal(bool)  # True if saved
 
     db = None
     db_id = None
@@ -62,6 +63,7 @@ def __init__(self, context_dir: Path = None, connect_to_kafka: bool = True):
         self._received_update = False
         self._context_path = None
         self._context_is_saved = True
+        self._context_code_to_save = None
 
         self._settings_db_path = Path.home() / ".local" / "state" / "damnit" / "settings.db"
 
@@ -109,13 +111,11 @@ def closeEvent(self, event):
         if not self._context_is_saved:
             dialog = QMessageBox(QMessageBox.Warning,
                                  "Warning - unsaved changes",
-                                 "There are unsaved changes to the context, do you want to save before exiting?",
-                                 QMessageBox.Save | QMessageBox.Discard | QMessageBox.Cancel)
+                                 "There are unsaved changes to the context, do you want to go back and save?",
+                                 QMessageBox.Discard | QMessageBox.Cancel)
             result = dialog.exec()
 
-            if result == QMessageBox.Save:
-                self.save_context()
-            elif result == QMessageBox.Cancel:
+            if result == QMessageBox.Cancel:
                 event.ignore()
                 return
 
@@ -741,6 +741,7 @@ def configure_editor(self):
         test_widget = QtWidgets.QWidget()
 
         self._editor.textChanged.connect(self.on_context_changed)
+        self._editor.check_result.connect(self.test_context_result)
 
         vbox = QtWidgets.QGridLayout()
         test_widget.setLayout(vbox)
@@ -796,7 +797,17 @@ def reload_context(self):
         self.mark_context_saved()
 
     def test_context(self):
-        test_result, output = self._editor.test_context(self.db, self._context_path.parent)
+        self.set_error_icon('wait')
+        self._editor.launch_test_context(self.db)
+
+    def test_context_result(self, test_result, output, checked_code):
+        # want_save, self._context_save_wanted = self._context_save_wanted, False
+        if self._context_code_to_save == checked_code:
+            if saving := test_result is not ContextTestResult.ERROR:
+                self._context_path.write_text(self._context_code_to_save)
+                self.mark_context_saved()
+            self._context_code_to_save = None
+            self.save_context_finished.emit(saving)
 
         if test_result == ContextTestResult.ERROR:
             self.set_error_widget_text(output)
@@ -822,7 +833,6 @@ def test_context(self):
                 self.set_error_icon("green")
 
         self._editor.setFocus()
-        return test_result
 
     def set_error_icon(self, icon):
         self._context_status_icon.load(icon_path(f"{icon}_circle.svg"))
@@ -835,12 +845,9 @@ def set_error_widget_text(self, text):
         QtCore.QTimer.singleShot(100, lambda: self._error_widget.setText(text))
 
     def save_context(self):
-        if self.test_context() == ContextTestResult.ERROR:
-            return
-
-        self._context_path.write_text(self._editor.text())
-        self.mark_context_saved()
-        self._editor.setFocus()
+        self._context_code_to_save = self._editor.text()
+        self.test_context()
+        # If the check passes, .test_context_result() saves the file
 
     def mark_context_saved(self):
         self._context_is_saved = True

tests/test_gui.py

@@ -84,16 +84,22 @@ def test_editor(mock_db, mock_ctx, qtbot):
     assert "Ctrl + S" in status_bar.currentMessage()
 
     # Saving OK code should work
-    win._save_btn.clicked.emit()
-    assert editor.test_context(db, db_dir)[0] == ContextTestResult.OK
+    with qtbot.waitSignal(win.save_context_finished):
+        win._save_btn.clicked.emit()
+    assert ctx_path.read_text() == old_code
+
+    with qtbot.waitSignal(editor.check_result) as sig:
+        editor.launch_test_context(db)
+    assert sig.args[0] == ContextTestResult.OK
+
     assert ctx_path.read_text() == old_code
     assert status_bar.currentMessage() == str(ctx_path.resolve())
 
     # The Validate button should trigger validation. Note that we mock
     # editor.test_context() function instead of MainWindow.test_context()
     # because the win._check_btn.clicked has already been connected to the
     # original function, so mocking it will not make Qt call the mock object.
-    with patch.object(editor, "test_context", return_value=(None, None)) as test_context:
+    with patch.object(editor, "launch_test_context") as test_context:
         win._check_btn.clicked.emit()
         test_context.assert_called_once()
 
@@ -118,15 +124,15 @@ def test_editor(mock_db, mock_ctx, qtbot):
     qtbot.waitExposed(win)
     assert win.isVisible()
 
-    # 'Save' should close the window and save
-    with patch.object(QMessageBox, "exec", return_value=QMessageBox.Save):
-        win.close()
-        assert win.isHidden()
-        assert ctx_path.read_text() == new_code
+    # Save the valid code
+    with qtbot.waitSignal(win.save_context_finished):
+        win.save_context()
+    assert ctx_path.read_text() == new_code
 
     # Attempting to save ERROR'ing code should not save anything
     editor.setText("123 = 456")
-    win.save_context()
+    with qtbot.waitSignal(win.save_context_finished):
+        win.save_context()
     assert ctx_path.read_text() == new_code
 
     # But saving WARNING code should work
@@ -135,8 +141,11 @@ def test_editor(mock_db, mock_ctx, qtbot):
     x = 1
     """)
     editor.setText(warning_code)
-    assert editor.test_context(db, db_dir)[0] == ContextTestResult.WARNING
-    win.save_context()
+    with qtbot.waitSignal(editor.check_result) as sig:
+        editor.launch_test_context(db)
+    assert sig.args[0] == ContextTestResult.WARNING
+    with qtbot.waitSignal(editor.check_result):
+        win.save_context()
     assert ctx_path.read_text() == warning_code
 
 def test_settings(mock_db_with_data, mock_ctx, tmp_path, monkeypatch, qtbot):