downloads: introduce target= param for .get/.fetch

This parameter replaces the filename and fileobj parameters. This makes
it easier to add more download targets, since only one may be "chosen".
With the OPEN_DOWNLOAD special case added, handling of filename got a
bit ugly, since it may be either None, OPEN_DOWNLOAD or a str with the
file path, and we had to make sure only one target was chosen.

With the new target enum, this handling can be simplified and we
automatically get the guarantee that only one target is chosen.

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(

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.

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.
+            target: Where to save the download as usertypes.DownloadTarget.
-            filename: A path to write the data to.
             **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,
+                                  target=target,
-                                  filename=filename,
                                   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.
+            target: Where to save the download as usertypes.DownloadTarget.
-            filename: A path to write the data to.
             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
+            if isinstance(target, usertypes.DownloadTarget.File):
-                    filename is not usertypes.OPEN_DOWNLOAD):
+                suggested_filename = os.path.basename(target.filename)
-                suggested_filename = os.path.basename(filename)
+            elif (isinstance(target, usertypes.DownloadTarget.FileObj) and
-            elif fileobj is not None and getattr(fileobj, 'name', None):
+                  getattr(target.fileobj, 'name', None)):
-                suggested_filename = fileobj.name
+                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:
+        if target is not None:
-            download.set_fileobj(fileobj)
+            self._set_download_target(download, suggested_filename, target)
-            download.autoclose = False
-            return download
-
-        if filename is not None:
-            self.set_filename_for_download(download, suggested_filename,
-                                           filename)
             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,
+            target = usertypes.DownloadTarget.File(filename)
-                                           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,
+    def _set_download_target(self, download, suggested_filename, target):
-                                  filename):
+        """Set the target for a given download.
-        """Set the filename for a given download.
-
-        This correctly handles the case where filename = OPEN_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:
+        if isinstance(target, usertypes.DownloadTarget.FileObj):
-            download.set_filename(filename)
+            download.set_fileobj(target.fileobj)
-            return
+            download.autoclose = False
-        tmp_manager = objreg.get('temporary-downloads')
+        elif isinstance(target, usertypes.DownloadTarget.File):
-        fobj = tmp_manager.get_tmpfile(suggested_filename)
+            download.set_filename(target.filename)
-        download.finished.connect(download.open_file)
+        elif isinstance(target, usertypes.DownloadTarget.OpenDownload):
-        download.autoclose = True
+            tmp_manager = objreg.get('temporary-downloads')
-        download.set_fileobj(fobj)
+            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.

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))

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()

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.