Add GUIProcess.

This aims to unify the code which spawns a process and then shows statusbar
notifications when it exited, etc.

qutebrowser/browser/commands.py

@@ -28,7 +28,7 @@ import xml.etree.ElementTree
 
 from PyQt5.QtWebKit import QWebSettings
 from PyQt5.QtWidgets import QApplication, QTabBar
-from PyQt5.QtCore import pyqtSlot, Qt, QUrl, QEvent, QProcess
+from PyQt5.QtCore import Qt, QUrl, QEvent
 from PyQt5.QtGui import QClipboard, QKeyEvent
 from PyQt5.QtPrintSupport import QPrintDialog, QPrintPreviewDialog
 from PyQt5.QtWebKitWidgets import QWebPage
@@ -43,7 +43,7 @@ from qutebrowser.keyinput import modeman
 from qutebrowser.utils import (message, usertypes, log, qtutils, urlutils,
                                objreg, utils)
 from qutebrowser.utils.usertypes import KeyMode
-from qutebrowser.misc import editor
+from qutebrowser.misc import editor, guiprocess
 
 
 class CommandDispatcher:
@@ -944,31 +944,13 @@ class CommandDispatcher:
         if userscript:
             self.run_userscript(cmd, *args)
         else:
-            proc = QProcess(self._tabbed_browser)
+            proc = guiprocess.GUIProcess(self._win_id, what='command',
-            proc.error.connect(self.on_process_error)
+                                         verbose=not quiet,
+                                         parent=self._tabbed_browser)
             if detach:
-                ok = proc.startDetached(cmd, args)
+                proc.start_detached(cmd, args)
-                if not ok:
-                    raise cmdexc.CommandError("Error while spawning command")
             else:
                 proc.start(cmd, args)
-            if not quiet:
-                proc.finished.connect(self.on_process_finished)
-
-    @pyqtSlot('QProcess::ProcessError')
-    def on_process_error(self, error):
-        """Display an error if a :spawn'ed process failed."""
-        msg = qtutils.QPROCESS_ERRORS[error]
-        message.error(self._win_id,
-                      "Error while spawning command: {}".format(msg),
-                      immediately=True)
-
-    @pyqtSlot(int, 'QProcess::ExitStatus')
-    def on_process_finished(self, code, _status):
-        """Display an error if a :spawn'ed process exited with non-0 status."""
-        if code != 0:
-            message.error(self._win_id, "Spawned command exited with status "
-                          "{}!".format(code))
 
     @cmdutils.register(instance='command-dispatcher', scope='window')
     def home(self):
@@ -1202,7 +1184,7 @@ class CommandDispatcher:
     def on_editing_finished(self, elem, text):
         """Write the editor text into the form field and clean up tempfile.
 
-        Callback for QProcess when the editor was closed.
+        Callback for GUIProcess when the editor was closed.
 
         Args:
             elem: The WebElementWrapper which was modified.

qutebrowser/browser/hints.py

@@ -24,7 +24,7 @@ import functools
 import collections
 
 from PyQt5.QtCore import (pyqtSignal, pyqtSlot, QObject, QEvent, Qt, QUrl,
-                          QTimer, QProcess)
+                          QTimer)
 from PyQt5.QtGui import QMouseEvent, QClipboard
 from PyQt5.QtWidgets import QApplication
 from PyQt5.QtWebKit import QWebElement
@@ -35,6 +35,7 @@ from qutebrowser.keyinput import modeman, modeparsers
 from qutebrowser.browser import webelem
 from qutebrowser.commands import userscripts, cmdexc, cmdutils, runners
 from qutebrowser.utils import usertypes, log, qtutils, message, objreg
+from qutebrowser.misc import guiprocess
 
 
 ElemTuple = collections.namedtuple('ElemTuple', ['elem', 'label'])
@@ -548,18 +549,9 @@ class HintManager(QObject):
         urlstr = url.toString(QUrl.FullyEncoded | QUrl.RemovePassword)
         args = context.get_args(urlstr)
         cmd, *args = args
-        proc = QProcess(self)
+        proc = guiprocess.GUIProcess(self._win_id, what='command', parent=self)
-        proc.error.connect(self.on_process_error)
         proc.start(cmd, args)
 
-    @pyqtSlot('QProcess::ProcessError')
-    def on_process_error(self, error):
-        """Display an error if a :spawn'ed process failed."""
-        msg = qtutils.QPROCESS_ERRORS[error]
-        message.error(self._win_id,
-                      "Error while spawning command: {}".format(msg),
-                      immediately=True)
-
     def _resolve_url(self, elem, baseurl):
         """Resolve a URL and check if we want to keep it.
 

qutebrowser/commands/userscripts.py

@@ -23,12 +23,12 @@ import os
 import os.path
 import tempfile
 
-from PyQt5.QtCore import (pyqtSignal, pyqtSlot, QObject, QSocketNotifier,
+from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QSocketNotifier
-                          QProcessEnvironment, QProcess)
 
-from qutebrowser.utils import message, log, objreg, standarddir, qtutils
+from qutebrowser.utils import message, log, objreg, standarddir
 from qutebrowser.commands import runners, cmdexc
 from qutebrowser.config import config
+from qutebrowser.misc import guiprocess
 
 
 class _QtFIFOReader(QObject):
@@ -70,7 +70,7 @@ class _BaseUserscriptRunner(QObject):
 
     Attributes:
         _filepath: The path of the file/FIFO which is being read.
-        _proc: The QProcess which is being executed.
+        _proc: The GUIProcess which is being executed.
         _win_id: The window ID this runner is associated with.
 
     Signals:
@@ -89,33 +89,29 @@ class _BaseUserscriptRunner(QObject):
         self._env = None
 
     def _run_process(self, cmd, *args, env):
-        """Start the given command via QProcess.
+        """Start the given command.
 
         Args:
             cmd: The command to be started.
             *args: The arguments to hand to the command
             env: A dictionary of environment variables to add.
         """
