From 716ce701f5e020a785e38b2dc267f03a751bc3c5 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 2 Aug 2016 13:29:03 +0200 Subject: [PATCH 01/17] utils: move elide_filename to own function Also increase the elide limit in TempDownloadManager to 50, since that's probably still enough for all systems. --- qutebrowser/browser/webkit/downloads.py | 3 +-- qutebrowser/utils/utils.py | 32 +++++++++++++++++++++++++ tests/unit/utils/test_utils.py | 18 ++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index e5f9e8709..d7fe3b4a6 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -1334,8 +1334,7 @@ class TempDownloadManager(QObject): encoding = sys.getfilesystemencoding() suggested_name = utils.force_encoding(suggested_name, encoding) # Make sure that the filename is not too long - if len(suggested_name) > 50: - suggested_name = suggested_name[:25] + '...' + suggested_name[-25:] + suggested_name = utils.elide_filename(suggested_name, 50) fobj = tempfile.NamedTemporaryFile(dir=tmpdir.name, delete=False, suffix=suggested_name) self.files.append(fobj) diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index f24365ce2..fd8419d12 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -58,6 +58,38 @@ def elide(text, length): return text[:length - 1] + '\u2026' +def elide_filename(filename, length): + """Elide a filename to the given length. + + The difference to the elide() is that the text is removed from + the middle instead of from the end. This preserves file name extensions. + Additionally, standard ASCII dots are used ("...") instead of the unicode + "…" (U+2026) so it works regardless of the filesystem encoding. + + This function does not handle path separators. + + Args: + filename: The filename to elide. + length: The maximum length of the filename, must be at least 3. + + Return: + The elided filename. + """ + elidestr = '...' + if length < len(elidestr): + raise ValueError('length must be greater or equal to 3') + if len(filename) <= length: + return filename + # Account for '...' + length -= len(elidestr) + left = length // 2 + right = length - left + if right == 0: + return filename[:left] + elidestr + else: + return filename[:left] + elidestr + filename[-right:] + + def compact_text(text, elidelength=None): """Remove leading whitespace and newlines from a text and maybe elide it. diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index 49d20ebaf..9947c56bd 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -94,6 +94,24 @@ class TestEliding: assert utils.elide(text, length) == expected +class TestElidingFilenames: + + """Test elide_filename.""" + + def test_too_small(self): + """Test eliding to less than 3 characters which should fail.""" + with pytest.raises(ValueError): + utils.elide_filename('foo', 1) + + @pytest.mark.parametrize('filename, length, expected', [ + ('foobar', 3, '...'), + ('foobar.txt', 50, 'foobar.txt'), + ('foobarbazqux.py', 10, 'foo...x.py'), + ]) + def test_elided(self, filename, length, expected): + assert utils.elide_filename(filename, length) == expected + + @pytest.fixture(params=[True, False]) def freezer(request, monkeypatch): if request.param and not getattr(sys, 'frozen', False): From fa6c552d7ba5c339567f706e4fc5e6d3f65e7b10 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 2 Aug 2016 19:40:57 +0200 Subject: [PATCH 02/17] add an application param for prompt-open-download --- doc/help/commands.asciidoc | 7 ++++ qutebrowser/browser/webkit/downloads.py | 34 ++++++++++++++++---- qutebrowser/mainwindow/statusbar/prompter.py | 15 ++++++--- qutebrowser/utils/usertypes.py | 11 +++++-- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index 9835f1a3f..e3caf954e 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -1168,8 +1168,15 @@ Answer no to a yes/no prompt. [[prompt-open-download]] === prompt-open-download +Syntax: +:prompt-open-download ['cmdline']+ + Immediately open a download. +==== positional arguments +* +'cmdline'+: The command line string to execute. The default will use the system's application to open the file, depending on + the filetype. + + [[prompt-yes]] === prompt-yes Answer yes to a yes/no prompt. diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index d7fe3b4a6..d03ff3dd1 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -22,6 +22,7 @@ import io import os import sys +import shlex import os.path import shutil import functools @@ -40,6 +41,7 @@ from qutebrowser.config import config from qutebrowser.commands import cmdexc, cmdutils from qutebrowser.utils import (message, usertypes, log, utils, urlutils, objreg, standarddir, qtutils) +from qutebrowser.misc import guiprocess from qutebrowser.browser.webkit import http from qutebrowser.browser.webkit.network import networkmanager @@ -955,19 +957,39 @@ class DownloadManager(QAbstractListModel): download.cancel() return download.finished.connect( - functools.partial(self._open_download, download)) + functools.partial(self._open_download, download, + target.cmdline)) download.autoclose = True download.set_fileobj(fobj) else: log.downloads.error("Unknown download target: {}".format(target)) - def _open_download(self, download): - """Open the given download but only if it was successful.""" - if download.successful: - download.open_file() - else: + def _open_download(self, download, cmdline): + """Open the given download but only if it was successful. + + Args: + download: The DownloadItem to use. + cmdline: The command to use, or None for the system's default + program. + """ + if not download.successful: log.downloads.debug("{} finished but not successful, not opening!" .format(download)) + if cmdline is None: + download.open_file() + return + + cmd, *args = shlex.split(cmdline) + filename = download._filename + if filename is None: + filename = getattr(download.fileobj, 'name', None) + if filename is None: + raise ValueError("Download has no filename to open") + args = [filename if arg == '{}' else arg for arg in args] + + proc = guiprocess.GUIProcess(self._win_id, what='process') + proc.start_detached(cmd, args) + def raise_no_download(self, count): """Raise an exception that the download doesn't exist. diff --git a/qutebrowser/mainwindow/statusbar/prompter.py b/qutebrowser/mainwindow/statusbar/prompter.py index 4fca341fc..0282f38cf 100644 --- a/qutebrowser/mainwindow/statusbar/prompter.py +++ b/qutebrowser/mainwindow/statusbar/prompter.py @@ -312,13 +312,20 @@ class Prompter(QObject): self._question.done() @cmdutils.register(instance='prompter', hide=True, scope='window', - modes=[usertypes.KeyMode.prompt]) - def prompt_open_download(self): - """Immediately open a download.""" + modes=[usertypes.KeyMode.prompt], maxsplit=0) + def prompt_open_download(self, cmdline: str=None): + """Immediately open a download. + + Args: + cmdline: The command line string to execute. The default will use + the system's application to open the file, depending on + the filetype. A `{}` is expanded to the temporary file + name. + """ if self._question.mode != usertypes.PromptMode.download: # We just ignore this if we don't have a download question. return - self._question.answer = usertypes.OpenFileDownloadTarget() + self._question.answer = usertypes.OpenFileDownloadTarget(cmdline) 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 bb8ad3734..1ed87af8e 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -297,11 +297,16 @@ class FileObjDownloadTarget(DownloadTarget): class OpenFileDownloadTarget(DownloadTarget): - """Save the download in a temp dir and directly open it.""" + """Save the download in a temp dir and directly open it. - def __init__(self): + Attributes: + cmdline: The command to use as string. A {} is expanded to th + filename. None means use the system's default. + """ + + def __init__(self, cmdline=None): # pylint: disable=super-init-not-called - pass + self.cmdline = cmdline class Question(QObject): From a85227fa254334ec879ef9aaeac23b6994d43dd7 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 2 Aug 2016 19:57:45 +0200 Subject: [PATCH 03/17] download-open: add cmdline parameter --- qutebrowser/browser/webkit/downloads.py | 47 ++++++++++++++----------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index d03ff3dd1..b5d940af7 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -530,8 +530,13 @@ class DownloadItem(QObject): self.cancel() @pyqtSlot() - def open_file(self): - """Open the downloaded file.""" + def open_file(self, cmdline=None): + """Open the downloaded file. + + Args: + cmdline: The command to use, or None for the system's default + program. + """ assert self.successful filename = self._filename if filename is None: @@ -539,8 +544,20 @@ class DownloadItem(QObject): if filename is None: log.downloads.error("No filename to open the download!") return - url = QUrl.fromLocalFile(filename) - QDesktopServices.openUrl(url) + + if cmdline is None: + log.downloads.debug("Opening {} with the system application" + .format(filename)) + url = QUrl.fromLocalFile(filename) + QDesktopServices.openUrl(url) + return + + cmd, *args = shlex.split(cmdline) + args = [filename if arg == '{}' else arg for arg in args] + log.downloads.debug("Opening {} with {}" + .format(filename, [cmd] + args)) + proc = guiprocess.GUIProcess(self._win_id, what='download') + proc.start_detached(cmd, args) def set_filename(self, filename): """Set the filename to save the download to. @@ -975,20 +992,8 @@ class DownloadManager(QAbstractListModel): if not download.successful: log.downloads.debug("{} finished but not successful, not opening!" .format(download)) - if cmdline is None: - download.open_file() return - - cmd, *args = shlex.split(cmdline) - filename = download._filename - if filename is None: - filename = getattr(download.fileobj, 'name', None) - if filename is None: - raise ValueError("Download has no filename to open") - args = [filename if arg == '{}' else arg for arg in args] - - proc = guiprocess.GUIProcess(self._win_id, what='process') - proc.start_detached(cmd, args) + download.open_file(cmdline) def raise_no_download(self, count): @@ -1048,12 +1053,14 @@ class DownloadManager(QAbstractListModel): self.remove_item(download) log.downloads.debug("deleted download {}".format(download)) - @cmdutils.register(instance='download-manager', scope='window') + @cmdutils.register(instance='download-manager', scope='window', maxsplit=0) @cmdutils.argument('count', count=True) - def download_open(self, count=0): + def download_open(self, cmdline: str=None, count=0): """Open the last/[count]th download. Args: + cmdline: The command which should be used to open the file. A {} as + argument will be replaced by the download's filename. count: The index of the download to open. """ try: @@ -1064,7 +1071,7 @@ class DownloadManager(QAbstractListModel): if not count: count = len(self.downloads) raise cmdexc.CommandError("Download {} is not done!".format(count)) - download.open_file() + download.open_file(cmdline) @cmdutils.register(instance='download-manager', scope='window') @cmdutils.argument('count', count=True) From e8bfc25bbcd05579cb8a480efe990eef75d01f28 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 3 Aug 2016 10:25:14 +0200 Subject: [PATCH 04/17] downloads: replace placeholder in cmdline This allows commands like --file={} to work properly. --- qutebrowser/browser/webkit/downloads.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index b5d940af7..662a291c9 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -553,7 +553,7 @@ class DownloadItem(QObject): return cmd, *args = shlex.split(cmdline) - args = [filename if arg == '{}' else arg for arg in args] + args = [arg.replace('{}', filename) for arg in args] log.downloads.debug("Opening {} with {}" .format(filename, [cmd] + args)) proc = guiprocess.GUIProcess(self._win_id, what='download') From afa7494c5fbf99388d279b5fa27df64ebe4f1e55 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 3 Aug 2016 17:55:52 +0200 Subject: [PATCH 05/17] add tests for download-open/prompt-open-download This has tests for * standard :download-open * standard :prompt-open-download * :prompt-open-download + cancel the download (issue #1728) --- tests/end2end/features/downloads.feature | 28 ++++++++++++++++---- tests/end2end/features/test_downloads_bdd.py | 12 +++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index 7548c0dda..882f51f3d 100644 --- a/tests/end2end/features/downloads.feature +++ b/tests/end2end/features/downloads.feature @@ -158,11 +158,11 @@ Feature: Downloading things from a website. ## :download-open - # Scenario: Opening a download - # When I open data/downloads/download.bin - # And I wait until the download is finished - # And I run :download-open - # Then ... + Scenario: Opening a download + When I open data/downloads/download.bin + And I wait until the download is finished + And I open the download + Then "Opening *download.bin* with [*python*]" should be logged Scenario: Opening a download which does not exist When I run :download-open with count 42 @@ -178,6 +178,24 @@ Feature: Downloading things from a website. And I run :download-open with count 1 Then the error "Download 1 is not done!" should be shown + ## opening a file directly (prompt-open-download) + + Scenario: Opening a download directly + When I set storage -> prompt-download-directory to true + And I open data/downloads/download.bin + And I directly open the download + And I wait until the download is finished + Then "Opening *download.bin* with [*python*]" should be logged + + ## https://github.com/The-Compiler/qutebrowser/issues/1728 + + Scenario: Cancelling a download that should be opened + When I set storage -> prompt-download-directory to true + And I run :download http://localhost:(port)/drip?numbytes=128&duration=5 + And I directly open the download + And I run :download-cancel + Then "* finished but not successful, not opening!" should be logged + ## completion -> download-path-suggestion Scenario: completion -> download-path-suggestion = path diff --git a/tests/end2end/features/test_downloads_bdd.py b/tests/end2end/features/test_downloads_bdd.py index 06271cb5c..fa7c41fc6 100644 --- a/tests/end2end/features/test_downloads_bdd.py +++ b/tests/end2end/features/test_downloads_bdd.py @@ -18,6 +18,7 @@ # along with qutebrowser. If not, see . import os +import sys import pytest_bdd as bdd bdd.scenarios('downloads.feature') @@ -68,3 +69,14 @@ def download_prompt(tmpdir, quteproc, path): "text='Save file to:'>, *".format(full_path=full_path)) quteproc.wait_for(message=msg) quteproc.send_cmd(':leave-mode') + + +@bdd.when("I open the download") +def download_open(quteproc): + cmd = '"{}" -c pass'.format(sys.executable) + quteproc.send_cmd(':download-open {}'.format(cmd)) + +@bdd.when("I directly open the download") +def download_open(quteproc): + cmd = '"{}" -c pass'.format(sys.executable) + quteproc.send_cmd(':prompt-open-download {}'.format(cmd)) From 71102cceb0be667c7f7a51752d1b8265dc0e3739 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 3 Aug 2016 18:06:28 +0200 Subject: [PATCH 06/17] docstring & documentation fixes --- doc/help/commands.asciidoc | 21 +++++++++++++++++--- qutebrowser/browser/webkit/downloads.py | 7 +++++-- qutebrowser/mainwindow/statusbar/prompter.py | 9 +++++---- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index e3caf954e..54cf56042 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -215,11 +215,22 @@ The index of the download to delete. [[download-open]] === download-open +Syntax: +:download-open ['cmdline']+ + Open the last/[count]th download. +If no specific command is given, this will use the system's default application to open the file. + +==== positional arguments +* +'cmdline'+: The command which should be used to open the file. A `{}` is expanded to the temporary file name. + + ==== count The index of the download to open. +==== note +* This command does not split arguments after the last argument and handles quotes literally. + [[download-remove]] === download-remove Syntax: +:download-remove [*--all*]+ @@ -1172,10 +1183,14 @@ Syntax: +:prompt-open-download ['cmdline']+ Immediately open a download. -==== positional arguments -* +'cmdline'+: The command line string to execute. The default will use the system's application to open the file, depending on - the filetype. +If no specific command is given, this will use the system's default application to open the file. +==== positional arguments +* +'cmdline'+: The command which should be used to open the file. A `{}` is expanded to the temporary file name. + + +==== note +* This command does not split arguments after the last argument and handles quotes literally. [[prompt-yes]] === prompt-yes diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 662a291c9..9a1f40d55 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -1058,9 +1058,12 @@ class DownloadManager(QAbstractListModel): def download_open(self, cmdline: str=None, count=0): """Open the last/[count]th download. + If no specific command is given, this will use the system's default + application to open the file. + Args: - cmdline: The command which should be used to open the file. A {} as - argument will be replaced by the download's filename. + cmdline: The command which should be used to open the file. A `{}` + is expanded to the temporary file name. count: The index of the download to open. """ try: diff --git a/qutebrowser/mainwindow/statusbar/prompter.py b/qutebrowser/mainwindow/statusbar/prompter.py index 0282f38cf..cbcb965af 100644 --- a/qutebrowser/mainwindow/statusbar/prompter.py +++ b/qutebrowser/mainwindow/statusbar/prompter.py @@ -316,11 +316,12 @@ class Prompter(QObject): def prompt_open_download(self, cmdline: str=None): """Immediately open a download. + If no specific command is given, this will use the system's default + application to open the file. + Args: - cmdline: The command line string to execute. The default will use - the system's application to open the file, depending on - the filetype. A `{}` is expanded to the temporary file - name. + cmdline: The command which should be used to open the file. A `{}` + is expanded to the temporary file name. """ if self._question.mode != usertypes.PromptMode.download: # We just ignore this if we don't have a download question. From 1400a27508081c94ee649867edffbcf383945402 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 3 Aug 2016 18:14:45 +0200 Subject: [PATCH 07/17] more tests for OpenFileDownloadTarget --- tests/unit/utils/usertypes/test_downloadtarget.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/unit/utils/usertypes/test_downloadtarget.py b/tests/unit/utils/usertypes/test_downloadtarget.py index e89401144..4da2e7a81 100644 --- a/tests/unit/utils/usertypes/test_downloadtarget.py +++ b/tests/unit/utils/usertypes/test_downloadtarget.py @@ -41,8 +41,13 @@ def test_fileobj(): def test_openfile(): - # Just make sure no error is raised, that should be enough. - usertypes.OpenFileDownloadTarget() + target = usertypes.OpenFileDownloadTarget() + assert target.cmdline is None + + +def test_openfile_custom_command(): + target = usertypes.OpenFileDownloadTarget('echo') + assert target.cmdline == 'echo' @pytest.mark.parametrize('obj', [ From 5ca8f77fca379da12b85b27b215b6ecb59442a80 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 3 Aug 2016 18:16:28 +0200 Subject: [PATCH 08/17] lint fixes --- qutebrowser/browser/webkit/downloads.py | 1 - qutebrowser/utils/usertypes.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 9a1f40d55..7413d02f4 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -995,7 +995,6 @@ class DownloadManager(QAbstractListModel): return download.open_file(cmdline) - def raise_no_download(self, count): """Raise an exception that the download doesn't exist. diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py index 1ed87af8e..bd4930094 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -300,7 +300,7 @@ class OpenFileDownloadTarget(DownloadTarget): """Save the download in a temp dir and directly open it. Attributes: - cmdline: The command to use as string. A {} is expanded to th + cmdline: The command to use as string. A {} is expanded to the filename. None means use the system's default. """ From d2107498a2c624edf02f7b4598ce0499b4ce4b1e Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 4 Aug 2016 01:06:42 +0200 Subject: [PATCH 09/17] add a test for downloads with long filenames See issue #1725. --- tests/end2end/data/downloads/issue1725.html | 13 +++++++++++++ tests/end2end/features/downloads.feature | 11 +++++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/end2end/data/downloads/issue1725.html diff --git a/tests/end2end/data/downloads/issue1725.html b/tests/end2end/data/downloads/issue1725.html new file mode 100644 index 000000000..7bc0cace2 --- /dev/null +++ b/tests/end2end/data/downloads/issue1725.html @@ -0,0 +1,13 @@ + + + test case for issue 1725 + + +

