diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index 9835f1a3f..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*]+ @@ -1168,8 +1179,19 @@ Answer no to a yes/no prompt. [[prompt-open-download]] === prompt-open-download +Syntax: +:prompt-open-download ['cmdline']+ + Immediately open a 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. + + +==== note +* This command does not split arguments after the last argument and handles quotes literally. + [[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 e5f9e8709..265d6ce33 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 @@ -528,8 +530,15 @@ 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 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 if filename is None: @@ -537,8 +546,22 @@ 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 = [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') + proc.start_detached(cmd, args) def set_filename(self, filename): """Set the filename to save the download to. @@ -955,19 +978,25 @@ 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: Passed to DownloadItem.open_file(). + """ + if not download.successful: log.downloads.debug("{} finished but not successful, not opening!" .format(download)) + return + download.open_file(cmdline) def raise_no_download(self, count): """Raise an exception that the download doesn't exist. @@ -1026,12 +1055,19 @@ 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. + 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 `{}` + 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: @@ -1042,7 +1078,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) @@ -1334,8 +1370,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/mainwindow/statusbar/prompter.py b/qutebrowser/mainwindow/statusbar/prompter.py index 4fca341fc..337b71e1d 100644 --- a/qutebrowser/mainwindow/statusbar/prompter.py +++ b/qutebrowser/mainwindow/statusbar/prompter.py @@ -312,13 +312,23 @@ 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. + + 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 `{}` + 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. 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..65c3c31e2 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -297,11 +297,17 @@ 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 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): # pylint: disable=super-init-not-called - pass + self.cmdline = cmdline class Question(QObject): 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/end2end/data/downloads/issue1725.html b/tests/end2end/data/downloads/issue1725.html new file mode 100644 index 000000000..b413e69f2 --- /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 7548c0dda..5b623442d 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,36 @@ 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 + + ## 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 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 + ## 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..0f810a6c3 100644 --- a/tests/end2end/features/test_downloads_bdd.py +++ b/tests/end2end/features/test_downloads_bdd.py @@ -18,6 +18,8 @@ # along with qutebrowser. If not, see . import os +import sys +import shlex import pytest_bdd as bdd bdd.scenarios('downloads.feature') @@ -68,3 +70,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(shlex.quote(sys.executable)) + quteproc.send_cmd(':download-open {}'.format(cmd)) + +@bdd.when("I directly open the download") +def download_open_with_prompt(quteproc): + cmd = '{} -c pass'.format(shlex.quote(sys.executable)) + quteproc.send_cmd(':prompt-open-download {}'.format(cmd)) 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() 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): 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', [