-        self._env = env
+        self._env = {'QUTE_FIFO': self._filepath}
-        self._proc = QProcess(self)
+        self._env.update(env)
-        procenv = QProcessEnvironment.systemEnvironment()
+        self._proc = guiprocess.GUIProcess(self._win_id, 'userscript',
-        procenv.insert('QUTE_FIFO', self._filepath)
+                                           additional_env=self._env,
-        if env is not None:
+                                           parent=self)
-            for k, v in env.items():
+        self._proc.proc.error.connect(self.on_proc_error)
-                procenv.insert(k, v)
+        self._proc.proc.finished.connect(self.on_proc_finished)
-        self._proc.setProcessEnvironment(procenv)
-        self._proc.error.connect(self.on_proc_error)
-        self._proc.finished.connect(self.on_proc_finished)
         self._proc.start(cmd, args)
 
     def _cleanup(self):
         """Clean up temporary files."""
         tempfiles = [self._filepath]
-        if self._env is not None:
+        if 'QUTE_HTML' in self._env:
-            if 'QUTE_HTML' in self._env:
+            tempfiles.append(self._env['QUTE_HTML'])
-                tempfiles.append(self._env['QUTE_HTML'])
+        if 'QUTE_TEXT' in self._env:
-            if 'QUTE_TEXT' in self._env:
+            tempfiles.append(self._env['QUTE_TEXT'])
-                tempfiles.append(self._env['QUTE_TEXT'])
         for fn in tempfiles:
             log.procs.debug("Deleting temporary file {}.".format(fn))
             try:
@@ -151,12 +147,7 @@ class _BaseUserscriptRunner(QObject):
 
     def on_proc_error(self, error):
         """Called when the process encountered an error."""
-        msg = qtutils.QPROCESS_ERRORS[error]
+        raise NotImplementedError
-        # NOTE: Do not replace this with "raise CommandError" as it's
-        # executed async.
-        message.error(self._win_id,
-                      "Error while calling userscript: {}".format(msg))
-        log.procs.debug("Userscript process error: {} - {}".format(error, msg))
 
 
 class _POSIXUserscriptRunner(_BaseUserscriptRunner):
@@ -195,12 +186,10 @@ class _POSIXUserscriptRunner(_BaseUserscriptRunner):
 
     def on_proc_finished(self):
         """Interrupt the reader when the process finished."""
-        log.procs.debug("Userscript process finished.")
         self.finish()
 
     def on_proc_error(self, error):
         """Interrupt the reader when the process had an error."""
-        super().on_proc_error(error)
         self.finish()
 
     def finish(self):
@@ -245,7 +234,6 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner):
 
     def on_proc_finished(self):
         """Read back the commands when the process finished."""
-        log.procs.debug("Userscript process finished.")
         try:
             with open(self._filepath, 'r', encoding='utf-8') as f:
                 for line in f:
@@ -257,7 +245,6 @@ class _WindowsUserscriptRunner(_BaseUserscriptRunner):
 
     def on_proc_error(self, error):
         """Clean up when the process had an error."""
-        super().on_proc_error(error)
         self._cleanup()
         self.finished.emit()
 

qutebrowser/misc/editor.py

@@ -22,10 +22,11 @@
 import os
 import tempfile
 
-from PyQt5.QtCore import pyqtSignal, QProcess, QObject
+from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QProcess
 
 from qutebrowser.config import config
-from qutebrowser.utils import message, log, qtutils
+from qutebrowser.utils import message, log
+from qutebrowser.misc import guiprocess
 
 
 class ExternalEditor(QObject):