Using :prompt-open-download with a file that has a loooooong filename

+

+ + Download me! + +

+ + diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index 882f51f3d..0b3b66856 100644 --- a/tests/end2end/features/downloads.feature +++ b/tests/end2end/features/downloads.feature @@ -196,6 +196,17 @@ Feature: Downloading things from a website. And I run :download-cancel Then "* finished but not successful, not opening!" should be logged + ## https://github.com/The-Compiler/qutebrowser/issues/1725 + + Scenario: Directly open a download with a very long filename + When I set storage -> prompt-download-directory to true + And I open data/downloads/issue1725.html + And I run :hint + And I run :follow-hint a + And I directly open the download + And I wait until the download is finished + Then "Opening * with [*python*]" should be logged + ## completion -> download-path-suggestion Scenario: completion -> download-path-suggestion = path From cbe60b06384c5987c6a5464377b07d902605537d Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 4 Aug 2016 01:24:08 +0200 Subject: [PATCH 10/17] add tests for download with special characters See issue 1726. --- tests/end2end/test_invocations.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/end2end/test_invocations.py b/tests/end2end/test_invocations.py index 676c09bab..d34859410 100644 --- a/tests/end2end/test_invocations.py +++ b/tests/end2end/test_invocations.py @@ -19,6 +19,8 @@ """Test starting qutebrowser with special arguments/environments.""" +import sys + import pytest BASE_ARGS = ['--debug', '--json-logging', '--no-err-windows', 'about:blank'] @@ -74,14 +76,28 @@ def test_ascii_locale(httpbin, tmpdir, quteproc_new): """Test downloads with LC_ALL=C set. https://github.com/The-Compiler/qutebrowser/issues/908 + https://github.com/The-Compiler/qutebrowser/issues/1726 """ args = ['--temp-basedir'] + BASE_ARGS quteproc_new.start(args, env={'LC_ALL': 'C'}) quteproc_new.set_setting('storage', 'download-directory', str(tmpdir)) + + # Test a normal download quteproc_new.set_setting('storage', 'prompt-download-directory', 'false') url = 'http://localhost:{port}/data/downloads/ä-issue908.bin'.format( port=httpbin.port) quteproc_new.send_cmd(':download {}'.format(url)) + quteproc_new.wait_for(category='downloads', message='Download finished') + + # Test :prompt-open-download + quteproc_new.set_setting('storage', 'prompt-download-directory', 'true') + quteproc_new.send_cmd(':download {}'.format(url)) + quteproc_new.send_cmd(':prompt-open-download "{}" -c pass' + .format(sys.executable)) + quteproc_new.wait_for(category='downloads', message='Download finished') + quteproc_new.wait_for(category='downloads', + message='Opening * with [*python*]') + quteproc_new.send_cmd(':quit') quteproc_new.wait_for_quit() From 8f4377937d489abcc88146e73dede09907ee373f Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 4 Aug 2016 01:41:50 +0200 Subject: [PATCH 11/17] use shlex.quote instead of "{}" --- tests/end2end/features/test_downloads_bdd.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/end2end/features/test_downloads_bdd.py b/tests/end2end/features/test_downloads_bdd.py index fa7c41fc6..e37eae90e 100644 --- a/tests/end2end/features/test_downloads_bdd.py +++ b/tests/end2end/features/test_downloads_bdd.py @@ -19,6 +19,7 @@ import os import sys +import shlex import pytest_bdd as bdd bdd.scenarios('downloads.feature') @@ -73,10 +74,10 @@ def download_prompt(tmpdir, quteproc, path): @bdd.when("I open the download") def download_open(quteproc): - cmd = '"{}" -c pass'.format(sys.executable) + cmd = '{} -c pass'.format(shlex.quote(sys.executable)) quteproc.send_cmd(':download-open {}'.format(cmd)) @bdd.when("I directly open the download") def download_open(quteproc): - cmd = '"{}" -c pass'.format(sys.executable) + cmd = '{} -c pass'.format(shlex.quote(sys.executable)) quteproc.send_cmd(':prompt-open-download {}'.format(cmd)) From 47b455957f8a6e98bbc07998bb47dc1fed532de7 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 4 Aug 2016 01:47:02 +0200 Subject: [PATCH 12/17] tests: actually download file Otherwise the :prompt-open-download will throw an error since the request is not fast enough. --- tests/end2end/features/downloads.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index 0b3b66856..7ec5da2ae 100644 --- a/tests/end2end/features/downloads.feature +++ b/tests/end2end/features/downloads.feature @@ -201,7 +201,7 @@ Feature: Downloading things from a website. Scenario: Directly open a download with a very long filename When I set storage -> prompt-download-directory to true And I open data/downloads/issue1725.html - And I run :hint + And I run :hint all download And I run :follow-hint a And I directly open the download And I wait until the download is finished From e21bdab2c0c7d0ef46abb6f324152934c9e3e3cb Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 4 Aug 2016 02:14:46 +0200 Subject: [PATCH 13/17] rename function to remove duplicate --- tests/end2end/features/test_downloads_bdd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/end2end/features/test_downloads_bdd.py b/tests/end2end/features/test_downloads_bdd.py index e37eae90e..0f810a6c3 100644 --- a/tests/end2end/features/test_downloads_bdd.py +++ b/tests/end2end/features/test_downloads_bdd.py @@ -78,6 +78,6 @@ def download_open(quteproc): quteproc.send_cmd(':download-open {}'.format(cmd)) @bdd.when("I directly open the download") -def download_open(quteproc): +def download_open_with_prompt(quteproc): cmd = '{} -c pass'.format(shlex.quote(sys.executable)) quteproc.send_cmd(':prompt-open-download {}'.format(cmd)) From b9b3bdf9dd23be93ecb6e0d4b422844abe25266a Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 4 Aug 2016 11:03:05 +0200 Subject: [PATCH 14/17] Minor Text Fixes --- tests/end2end/data/downloads/issue1725.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/end2end/data/downloads/issue1725.html b/tests/end2end/data/downloads/issue1725.html index 7bc0cace2..b413e69f2 100644 --- a/tests/end2end/data/downloads/issue1725.html +++ b/tests/end2end/data/downloads/issue1725.html @@ -5,7 +5,7 @@

