diff --git a/qutebrowser/browser/adblock.py b/qutebrowser/browser/adblock.py index 9fc09bd7b..4d0c02b3d 100644 --- a/qutebrowser/browser/adblock.py +++ b/qutebrowser/browser/adblock.py @@ -27,7 +27,7 @@ import zipfile import fnmatch from qutebrowser.config import config -from qutebrowser.utils import objreg, standarddir, log, message +from qutebrowser.utils import objreg, standarddir, log, message, usertypes from qutebrowser.commands import cmdutils, cmdexc @@ -210,7 +210,8 @@ class HostBlocker: else: fobj = io.BytesIO() fobj.name = 'adblock: ' + url.host() - download = download_manager.get(url, fileobj=fobj, + target = usertypes.DownloadTarget.FileObj(fobj) + download = download_manager.get(url, target=target, auto_remove=True) self._in_progress.append(download) download.finished.connect( diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 0117026b3..6ad80d5db 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1221,15 +1221,23 @@ class CommandDispatcher: " as mhtml.") url = urlutils.qurl_from_user_input(url) urlutils.raise_cmdexc_if_invalid(url) - download_manager.get(url, filename=dest) + if dest is None: + target = None + else: + target = usertypes.DownloadTarget.File(dest) + download_manager.get(url, target=target) elif mhtml_: self._download_mhtml(dest) else: # FIXME:qtwebengine have a proper API for this tab = self._current_widget() page = tab._widget.page() # pylint: disable=protected-access + if dest is None: + target = None + else: + target = usertypes.DownloadTarget.File(dest) download_manager.get(self._current_url(), page=page, - filename=dest) + target=target) def _download_mhtml(self, dest=None): """Download the current page as an MHTML file, including all assets. diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index e5359dbde..f3c6b1cfe 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -784,7 +784,7 @@ class DownloadManager(QAbstractListModel): **kwargs: passed to get_request(). Return: - If the download could start immediately, (fileobj/filename given), + If the download could start immediately, (target given), the created DownloadItem. If not, None. @@ -795,23 +795,20 @@ class DownloadManager(QAbstractListModel): req = QNetworkRequest(url) return self.get_request(req, **kwargs) - def get_request(self, request, *, fileobj=None, filename=None, **kwargs): + def get_request(self, request, *, target=None, **kwargs): """Start a download with a QNetworkRequest. Args: request: The QNetworkRequest to download. - fileobj: The file object to write the answer to. - filename: A path to write the data to. + target: Where to save the download as usertypes.DownloadTarget. **kwargs: Passed to fetch_request. Return: - If the download could start immediately, (fileobj/filename given), + If the download could start immediately, (target given), the created DownloadItem. If not, None. """ - if fileobj is not None and filename is not None: # pragma: no cover - raise TypeError("Only one of fileobj/filename may be given!") # WORKAROUND for Qt corrupting data loaded from cache: # https://bugreports.qt.io/browse/QTBUG-42757 request.setAttribute(QNetworkRequest.CacheLoadControlAttribute, @@ -840,8 +837,7 @@ class DownloadManager(QAbstractListModel): suggested_fn = 'qutebrowser-download' return self.fetch_request(request, - fileobj=fileobj, - filename=filename, + target=target, suggested_filename=suggested_fn, **kwargs) @@ -864,28 +860,25 @@ class DownloadManager(QAbstractListModel): return self.fetch(reply, **kwargs) @pyqtSlot('QNetworkReply') - def fetch(self, reply, *, fileobj=None, filename=None, auto_remove=False, + def fetch(self, reply, *, target=None, auto_remove=False, suggested_filename=None, prompt_download_directory=None): """Download a QNetworkReply to disk. Args: reply: The QNetworkReply to download. - fileobj: The file object to write the answer to. - filename: A path to write the data to. + target: Where to save the download as usertypes.DownloadTarget. auto_remove: Whether to remove the download even if ui -> remove-finished-downloads is set to -1. Return: The created DownloadItem. """ - if fileobj is not None and filename is not None: # pragma: no cover - raise TypeError("Only one of fileobj/filename may be given!") if not suggested_filename: - if (filename is not None and - filename is not usertypes.OPEN_DOWNLOAD): - suggested_filename = os.path.basename(filename) - elif fileobj is not None and getattr(fileobj, 'name', None): - suggested_filename = fileobj.name + if isinstance(target, usertypes.DownloadTarget.File): + suggested_filename = os.path.basename(target.filename) + elif (isinstance(target, usertypes.DownloadTarget.FileObj) and + getattr(target.fileobj, 'name', None)): + suggested_filename = target.fileobj.name else: _, suggested_filename = http.parse_content_disposition(reply) log.downloads.debug("fetch: {} -> {}".format(reply.url(), @@ -917,14 +910,8 @@ class DownloadManager(QAbstractListModel): if not self._update_timer.isActive(): self._update_timer.start() - if fileobj is not None: - download.set_fileobj(fileobj) - download.autoclose = False - return download - - if filename is not None: - self.set_filename_for_download(download, suggested_filename, - filename) + if target is not None: + self._set_download_target(download, suggested_filename, target) return download # Neither filename nor fileobj were given, prepare a question @@ -935,14 +922,14 @@ class DownloadManager(QAbstractListModel): # User doesn't want to be asked, so just use the download_dir if filename is not None: - self.set_filename_for_download(download, suggested_filename, - filename) + target = usertypes.DownloadTarget.File(filename) + self._set_download_target(download, suggested_filename, target) return download # Ask the user for a filename self._postprocess_question(q) q.answered.connect( - functools.partial(self.set_filename_for_download, download, + functools.partial(self._set_download_target, download, suggested_filename)) q.cancelled.connect(download.cancel) download.cancelled.connect(q.abort) @@ -951,25 +938,27 @@ class DownloadManager(QAbstractListModel): return download - def set_filename_for_download(self, download, suggested_filename, - filename): - """Set the filename for a given download. - - This correctly handles the case where filename = OPEN_DOWNLOAD. + def _set_download_target(self, download, suggested_filename, target): + """Set the target for a given download. Args: download: The download to set the filename for. suggested_filename: The suggested filename. - filename: The filename as string or usertypes.OPEN_DOWNLOAD. + target: The usertypes.DownloadTarget for this download. """ - if filename is not usertypes.OPEN_DOWNLOAD: - download.set_filename(filename) - return - tmp_manager = objreg.get('temporary-downloads') - fobj = tmp_manager.get_tmpfile(suggested_filename) - download.finished.connect(download.open_file) - download.autoclose = True - download.set_fileobj(fobj) + if isinstance(target, usertypes.DownloadTarget.FileObj): + download.set_fileobj(target.fileobj) + download.autoclose = False + elif isinstance(target, usertypes.DownloadTarget.File): + download.set_filename(target.filename) + elif isinstance(target, usertypes.DownloadTarget.OpenDownload): + tmp_manager = objreg.get('temporary-downloads') + fobj = tmp_manager.get_tmpfile(suggested_filename) + download.finished.connect(download.open_file) + download.autoclose = True + download.set_fileobj(fobj) + else: + log.downloads.error("Unknown download target: {}".format(target)) def raise_no_download(self, count): """Raise an exception that the download doesn't exist. diff --git a/qutebrowser/browser/webkit/mhtml.py b/qutebrowser/browser/webkit/mhtml.py index a9b485e35..399516f9a 100644 --- a/qutebrowser/browser/webkit/mhtml.py +++ b/qutebrowser/browser/webkit/mhtml.py @@ -35,7 +35,8 @@ import email.message from PyQt5.QtCore import QUrl from qutebrowser.browser.webkit import webelem, downloads -from qutebrowser.utils import log, objreg, message, usertypes, utils, urlutils +from qutebrowser.utils import (log, objreg, message, usertypes, utils, + urlutils, usertypes) try: import cssutils @@ -343,7 +344,8 @@ class _Downloader: download_manager = objreg.get('download-manager', scope='window', window=self._win_id) - item = download_manager.get(url, fileobj=_NoCloseBytesIO(), + target = usertypes.DownloadTarget.FileObj(_NoCloseBytesIO()) + item = download_manager.get(url, target=target, auto_remove=True) self.pending_downloads.add((url, item)) item.finished.connect(functools.partial(self._finished, url, item)) diff --git a/qutebrowser/mainwindow/statusbar/prompter.py b/qutebrowser/mainwindow/statusbar/prompter.py index 2700a2220..3b70d6ac2 100644 --- a/qutebrowser/mainwindow/statusbar/prompter.py +++ b/qutebrowser/mainwindow/statusbar/prompter.py @@ -248,7 +248,8 @@ class Prompter(QObject): self._question.done() elif self._question.mode == usertypes.PromptMode.download: # User just entered a path for a download. - self._question.answer = prompt.lineedit.text() + target = usertypes.DownloadTarget.File(prompt.lineedit.text()) + self._question.answer = target modeman.maybe_leave(self._win_id, usertypes.KeyMode.prompt, 'prompt accept') self._question.done() @@ -298,7 +299,7 @@ class Prompter(QObject): if self._question.mode != usertypes.PromptMode.download: # We just ignore this if we don't have a download question. return - self._question.answer = usertypes.OPEN_DOWNLOAD + self._question.answer = usertypes.DownloadTarget.OpenDownload() modeman.maybe_leave(self._win_id, usertypes.KeyMode.prompt, 'download open') self._question.done() diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py index b21676fde..38831bd36 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -33,7 +33,6 @@ from qutebrowser.utils import log, qtutils, utils _UNSET = object() -OPEN_DOWNLOAD = object() def enum(name, items, start=1, is_int=False): @@ -257,6 +256,52 @@ LoadStatus = enum('LoadStatus', ['none', 'success', 'success_https', 'error', Backend = enum('Backend', ['QtWebKit', 'QtWebEngine']) +# Where a download should be saved +class DownloadTarget: + + """Augmented enum that directs how a download should be saved. + + Objects of this class cannot be instantiated directly, use the "subclasses" + instead. + """ + + def __init__(self): + raise NotImplementedError + + # Due to technical limitations, these can't be actual subclasses without a + # workaround. But they should still be part of DownloadTarget to get the + # enum-like access (usertypes.DownloadTarget.File, like + # usertypes.PromptMode.download). + + class File: + + """Save the download to the given file. + + Attributes: + filename: Filename where the download should be saved. + """ + + def __init__(self, filename): + self.filename = filename + + class FileObj: + + """Save the download to the given file-like object. + + Attributes: + fileobj: File-like object where the download should be written to. + """ + + def __init__(self, fileobj): + self.fileobj = fileobj + + class OpenDownload: + + """Save the download in a temp dir and directly open it.""" + + pass + + class Question(QObject): """A question asked to the user, e.g. via the status bar.