@@ -36,7 +37,7 @@ class ExternalEditor(QObject):
         _text: The current text before the editor is opened.
         _oshandle: The OS level handle to the tmpfile.
         _filehandle: The file handle to the tmpfile.
-        _proc: The QProcess of the editor.
+        _proc: The GUIProcess of the editor.
         _win_id: The window ID the ExternalEditor is associated with.
     """
 
@@ -69,15 +70,10 @@ class ExternalEditor(QObject):
         log.procs.debug("Editor closed")
         if exitstatus != QProcess.NormalExit:
             # No error/cleanup here, since we already handle this in
-            # on_proc_error
+            # on_proc_error.
             return
         try:
             if exitcode != 0:
-                # NOTE: Do not replace this with "raise CommandError" as it's
-                # executed async.
-                message.error(
-                    self._win_id, "Editor did quit abnormally (status "
-                                  "{})!".format(exitcode))
                 return
             encoding = config.get('general', 'editor-encoding')
             try:
@@ -94,13 +90,8 @@ class ExternalEditor(QObject):
         finally:
             self._cleanup()
 
-    def on_proc_error(self, error):
+    @pyqtSlot(QProcess.ProcessError)
-        """Display an error message and clean up when editor crashed."""
+    def on_proc_error(self, _err):
-        msg = qtutils.QPROCESS_ERRORS[error]
-        # NOTE: Do not replace this with "raise CommandError" as it's
-        # executed async.
-        message.error(self._win_id,
-                      "Error while calling editor: {}".format(msg))
         self._cleanup()
 
     def edit(self, text):
@@ -123,9 +114,10 @@ class ExternalEditor(QObject):
             message.error(self._win_id, "Failed to create initial file: "
                                         "{}".format(e))
             return
-        self._proc = QProcess(self)
+        self._proc = guiprocess.GUIProcess(self._win_id, what='editor',
-        self._proc.finished.connect(self.on_proc_closed)
+                                           parent=self)
-        self._proc.error.connect(self.on_proc_error)
+        self._proc.proc.finished.connect(self.on_proc_closed)
+        self._proc.proc.error.connect(self.on_proc_error)
         editor = config.get('general', 'editor')
         executable = editor[0]
         args = [self._filename if arg == '{}' else arg for arg in editor[1:]]

qutebrowser/misc/guiprocess.py

@@ -0,0 +1,131 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2015 Florian Bruhin (The Compiler) <mail@qutebrowser.org>
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+"""A QProcess which shows notifications in the GUI."""
+
+import shlex
+
+from PyQt5.QtCore import pyqtSlot, QProcess, QIODevice, QProcessEnvironment
+
+from qutebrowser.utils import message, log
+
+# A mapping of QProcess::ErrorCode's to human-readable strings.
+
+ERROR_STRINGS = {
+    QProcess.FailedToStart: "The process failed to start.",
+    QProcess.Crashed: "The process crashed.",
+    QProcess.Timedout: "The last waitFor...() function timed out.",
+    QProcess.WriteError: ("An error occurred when attempting to write to the "
+                          "process."),
+    QProcess.ReadError: ("An error occurred when attempting to read from the "
+                         "process."),
+    QProcess.UnknownError: "An unknown error occurred.",
+}
+
+
+class GUIProcess:
+
+    """An external process which shows notifications in the GUI.
+
+    Args:
+        proc: The underlying QProcess.
+        _win_id: The window ID this process is used in.
+        _what: What kind of thing is spawned (process/editor/userscript/...).
+               Used in messages.
+        _verbose: Whether to show more messages.
+        _started: Whether the underlying process is started.
+    """
+
+    def __init__(self, win_id, what, *, verbose=False, additional_env=None,
+                 parent=None):
+        self._win_id = win_id
+        self._what = what
+        self._verbose = verbose
+        self._started = False
+
+        self.proc = QProcess(parent)
+        self.proc.error.connect(self.on_error)
+        self.proc.finished.connect(self.on_finished)
+        self.proc.started.connect(self.on_started)
+
+        if additional_env is not None:
+            procenv = QProcessEnvironment.systemEnvironment()
+            for k, v in additional_env.items():
+                procenv.insert(k, v)
+            self.proc.setProcessEnvironment(procenv)
+
+    @pyqtSlot(QProcess.ProcessError)
+    def on_error(self, error):
+        """Show a message if there was an error while spawning."""
+        msg = ERROR_STRINGS[error]
+        message.error(self._win_id, "Error while spawning {}: {}".format(
+                      self._what, msg), immediately=True)
+
+    @pyqtSlot(int, QProcess.ExitStatus)
+    def on_finished(self, code, status):
+        """Show a message when the process finished."""
+        self._started = False
+        log.procs.debug("Process finished with code {}, status {}.".format(
+            code, status))
+        if status == QProcess.CrashExit:
+            message.error(self._win_id,
+                          "{} crashed!".format(self._what.capitalize()),
+                          immediately=True)
+        elif status == QProcess.NormalExit and code == 0:
+            if self._verbose:
+                message.info(self._win_id, "{} exited successfully.".format(
+                    self._what.capitalize()))
+        else:
+            assert status == QProcess.NormalExit
+            message.error(self._win_id, "{} exited with status {}.".format(
+                self._what.capitalize(), code))
+
+    @pyqtSlot()
+    def on_started(self):
+        """Called when the process started successfully."""
+        log.procs.debug("Process started.")
+        assert not self._started
+        self._started = True
+
+    def _pre_start(self, cmd, args):
+        """Things to do before starting a QProcess."""
+        if self._started:
+            raise ValueError("Trying to start a running QProcess!")
+        if self._verbose:
+            fake_cmdline = ' '.join(shlex.quote(e) for e in [cmd] + list(args))
+            message.info(self._win_id, 'Executing: ' + fake_cmdline)
+
+    def start(self, cmd, args, mode=QIODevice.ReadWrite):
+        """Convenience wrapper around QProcess::start."""
+        log.procs.debug("Starting process.")
+        self._pre_start(cmd, args)
+        self.proc.start(cmd, args, mode)
+
+    def start_detached(self, cmd, args, cwd=None):
+        """Convenience wrapper around QProcess::startDetached."""
+        log.procs.debug("Starting detached.")
+        self._pre_start(cmd, args)
+        ok = self.proc.startDetached(cmd, args, cwd)
+
+        if ok:
+            log.procs.debug("Process started.")
+            self._started = True
+        else:
+            message.error(self._win_id, "Error while spawning {}: {}.".format(
+                          self._what, self.proc.error()), immediately=True)

qutebrowser/utils/qtutils.py

@@ -36,7 +36,7 @@ import distutils.version  # pylint: disable=no-name-in-module,import-error
 import contextlib
 
 from PyQt5.QtCore import (qVersion, QEventLoop, QDataStream, QByteArray,
-                          QIODevice, QSaveFile, QProcess)
+                          QIODevice, QSaveFile)
 from PyQt5.QtWidgets import QApplication
 
 
@@ -400,17 +400,3 @@ class EventLoop(QEventLoop):
         self._executing = True
         super().exec_(flags)
         self._executing = False
-
-
-# A mapping of QProcess::ErrorCode's to human-readable strings.
-
-QPROCESS_ERRORS = {
-    QProcess.FailedToStart: "The process failed to start.",
-    QProcess.Crashed: "The process crashed.",
-    QProcess.Timedout: "The last waitFor...() function timed out.",
-    QProcess.WriteError: ("An error occurred when attempting to write to the "
-                          "process."),
-    QProcess.ReadError: ("An error occurred when attempting to read from the "
-                         "process."),
-    QProcess.UnknownError: "An unknown error occurred.",
-}

tests/misc/test_editor.py

@@ -42,7 +42,7 @@ class TestArg:
 
     @pytest.yield_fixture(autouse=True)
     def setup(self, monkeypatch, stubs):
-        monkeypatch.setattr('qutebrowser.misc.editor.QProcess',
+        monkeypatch.setattr('qutebrowser.misc.editor.guiprocess.QProcess',
                             stubs.FakeQProcess())
         self.editor = editor.ExternalEditor(0)
         yield
@@ -101,7 +101,7 @@ class TestFileHandling:
     def setup(self, monkeypatch, stubs, config_stub):
         monkeypatch.setattr('qutebrowser.misc.editor.message',
                             stubs.MessageModule())
-        monkeypatch.setattr('qutebrowser.misc.editor.QProcess',
+        monkeypatch.setattr('qutebrowser.misc.editor.guiprocess.QProcess',
                             stubs.FakeQProcess())
         config_stub.data = {'general': {'editor': [''],
                                         'editor-encoding': 'utf-8'}}
@@ -148,7 +148,7 @@ class TestModifyTests:
 
     @pytest.fixture(autouse=True)
     def setup(self, monkeypatch, stubs, config_stub):
-        monkeypatch.setattr('qutebrowser.misc.editor.QProcess',
+        monkeypatch.setattr('qutebrowser.misc.editor.guiprocess.QProcess',
                             stubs.FakeQProcess())
         config_stub.data = {'general': {'editor': [''],
                                         'editor-encoding': 'utf-8'}}
@@ -220,7 +220,7 @@ class TestErrorMessage:
 
     @pytest.yield_fixture(autouse=True)
     def setup(self, monkeypatch, stubs, config_stub):
-        monkeypatch.setattr('qutebrowser.misc.editor.QProcess',
+        monkeypatch.setattr('qutebrowser.misc.editor.guiprocess.QProcess',
                             stubs.FakeQProcess())
         monkeypatch.setattr('qutebrowser.misc.editor.message',
                             stubs.MessageModule())