Using :prompt-open-download with a file that has a loooooong filename

- + Download me!

From 8730dc6f8e24731f126cd0c73d38540b03ba3bd1 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 4 Aug 2016 11:16:43 +0200 Subject: [PATCH 15/17] tests: use :hint instead of :hint all download :hint all download does not use the response headers to determine the filename (the prompt is shown before a request is even done), so our long filename was pretty useless. :hint works because it does a request first and uses the right filename, but we need to wait until the prompt is shown before we can do :prompt-open-download, since the request is a bit slower and would fail otherwise. --- tests/end2end/features/downloads.feature | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index 7ec5da2ae..5b623442d 100644 --- a/tests/end2end/features/downloads.feature +++ b/tests/end2end/features/downloads.feature @@ -201,8 +201,9 @@ Feature: Downloading things from a website. Scenario: Directly open a download with a very long filename When I set storage -> prompt-download-directory to true And I open data/downloads/issue1725.html - And I run :hint all download + And I run :hint And I run :follow-hint a + And I wait for "Asking question text='Save file to:'>, *" in the log And I directly open the download And I wait until the download is finished Then "Opening * with [*python*]" should be logged From 017bfc9b9caf99e533e1e21b6559fdd49924e632 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 4 Aug 2016 11:50:35 +0200 Subject: [PATCH 16/17] open-download: append filename if no {} is found This allows shortcuts like ":download-open gvim" instead of ":download-open gvim {}" to work. --- qutebrowser/browser/webkit/downloads.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 7413d02f4..422984934 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -554,6 +554,8 @@ class DownloadItem(QObject): cmd, *args = shlex.split(cmdline) args = [arg.replace('{}', filename) for arg in args] + if '{}' not in cmdline: + args.append(filename) log.downloads.debug("Opening {} with {}" .format(filename, [cmd] + args)) proc = guiprocess.GUIProcess(self._win_id, what='download') From d21b42662d2c0864e7e84172bd76e5221c4ef8c8 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 4 Aug 2016 11:54:27 +0200 Subject: [PATCH 17/17] Minor Text Fixes default (program) -> default application surround {} with backticks mention that the filename will be appended in the docstrings --- qutebrowser/browser/webkit/downloads.py | 13 ++++++++----- qutebrowser/mainwindow/statusbar/prompter.py | 4 +++- qutebrowser/utils/usertypes.py | 5 +++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/webkit/downloads.py index 422984934..265d6ce33 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/webkit/downloads.py @@ -534,8 +534,10 @@ class DownloadItem(QObject): """Open the downloaded file. Args: - cmdline: The command to use, or None for the system's default - program. + cmdline: The command to use as string. A `{}` is expanded to the + filename. None means to use the system's default + application. If no `{}` is found, the filename is appended + to the cmdline. """ assert self.successful filename = self._filename @@ -988,8 +990,7 @@ class DownloadManager(QAbstractListModel): Args: download: The DownloadItem to use. - cmdline: The command to use, or None for the system's default - program. + cmdline: Passed to DownloadItem.open_file(). """ if not download.successful: log.downloads.debug("{} finished but not successful, not opening!" @@ -1064,7 +1065,9 @@ class DownloadManager(QAbstractListModel): Args: cmdline: The command which should be used to open the file. A `{}` - is expanded to the temporary file name. + is expanded to the temporary file name. If no `{}` is + present, the filename is automatically appended to the + cmdline. count: The index of the download to open. """ try: diff --git a/qutebrowser/mainwindow/statusbar/prompter.py b/qutebrowser/mainwindow/statusbar/prompter.py index cbcb965af..337b71e1d 100644 --- a/qutebrowser/mainwindow/statusbar/prompter.py +++ b/qutebrowser/mainwindow/statusbar/prompter.py @@ -321,7 +321,9 @@ class Prompter(QObject): Args: cmdline: The command which should be used to open the file. A `{}` - is expanded to the temporary file name. + is expanded to the temporary file name. If no `{}` is + present, the filename is automatically appended to the + cmdline. """ if self._question.mode != usertypes.PromptMode.download: # We just ignore this if we don't have a download question. diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py index bd4930094..65c3c31e2 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -300,8 +300,9 @@ class OpenFileDownloadTarget(DownloadTarget): """Save the download in a temp dir and directly open it. Attributes: - cmdline: The command to use as string. A {} is expanded to the - filename. None means use the system's default. + cmdline: The command to use as string. A `{}` is expanded to the + filename. None means to use the system's default application. + If no `{}` is found, the filename is appended to the cmdline. """ def __init__(self, cmdline=None):