From c060f9e5c2b454c940ca677274267d0f2aebea44 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 5 Jul 2016 23:01:48 +0200 Subject: [PATCH 01/18] allow downloads to be openened directly The file is saved in a temporary directory, which is cleaned up when the window is closed. --- qutebrowser/browser/webkit/downloads.py | 94 ++++++++++++++------ qutebrowser/mainwindow/mainwindow.py | 1 + qutebrowser/mainwindow/statusbar/prompter.py | 28 ++++-- qutebrowser/utils/usertypes.py | 4 +- 4 files changed, 94 insertions(+), 33 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 9a1a49db8..86670986b 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -25,6 +25,7 @@ import sys import os.path import shutil import functools +import tempfile import collections import sip @@ -513,7 +514,13 @@ class DownloadItem(QObject): def open_file(self): """Open the downloaded file.""" assert self.successful - url = QUrl.fromLocalFile(self._filename) + filename = self._filename + if filename is None: + filename = getattr(self.fileobj, 'name', None) + if filename is None: + log.downloads.error("No filename to open the download!") + return + url = QUrl.fromLocalFile(filename) QDesktopServices.openUrl(url) def set_filename(self, filename): @@ -731,6 +738,20 @@ class DownloadManager(QAbstractListModel): self._update_timer = usertypes.Timer(self, 'download-update') self._update_timer.timeout.connect(self.update_gui) self._update_timer.setInterval(REFRESH_INTERVAL) + self._tmpdir_obj = None + + def cleanup(self): + """Clean up any temporary files from this manager.""" + if self._tmpdir_obj is not None: + self._tmpdir_obj.cleanup() + + @property + def tmpdir(self): + """Lazily create a temporary directory if one is needed.""" + if self._tmpdir_obj is None: + self._tmpdir_obj = tempfile.TemporaryDirectory( + prefix='qutebrowser-downloads-') + return self._tmpdir_obj def __repr__(self): return utils.get_repr(self, downloads=len(self.downloads)) @@ -738,6 +759,9 @@ class DownloadManager(QAbstractListModel): def _postprocess_question(self, q): """Postprocess a Question object that is asked.""" q.destroyed.connect(functools.partial(self.questions.remove, q)) + # We set the mode here so that other code that uses ask_for_filename + # doesn't need to handle the special download mode. + q.mode = usertypes.PromptMode.download self.questions.append(q) @pyqtSlot() @@ -816,27 +840,11 @@ class DownloadManager(QAbstractListModel): if suggested_fn is None: suggested_fn = 'qutebrowser-download' - # We won't need a question if a filename or fileobj is already given - if fileobj is None and filename is None: - filename, q = ask_for_filename( - suggested_fn, self._win_id, parent=self, - prompt_download_directory=prompt_download_directory - ) - - if fileobj is not None or filename is not None: - return self.fetch_request(request, - fileobj=fileobj, - filename=filename, - suggested_filename=suggested_fn, - **kwargs) - q.answered.connect( - lambda fn: self.fetch_request(request, - filename=fn, - suggested_filename=suggested_fn, - **kwargs)) - self._postprocess_question(q) - q.ask() - return None + return self.fetch_request(request, + fileobj=fileobj, + filename=filename, + suggested_filename=suggested_fn, + **kwargs) def fetch_request(self, request, *, page=None, **kwargs): """Download a QNetworkRequest to disk. @@ -874,7 +882,8 @@ class DownloadManager(QAbstractListModel): 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: + 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 @@ -915,7 +924,8 @@ class DownloadManager(QAbstractListModel): return download if filename is not None: - download.set_filename(filename) + self.set_filename_for_download(download, suggested_filename, + filename) return download # Neither filename nor fileobj were given, prepare a question @@ -926,12 +936,15 @@ class DownloadManager(QAbstractListModel): # User doesn't want to be asked, so just use the download_dir if filename is not None: - download.set_filename(filename) + self.set_filename_for_download(download, suggested_filename, + filename) return download # Ask the user for a filename self._postprocess_question(q) - q.answered.connect(download.set_filename) + q.answered.connect( + functools.partial(self.set_filename_for_download, download, + suggested_filename)) q.cancelled.connect(download.cancel) download.cancelled.connect(q.abort) download.error.connect(q.abort) @@ -939,6 +952,35 @@ 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. + + Args: + download: The download to set the filename for. + suggested_filename: The suggested filename. + filename: The filename as string or usertypes.OPEN_DOWNLOAD. + """ + if filename is not usertypes.OPEN_DOWNLOAD: + download.set_filename(filename) + return + # Find the next free filename without causing a race condition + index = 0 + while True: + basename = '{}-{}'.format(index, suggested_filename) + path = os.path.join(self.tmpdir.name, basename) + try: + fobj = open(path, 'xb') + except FileExistsError: + index += 1 + else: + break + download.finished.connect(download.open_file) + download.autoclose = True + download.set_fileobj(fobj) + def raise_no_download(self, count): """Raise an exception that the download doesn't exist. diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index 54a735609..77b24e116 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -452,6 +452,7 @@ class MainWindow(QWidget): self._save_geometry() log.destroy.debug("Closing window {}".format(self.win_id)) self.tabbed_browser.shutdown() + self._get_object('download-manager').cleanup() def closeEvent(self, e): """Override closeEvent to display a confirmation if needed.""" diff --git a/qutebrowser/mainwindow/statusbar/prompter.py b/qutebrowser/mainwindow/statusbar/prompter.py index dbf1084b0..2700a2220 100644 --- a/qutebrowser/mainwindow/statusbar/prompter.py +++ b/qutebrowser/mainwindow/statusbar/prompter.py @@ -80,6 +80,7 @@ class Prompter(QObject): usertypes.PromptMode.text: usertypes.KeyMode.prompt, usertypes.PromptMode.user_pwd: usertypes.KeyMode.prompt, usertypes.PromptMode.alert: usertypes.KeyMode.prompt, + usertypes.PromptMode.download: usertypes.KeyMode.prompt, } show_prompt = pyqtSignal() @@ -164,12 +165,9 @@ class Prompter(QObject): suffix = " (no)" prompt.txt.setText(self._question.text + suffix) prompt.lineedit.hide() - elif self._question.mode == usertypes.PromptMode.text: - prompt.txt.setText(self._question.text) - if self._question.default: - prompt.lineedit.setText(self._question.default) - prompt.lineedit.show() - elif self._question.mode == usertypes.PromptMode.user_pwd: + elif self._question.mode in {usertypes.PromptMode.text, + usertypes.PromptMode.user_pwd, + usertypes.PromptMode.download}: prompt.txt.setText(self._question.text) if self._question.default: prompt.lineedit.setText(self._question.default) @@ -248,6 +246,12 @@ class Prompter(QObject): modeman.maybe_leave(self._win_id, usertypes.KeyMode.prompt, 'prompt accept') self._question.done() + elif self._question.mode == usertypes.PromptMode.download: + # User just entered a path for a download. + self._question.answer = prompt.lineedit.text() + modeman.maybe_leave(self._win_id, usertypes.KeyMode.prompt, + 'prompt accept') + self._question.done() elif self._question.mode == usertypes.PromptMode.yesno: # User wants to accept the default of a yes/no question. self._question.answer = self._question.default @@ -287,6 +291,18 @@ class Prompter(QObject): 'prompt accept') self._question.done() + @cmdutils.register(instance='prompter', hide=True, scope='window', + modes=[usertypes.KeyMode.prompt]) + def prompt_open_download(self): + """Immediately open a download.""" + 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 + modeman.maybe_leave(self._win_id, usertypes.KeyMode.prompt, + 'download open') + self._question.done() + @pyqtSlot(usertypes.Question, bool) def ask_question(self, question, blocking): """Display a question in the statusbar. diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py index 872531d4f..b21676fde 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -33,6 +33,7 @@ from qutebrowser.utils import log, qtutils, utils _UNSET = object() +OPEN_DOWNLOAD = object() def enum(name, items, start=1, is_int=False): @@ -221,7 +222,8 @@ class NeighborList(collections.abc.Sequence): # The mode of a Question. -PromptMode = enum('PromptMode', ['yesno', 'text', 'user_pwd', 'alert']) +PromptMode = enum('PromptMode', ['yesno', 'text', 'user_pwd', 'alert', + 'download']) # Where to open a clicked link. From 81b2688c70748845eab89236b4026b40c80b112c Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 6 Jul 2016 18:41:08 +0200 Subject: [PATCH 02/18] downloads: use global TempDownloadManager This way, all temporary downloads will end up in the same directory and everything is cleaned up at program exit, not when the corresponding window is closed. --- qutebrowser/app.py | 6 +- qutebrowser/browser/webkit/downloads.py | 83 +++++++++++++++++-------- qutebrowser/mainwindow/mainwindow.py | 1 - 3 files changed, 63 insertions(+), 27 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index c0662eaa0..6cbe7674f 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -47,7 +47,7 @@ from qutebrowser.completion.models import instances as completionmodels from qutebrowser.commands import cmdutils, runners, cmdexc from qutebrowser.config import style, config, websettings, configexc from qutebrowser.browser import urlmarks, adblock -from qutebrowser.browser.webkit import cookies, cache, history +from qutebrowser.browser.webkit import cookies, cache, history, downloads from qutebrowser.browser.webkit.network import (qutescheme, proxy, networkmanager) from qutebrowser.mainwindow import mainwindow @@ -438,6 +438,8 @@ def _init_modules(args, crash_handler): os.environ.pop('QT_WAYLAND_DISABLE_WINDOWDECORATION', None) _maybe_hide_mouse_cursor() objreg.get('config').changed.connect(_maybe_hide_mouse_cursor) + temp_downloads = downloads.TempDownloadManager(qApp) + objreg.register('temporary-downloads', temp_downloads) def _init_late_modules(args): @@ -711,6 +713,8 @@ class Quitter: not restart): atexit.register(shutil.rmtree, self._args.basedir, ignore_errors=True) + # Delete temp download dir + objreg.get('temporary-downloads').cleanup() # If we don't kill our custom handler here we might get segfaults log.destroy.debug("Deactivating message handler...") qInstallMessageHandler(None) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 86670986b..45cd73baa 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -738,20 +738,6 @@ class DownloadManager(QAbstractListModel): self._update_timer = usertypes.Timer(self, 'download-update') self._update_timer.timeout.connect(self.update_gui) self._update_timer.setInterval(REFRESH_INTERVAL) - self._tmpdir_obj = None - - def cleanup(self): - """Clean up any temporary files from this manager.""" - if self._tmpdir_obj is not None: - self._tmpdir_obj.cleanup() - - @property - def tmpdir(self): - """Lazily create a temporary directory if one is needed.""" - if self._tmpdir_obj is None: - self._tmpdir_obj = tempfile.TemporaryDirectory( - prefix='qutebrowser-downloads-') - return self._tmpdir_obj def __repr__(self): return utils.get_repr(self, downloads=len(self.downloads)) @@ -966,17 +952,8 @@ class DownloadManager(QAbstractListModel): if filename is not usertypes.OPEN_DOWNLOAD: download.set_filename(filename) return - # Find the next free filename without causing a race condition - index = 0 - while True: - basename = '{}-{}'.format(index, suggested_filename) - path = os.path.join(self.tmpdir.name, basename) - try: - fobj = open(path, 'xb') - except FileExistsError: - index += 1 - else: - break + 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) @@ -1291,3 +1268,59 @@ class DownloadManager(QAbstractListModel): The number of unfinished downloads. """ return sum(1 for download in self.downloads if not download.done) + + +class TempDownloadManager(QObject): + + """Manager to handle temporary download files. + + The downloads are downloaded to a temporary location and then openened with + the system standard application. The temporary files are deleted when + qutebrowser is shutdown. + + Attributes: + files: A list of NamedTemporaryFiles of downloaded items. + """ + + def __init__(self, parent=None): + super().__init__(parent) + self.files = [] + self._tmpdir = None + + def cleanup(self): + """Clean up any temporary files.""" + if self._tmpdir is not None: + self._tmpdir.cleanup() + self._tmpdir = None + + def get_tmpdir(self): + """Return the temporary directory that is used for downloads. + + The directory is created lazily on first access. + + Return: + The tempfile.TemporaryDirectory that is used. + """ + if self._tmpdir is None: + self._tmpdir = tempfile.TemporaryDirectory( + prefix='qutebrowser-downloads-') + return self._tmpdir + + def get_tmpfile(self, suggested_name): + """Return a temporary file in the temporary downloads directory. + + The files are kept as long as qutebrowser is running and automatically + cleaned up at program exit. + + Args: + suggested_name: str of the "suggested"/original filename. Used as a + suffix, so any file extenions are preserved. + + Return: + A tempfile.NamedTemporaryFile that should be used to save the file. + """ + tmpdir = self.get_tmpdir() + fobj = tempfile.NamedTemporaryFile(dir=tmpdir.name, delete=False, + suffix=suggested_name) + self.files.append(fobj) + return fobj diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index 77b24e116..54a735609 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -452,7 +452,6 @@ class MainWindow(QWidget): self._save_geometry() log.destroy.debug("Closing window {}".format(self.win_id)) self.tabbed_browser.shutdown() - self._get_object('download-manager').cleanup() def closeEvent(self, e): """Override closeEvent to display a confirmation if needed.""" From b130c2a284a8594e905ed7053e22cf67c8a1f7cf Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 6 Jul 2016 18:48:21 +0200 Subject: [PATCH 03/18] keybindings: add default for prompt-open-download --- qutebrowser/config/configdata.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 5447f8ed6..a13808e97 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -1575,6 +1575,7 @@ KEY_DATA = collections.OrderedDict([ ('prompt-accept', RETURN_KEYS), ('prompt-yes', ['y']), ('prompt-no', ['n']), + ('prompt-open-download', ['']), ])), ('command,prompt', collections.OrderedDict([ From 7608805c9a4a6a76123f7e3f91eaaaf51beb4fa2 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 6 Jul 2016 23:16:38 +0200 Subject: [PATCH 04/18] tests: adjust tests for new download mode Now the prompt is shown with PromptMode.download instead of PromptMode.text, which we need to change in the tests. --- tests/end2end/features/downloads.feature | 4 ++-- tests/end2end/features/misc.feature | 2 +- tests/end2end/features/test_downloads_bdd.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index 7016f3284..f15466053 100644 --- a/tests/end2end/features/downloads.feature +++ b/tests/end2end/features/downloads.feature @@ -30,7 +30,7 @@ Feature: Downloading things from a website. And I open data/downloads/issue1243.html And I run :hint links download And I run :follow-hint a - And I wait for "Asking question text='Save file to:'>, *" in the log + And I wait for "Asking question text='Save file to:'>, *" in the log And I run :leave-mode Then no crash should happen @@ -40,7 +40,7 @@ Feature: Downloading things from a website. And I open data/downloads/issue1214.html And I run :hint links download And I run :follow-hint a - And I wait for "Asking question text='Save file to:'>, *" in the log + And I wait for "Asking question text='Save file to:'>, *" in the log And I run :leave-mode Then no crash should happen diff --git a/tests/end2end/features/misc.feature b/tests/end2end/features/misc.feature index 08befc45a..b11ff5975 100644 --- a/tests/end2end/features/misc.feature +++ b/tests/end2end/features/misc.feature @@ -312,7 +312,7 @@ Feature: Various utility commands. And I open data/misc/test.pdf And I wait for "[qute://pdfjs/*] PDF * (PDF.js: *)" in the log And I run :jseval document.getElementById("download").click() - And I wait for "Asking question text='Save file to:'>, *" in the log + And I wait for "Asking question text='Save file to:'>, *" in the log And I run :leave-mode Then no crash should happen diff --git a/tests/end2end/features/test_downloads_bdd.py b/tests/end2end/features/test_downloads_bdd.py index dc5a478eb..06271cb5c 100644 --- a/tests/end2end/features/test_downloads_bdd.py +++ b/tests/end2end/features/test_downloads_bdd.py @@ -64,7 +64,7 @@ def download_should_exist(filename, tmpdir): def download_prompt(tmpdir, quteproc, path): full_path = path.replace('{downloaddir}', str(tmpdir)).replace('/', os.sep) msg = ("Asking question " + "default={full_path!r} mode= " "text='Save file to:'>, *".format(full_path=full_path)) quteproc.wait_for(message=msg) quteproc.send_cmd(':leave-mode') From 8795f89c889fe60ffeeca6af30c997ecd96279a8 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 7 Jul 2016 00:08:13 +0200 Subject: [PATCH 05/18] tests: fix tests for downloads bdd test The test was failing because of two reasons: First, the old code had filename questions in DownloadManager.get and DownloadManager.fetch which were almost identical, thus the part in DownloadManager.get was removed in an earlier commit. All filename asking is now done by DownloadManager.fetch. The good part is code deduplication, the bad part is slightly modified behavior: The new code doesn't wait for a filename to start the download, instead it tries to fill the buffer immediately. This made the test fail because qute:// has no registered handler, so in order for the test to pass now, the "no crash" part is not enough, we also need to expect the "No handler" error. Secondly, and a rather rare (race) condition was the handling of errors in the DownloadItem. If an error occured after the registration of self.on_reply_error as error handler and before the check reply.error() != QNetworkReply.NoError at the end of the function, the error signal would be emitted twice: Once by _die() (called by on_reply_error), and once by the init_reply function directly (in the last if block). This lead to duplicated error messages. This is also explained in a comment in the file (with small "stack traces"). --- qutebrowser/browser/webkit/downloads.py | 19 ++++++++++++++++++- tests/end2end/features/downloads.feature | 3 +-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 45cd73baa..c684e1608 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -281,6 +281,7 @@ class DownloadItem(QObject): _read_timer: A Timer which reads the QNetworkReply into self._buffer periodically. _win_id: The window ID the DownloadItem runs in. + _dead: Whether the Download has _die()'d. Signals: data_changed: The downloads metadata changed. @@ -329,6 +330,7 @@ class DownloadItem(QObject): self.init_reply(reply) self._win_id = win_id self.raw_headers = {} + self._dead = False def __repr__(self): return utils.get_repr(self, basename=self.basename) @@ -396,6 +398,21 @@ class DownloadItem(QObject): def _die(self, msg): """Abort the download and emit an error.""" assert not self.successful + # Prevent actions if calling _die() twice. This might happen if the + # error handler correctly connects, and the error occurs in init_reply + # between reply.error.connect and the reply.error() check. In this + # case, the connected error handlers will be called twice, once via the + # direct error.emit() and once here in _die(). The stacks look like + # this then: + # -> on_reply_error -> _die -> + # self.error.emit() + # and + # [init_reply -> ->] -> + # self.error.emit() + # which may lead to duplicate error messages (and failing tests) + if self._dead: + return + self._dead = True self._read_timer.stop() self.reply.downloadProgress.disconnect() self.reply.finished.disconnect() @@ -442,7 +459,7 @@ class DownloadItem(QObject): # Here no signals are connected to the DownloadItem yet, so we use a # singleShot QTimer to emit them after they are connected. if reply.error() != QNetworkReply.NoError: - QTimer.singleShot(0, lambda: self.error.emit(reply.errorString())) + QTimer.singleShot(0, lambda: self._die(reply.errorString())) def get_status_color(self, position): """Choose an appropriate color for presenting the download's status. diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index f15466053..7548c0dda 100644 --- a/tests/end2end/features/downloads.feature +++ b/tests/end2end/features/downloads.feature @@ -31,8 +31,7 @@ Feature: Downloading things from a website. And I run :hint links download And I run :follow-hint a And I wait for "Asking question text='Save file to:'>, *" in the log - And I run :leave-mode - Then no crash should happen + Then the error "Download error: No handler found for qute://!" should be shown Scenario: Downloading a data: link (issue 1214) When I set completion -> download-path-suggestion to filename From 203dad000435fba6ca922614eff65b9a1c552ade Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 7 Jul 2016 12:17:42 +0200 Subject: [PATCH 06/18] fix lint --- qutebrowser/browser/webkit/downloads.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index c684e1608..c769c489f 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -795,8 +795,7 @@ class DownloadManager(QAbstractListModel): req = QNetworkRequest(url) return self.get_request(req, **kwargs) - def get_request(self, request, *, fileobj=None, filename=None, - prompt_download_directory=None, **kwargs): + def get_request(self, request, *, fileobj=None, filename=None, **kwargs): """Start a download with a QNetworkRequest. Args: From 220203dd9ca518c0a70bf9c1a4761b15ae5f5634 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 7 Jul 2016 12:18:52 +0200 Subject: [PATCH 07/18] spellchecker: mark "occurs" as correct See http://www.thefreedictionary.com/occur --- scripts/dev/misc_checks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/dev/misc_checks.py b/scripts/dev/misc_checks.py index 47b2158ae..7f0bf493b 100644 --- a/scripts/dev/misc_checks.py +++ b/scripts/dev/misc_checks.py @@ -81,14 +81,14 @@ def check_spelling(): """Check commonly misspelled words.""" # Words which I often misspell words = {'[Bb]ehaviour', '[Qq]uitted', 'Ll]ikelyhood', '[Ss]ucessfully', - '[Oo]ccur[^r .]', '[Ss]eperator', '[Ee]xplicitely', '[Rr]esetted', + '[Oo]ccur[^rs .]', '[Ss]eperator', '[Ee]xplicitely', '[Aa]uxillary', '[Aa]ccidentaly', '[Aa]mbigious', '[Ll]oosly', '[Ii]nitialis', '[Cc]onvienence', '[Ss]imiliar', '[Uu]ncommited', '[Rr]eproducable', '[Aa]n [Uu]ser', '[Cc]onvienience', '[Ww]ether', '[Pp]rogramatically', '[Ss]plitted', '[Ee]xitted', '[Mm]ininum', '[Rr]esett?ed', '[Rr]ecieved', '[Rr]egularily', '[Uu]nderlaying', '[Ii]nexistant', '[Ee]lipsis', 'commiting', - 'existant'} + 'existant', '[Rr]esetted'} # Words which look better when splitted, but might need some fine tuning. words |= {'[Ww]ebelements', '[Mm]ouseevent', '[Kk]eysequence', From 16e1f8eac94559fd25d2eca8a3ade3a72c16133d Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 7 Jul 2016 12:50:01 +0200 Subject: [PATCH 08/18] downloads: fix docstring for .get() An earlier commit removed the prompt_download_directory parameter, it is now passed via **kwargs and should thus be removed from the docstring. --- qutebrowser/browser/webkit/downloads.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index c769c489f..e5359dbde 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -802,9 +802,6 @@ class DownloadManager(QAbstractListModel): request: The QNetworkRequest to download. fileobj: The file object to write the answer to. filename: A path to write the data to. - prompt_download_directory: Whether to prompt for the download dir - or automatically download. If None, the - config is used. **kwargs: Passed to fetch_request. Return: From d42d980dda1af4bcb3e26c92892cb4cca10b1c56 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 7 Jul 2016 13:18:38 +0200 Subject: [PATCH 09/18] 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 | 5 +- qutebrowser/browser/commands.py | 12 ++- qutebrowser/browser/webkit/downloads.py | 77 +++++++++----------- qutebrowser/browser/webkit/mhtml.py | 6 +- qutebrowser/mainwindow/statusbar/prompter.py | 5 +- qutebrowser/utils/usertypes.py | 47 +++++++++++- 6 files changed, 99 insertions(+), 53 deletions(-) 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. From 05c09a147641646d64f01f6c8ea6fcac6c5ebc43 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 7 Jul 2016 13:30:38 +0200 Subject: [PATCH 10/18] tests: fix test_adblock for new download manager --- tests/unit/browser/test_adblock.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index be801de73..1a9c820a7 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -92,12 +92,12 @@ class FakeDownloadManager: """Mock browser.downloads.DownloadManager.""" - def get(self, url, fileobj, **kwargs): + def get(self, url, target, **kwargs): """Return a FakeDownloadItem instance with a fileobj. The content is copied from the file the given url links to. """ - download_item = FakeDownloadItem(fileobj, name=url.path()) + download_item = FakeDownloadItem(target.fileobj, name=url.path()) with open(url.path(), 'rb') as fake_url_file: shutil.copyfileobj(fake_url_file, download_item.fileobj) return download_item From 17175ec3d407780b3972cee62853d09858061f00 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 7 Jul 2016 13:48:24 +0200 Subject: [PATCH 11/18] mhtml: remove duplicated usertypes import --- qutebrowser/browser/webkit/mhtml.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qutebrowser/browser/webkit/mhtml.py b/qutebrowser/browser/webkit/mhtml.py index 399516f9a..a0798b68a 100644 --- a/qutebrowser/browser/webkit/mhtml.py +++ b/qutebrowser/browser/webkit/mhtml.py @@ -35,8 +35,7 @@ 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, usertypes) +from qutebrowser.utils import log, objreg, message, usertypes, utils, urlutils try: import cssutils From ed5fb4de4a21f8e9868348f27a89f2875d497e89 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 12 Jul 2016 16:26:35 +0200 Subject: [PATCH 12/18] downloads: make get_tmpdir private --- qutebrowser/browser/webkit/downloads.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index f3c6b1cfe..61623372a 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -1295,7 +1295,7 @@ class TempDownloadManager(QObject): self._tmpdir.cleanup() self._tmpdir = None - def get_tmpdir(self): + def _get_tmpdir(self): """Return the temporary directory that is used for downloads. The directory is created lazily on first access. @@ -1321,7 +1321,7 @@ class TempDownloadManager(QObject): Return: A tempfile.NamedTemporaryFile that should be used to save the file. """ - tmpdir = self.get_tmpdir() + tmpdir = self._get_tmpdir() fobj = tempfile.NamedTemporaryFile(dir=tmpdir.name, delete=False, suffix=suggested_name) self.files.append(fobj) From a088f238e5ec20b2c00d0060bc7f8a78e598468d Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 12 Jul 2016 16:46:03 +0200 Subject: [PATCH 13/18] usertypes: remove Frankenstein's enum This approach is not as weird in Python and still works. DownloadTarget.OpenDownload has been renamed to OpenFileDownloadTarget, since OpenDownloadDownloadTarget didn't look as nice. --- qutebrowser/browser/adblock.py | 2 +- qutebrowser/browser/commands.py | 4 +- qutebrowser/browser/webkit/downloads.py | 12 +++--- qutebrowser/browser/webkit/mhtml.py | 2 +- qutebrowser/mainwindow/statusbar/prompter.py | 4 +- qutebrowser/utils/usertypes.py | 45 +++++++++----------- 6 files changed, 32 insertions(+), 37 deletions(-) diff --git a/qutebrowser/browser/adblock.py b/qutebrowser/browser/adblock.py index 4d0c02b3d..afdc9922e 100644 --- a/qutebrowser/browser/adblock.py +++ b/qutebrowser/browser/adblock.py @@ -210,7 +210,7 @@ class HostBlocker: else: fobj = io.BytesIO() fobj.name = 'adblock: ' + url.host() - target = usertypes.DownloadTarget.FileObj(fobj) + target = usertypes.FileObjDownloadTarget(fobj) download = download_manager.get(url, target=target, auto_remove=True) self._in_progress.append(download) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 6ad80d5db..871228595 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1224,7 +1224,7 @@ class CommandDispatcher: if dest is None: target = None else: - target = usertypes.DownloadTarget.File(dest) + target = usertypes.FileDownloadTarget(dest) download_manager.get(url, target=target) elif mhtml_: self._download_mhtml(dest) @@ -1235,7 +1235,7 @@ class CommandDispatcher: if dest is None: target = None else: - target = usertypes.DownloadTarget.File(dest) + target = usertypes.FileDownloadTarget(dest) download_manager.get(self._current_url(), page=page, target=target) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 61623372a..72364ec3e 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -874,9 +874,9 @@ class DownloadManager(QAbstractListModel): The created DownloadItem. """ if not suggested_filename: - if isinstance(target, usertypes.DownloadTarget.File): + if isinstance(target, usertypes.FileDownloadTarget): suggested_filename = os.path.basename(target.filename) - elif (isinstance(target, usertypes.DownloadTarget.FileObj) and + elif (isinstance(target, usertypes.FileObjDownloadTarget) and getattr(target.fileobj, 'name', None)): suggested_filename = target.fileobj.name else: @@ -922,7 +922,7 @@ class DownloadManager(QAbstractListModel): # User doesn't want to be asked, so just use the download_dir if filename is not None: - target = usertypes.DownloadTarget.File(filename) + target = usertypes.FileDownloadTarget(filename) self._set_download_target(download, suggested_filename, target) return download @@ -946,12 +946,12 @@ class DownloadManager(QAbstractListModel): suggested_filename: The suggested filename. target: The usertypes.DownloadTarget for this download. """ - if isinstance(target, usertypes.DownloadTarget.FileObj): + if isinstance(target, usertypes.FileObjDownloadTarget): download.set_fileobj(target.fileobj) download.autoclose = False - elif isinstance(target, usertypes.DownloadTarget.File): + elif isinstance(target, usertypes.FileDownloadTarget): download.set_filename(target.filename) - elif isinstance(target, usertypes.DownloadTarget.OpenDownload): + elif isinstance(target, usertypes.OpenFileDownloadTarget): tmp_manager = objreg.get('temporary-downloads') fobj = tmp_manager.get_tmpfile(suggested_filename) download.finished.connect(download.open_file) diff --git a/qutebrowser/browser/webkit/mhtml.py b/qutebrowser/browser/webkit/mhtml.py index a0798b68a..39212d712 100644 --- a/qutebrowser/browser/webkit/mhtml.py +++ b/qutebrowser/browser/webkit/mhtml.py @@ -343,7 +343,7 @@ class _Downloader: download_manager = objreg.get('download-manager', scope='window', window=self._win_id) - target = usertypes.DownloadTarget.FileObj(_NoCloseBytesIO()) + target = usertypes.FileObjDownloadTarget(_NoCloseBytesIO()) item = download_manager.get(url, target=target, auto_remove=True) self.pending_downloads.add((url, item)) diff --git a/qutebrowser/mainwindow/statusbar/prompter.py b/qutebrowser/mainwindow/statusbar/prompter.py index 3b70d6ac2..60a133cb1 100644 --- a/qutebrowser/mainwindow/statusbar/prompter.py +++ b/qutebrowser/mainwindow/statusbar/prompter.py @@ -248,7 +248,7 @@ class Prompter(QObject): self._question.done() elif self._question.mode == usertypes.PromptMode.download: # User just entered a path for a download. - target = usertypes.DownloadTarget.File(prompt.lineedit.text()) + target = usertypes.FileDownloadTarget(prompt.lineedit.text()) self._question.answer = target modeman.maybe_leave(self._win_id, usertypes.KeyMode.prompt, 'prompt accept') @@ -299,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.DownloadTarget.OpenDownload() + self._question.answer = usertypes.OpenFileDownloadTarget() 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 38831bd36..81ad0086a 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -259,46 +259,41 @@ 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. - """ + """Abstract base class for different download targets.""" 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: +class FileDownloadTarget(DownloadTarget): - """Save the download to the given file. + """Save the download to the given file. - Attributes: - filename: Filename where the download should be saved. - """ + Attributes: + filename: Filename where the download should be saved. + """ - def __init__(self, filename): - self.filename = filename + def __init__(self, filename): + self.filename = filename - class FileObj: - """Save the download to the given file-like object. +class FileObjDownloadTarget(DownloadTarget): - Attributes: - fileobj: File-like object where the download should be written to. - """ + """Save the download to the given file-like object. - def __init__(self, fileobj): - self.fileobj = fileobj + Attributes: + fileobj: File-like object where the download should be written to. + """ - class OpenDownload: + def __init__(self, fileobj): + self.fileobj = fileobj - """Save the download in a temp dir and directly open it.""" +class OpenFileDownloadTarget(DownloadTarget): + + """Save the download in a temp dir and directly open it.""" + + def __init__(self): pass From 940b124253a9360b601525a9140ce328dc909c00 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 12 Jul 2016 22:31:43 +0200 Subject: [PATCH 14/18] switch set literal to list literal See PR #1642. :kissing::notes: --- qutebrowser/mainwindow/statusbar/prompter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/mainwindow/statusbar/prompter.py b/qutebrowser/mainwindow/statusbar/prompter.py index 60a133cb1..c299c43f0 100644 --- a/qutebrowser/mainwindow/statusbar/prompter.py +++ b/qutebrowser/mainwindow/statusbar/prompter.py @@ -165,9 +165,9 @@ class Prompter(QObject): suffix = " (no)" prompt.txt.setText(self._question.text + suffix) prompt.lineedit.hide() - elif self._question.mode in {usertypes.PromptMode.text, + elif self._question.mode in [usertypes.PromptMode.text, usertypes.PromptMode.user_pwd, - usertypes.PromptMode.download}: + usertypes.PromptMode.download]: prompt.txt.setText(self._question.text) if self._question.default: prompt.lineedit.setText(self._question.default) From 75972217766476f59e8e2a77d6b94e0bf94f3add Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 12 Jul 2016 22:39:21 +0200 Subject: [PATCH 15/18] fix lint --- qutebrowser/utils/usertypes.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py index 81ad0086a..e365d5645 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -274,6 +274,7 @@ class FileDownloadTarget(DownloadTarget): """ def __init__(self, filename): + # pylint: disable=super-init-not-called self.filename = filename @@ -286,6 +287,7 @@ class FileObjDownloadTarget(DownloadTarget): """ def __init__(self, fileobj): + # pylint: disable=super-init-not-called self.fileobj = fileobj @@ -294,6 +296,7 @@ class OpenFileDownloadTarget(DownloadTarget): """Save the download in a temp dir and directly open it.""" def __init__(self): + # pylint: disable=super-init-not-called pass From 029ffe3fc7ad905b1d75b16e61547a33e22a727f Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 13 Jul 2016 14:17:41 +0200 Subject: [PATCH 16/18] tests: add tests for usertypes.DownloadTarget --- .../utils/usertypes/test_downloadtarget.py | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 tests/unit/utils/usertypes/test_downloadtarget.py diff --git a/tests/unit/utils/usertypes/test_downloadtarget.py b/tests/unit/utils/usertypes/test_downloadtarget.py new file mode 100644 index 000000000..e89401144 --- /dev/null +++ b/tests/unit/utils/usertypes/test_downloadtarget.py @@ -0,0 +1,54 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2016 Daniel Schadt +# +# 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 . + +"""Tests for the DownloadTarget class.""" + +from qutebrowser.utils import usertypes + +import pytest + + +def test_base(): + with pytest.raises(NotImplementedError): + usertypes.DownloadTarget() + + +def test_filename(): + target = usertypes.FileDownloadTarget("/foo/bar") + assert target.filename == "/foo/bar" + + +def test_fileobj(): + fobj = object() + target = usertypes.FileObjDownloadTarget(fobj) + assert target.fileobj is fobj + + +def test_openfile(): + # Just make sure no error is raised, that should be enough. + usertypes.OpenFileDownloadTarget() + + +@pytest.mark.parametrize('obj', [ + usertypes.FileDownloadTarget('foobar'), + usertypes.FileObjDownloadTarget(None), + usertypes.OpenFileDownloadTarget(), +]) +def test_class_hierarchy(obj): + assert isinstance(obj, usertypes.DownloadTarget) From b995b9d5da0343ad241fb086d68507e63c47cfd3 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 13 Jul 2016 22:25:23 +0200 Subject: [PATCH 17/18] downloads: fix docstrings for new return value --- qutebrowser/browser/webkit/downloads.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 72364ec3e..bb54a8774 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -784,10 +784,7 @@ class DownloadManager(QAbstractListModel): **kwargs: passed to get_request(). Return: - If the download could start immediately, (target given), - the created DownloadItem. - - If not, None. + The created DownloadItem. """ if not url.isValid(): urlutils.invalid_url_error(self._win_id, url, "start download") @@ -804,10 +801,7 @@ class DownloadManager(QAbstractListModel): **kwargs: Passed to fetch_request. Return: - If the download could start immediately, (target given), - the created DownloadItem. - - If not, None. + The created DownloadItem. """ # WORKAROUND for Qt corrupting data loaded from cache: # https://bugreports.qt.io/browse/QTBUG-42757 From d5cf8ef8948007b61a4afdb074afde823cf283b8 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 26 Jul 2016 10:54:45 +0200 Subject: [PATCH 18/18] Update docs --- CHANGELOG.asciidoc | 2 ++ doc/help/commands.asciidoc | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index f0bd07795..55c928a09 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -32,6 +32,8 @@ Added Note that two former default bundings conflict with that binding, unbinding them via `:unbind .i` and `:unbind .o` is recommended. - New `qute:bookmarks` page which displays all bookmarks and quickmarks. +- New `:prompt-open-download` (bound to `Ctrl-X`) which can be used to open a + download directly when getting the filename prompt. Changed ~~~~~~~ diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index eea3f0d7f..af9089135 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -936,6 +936,7 @@ How many steps to zoom out. |<>|Paste the primary selection at cursor position. |<>|Accept the current prompt. |<>|Answer no to a yes/no prompt. +|<>|Immediately open a download. |<>|Answer yes to a yes/no prompt. |<>|Repeat the last executed command. |<>|Move back a character. @@ -1160,6 +1161,10 @@ Accept the current prompt. === prompt-no Answer no to a yes/no prompt. +[[prompt-open-download]] +=== prompt-open-download +Immediately open a download. + [[prompt-yes]] === prompt-yes Answer yes to a yes/no prompt.