From 17bb9fc21c3e364e36de42d2df2bd5c4ad0b625c Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 29 May 2015 23:46:07 +0200 Subject: [PATCH] Use QProcess instead of subprocess. Closes #646. Fixes #688. --- CONTRIBUTING.asciidoc | 4 +--- qutebrowser/browser/commands.py | 33 ++++++++++++----------------- qutebrowser/browser/hints.py | 20 +++++++++++------ qutebrowser/commands/userscripts.py | 19 ++--------------- qutebrowser/misc/editor.py | 15 +++---------- qutebrowser/utils/qtutils.py | 16 +++++++++++++- 6 files changed, 47 insertions(+), 60 deletions(-) diff --git a/CONTRIBUTING.asciidoc b/CONTRIBUTING.asciidoc index 1975a9d7c..8123514a9 100644 --- a/CONTRIBUTING.asciidoc +++ b/CONTRIBUTING.asciidoc @@ -238,9 +238,7 @@ There are some exceptions to that: * `QThread` is used instead of Python threads because it provides signals and slots. -* `QProcess` is used instead of Python's `subprocess` if certain actions (e.g. -cleanup) when the process finished are desired, as it provides signals for -that. +* `QProcess` is used instead of Python's `subprocess` * `QUrl` is used instead of storing URLs as string, see the <> section for details. diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 9bfe81a10..984963b19 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -22,14 +22,13 @@ import re import os import shlex -import subprocess import posixpath import functools import xml.etree.ElementTree from PyQt5.QtWebKit import QWebSettings from PyQt5.QtWidgets import QApplication, QTabBar -from PyQt5.QtCore import Qt, QUrl, QEvent +from PyQt5.QtCore import pyqtSlot, Qt, QUrl, QEvent, QProcess from PyQt5.QtGui import QClipboard, QKeyEvent from PyQt5.QtPrintSupport import QPrintDialog, QPrintPreviewDialog from PyQt5.QtWebKitWidgets import QWebPage @@ -929,11 +928,6 @@ class CommandDispatcher: Note the {url} variable which gets replaced by the current URL might be useful here. - // - - We use subprocess rather than Qt's QProcess here because we really - don't care about the process anymore as soon as it's spawned. - Args: userscript: Run the command as an userscript. quiet: Don't print the commandline being executed. @@ -944,16 +938,21 @@ class CommandDispatcher: if not quiet: fake_cmdline = ' '.join(shlex.quote(arg) for arg in args) message.info(win_id, 'Executing: ' + fake_cmdline) + cmd, *args = args if userscript: - cmd = args[0] - args = [] if not args else args[1:] self.run_userscript(cmd, *args) else: - try: - subprocess.Popen(args) - except OSError as e: - raise cmdexc.CommandError("Error while spawning command: " - "{}".format(e)) + proc = QProcess(self._tabbed_browser) + 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) @cmdutils.register(instance='command-dispatcher', scope='window') def home(self): @@ -1166,12 +1165,6 @@ class CommandDispatcher: The editor which should be launched can be configured via the `general -> editor` config option. - - // - - We use QProcess rather than subprocess here because it makes it a lot - easier to execute some code as soon as the process has been finished - and do everything async. """ frame = self._current_widget().page().currentFrame() try: diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index eef512f55..b9b1664b5 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -21,11 +21,10 @@ import math import functools -import subprocess import collections from PyQt5.QtCore import (pyqtSignal, pyqtSlot, QObject, QEvent, Qt, QUrl, - QTimer) + QTimer, QProcess) from PyQt5.QtGui import QMouseEvent, QClipboard from PyQt5.QtWidgets import QApplication from PyQt5.QtWebKit import QWebElement @@ -548,11 +547,18 @@ class HintManager(QObject): """ urlstr = url.toString(QUrl.FullyEncoded | QUrl.RemovePassword) args = context.get_args(urlstr) - try: - subprocess.Popen(args) - except OSError as e: - msg = "Error while spawning command: {}".format(e) - message.error(self._win_id, msg, immediately=True) + cmd, *args = args + proc = QProcess(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. diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index 76e9f94a6..ceff1dffc 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -26,7 +26,7 @@ import tempfile from PyQt5.QtCore import (pyqtSignal, pyqtSlot, QObject, QSocketNotifier, QProcessEnvironment, QProcess) -from qutebrowser.utils import message, log, objreg, standarddir +from qutebrowser.utils import message, log, objreg, standarddir, qtutils from qutebrowser.commands import runners, cmdexc from qutebrowser.config import config @@ -73,10 +73,6 @@ class _BaseUserscriptRunner(QObject): _proc: The QProcess which is being executed. _win_id: The window ID this runner is associated with. - Class attributes: - PROCESS_MESSAGES: A mapping of QProcess::ProcessError members to - human-readable error strings. - Signals: got_cmd: Emitted when a new command arrived and should be executed. finished: Emitted when the userscript finished running. @@ -85,17 +81,6 @@ class _BaseUserscriptRunner(QObject): got_cmd = pyqtSignal(str) finished = pyqtSignal() - PROCESS_MESSAGES = { - 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.", - } - def __init__(self, win_id, parent=None): super().__init__(parent) self._win_id = win_id @@ -166,7 +151,7 @@ class _BaseUserscriptRunner(QObject): def on_proc_error(self, error): """Called when the process encountered an error.""" - msg = self.PROCESS_MESSAGES[error] + msg = qtutils.QPROCESS_ERRORS[error] # NOTE: Do not replace this with "raise CommandError" as it's # executed async. message.error(self._win_id, diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index 32e4100ca..32d108490 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -25,7 +25,7 @@ import tempfile from PyQt5.QtCore import pyqtSignal, QProcess, QObject from qutebrowser.config import config -from qutebrowser.utils import message, log +from qutebrowser.utils import message, log, qtutils class ExternalEditor(QObject): @@ -96,20 +96,11 @@ class ExternalEditor(QObject): def on_proc_error(self, error): """Display an error message and clean up when editor crashed.""" - messages = { - 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.", - } + 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(messages[error])) + "Error while calling editor: {}".format(msg)) self._cleanup() def edit(self, text): diff --git a/qutebrowser/utils/qtutils.py b/qutebrowser/utils/qtutils.py index 6573306ab..42bca6bec 100644 --- a/qutebrowser/utils/qtutils.py +++ b/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) + QIODevice, QSaveFile, QProcess) from PyQt5.QtWidgets import QApplication @@ -400,3 +400,17 @@ 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.", +}