Merge branch 'open-download-fixes' of https://github.com/Kingdread/qutebrowser into Kingdread-open-download-fixes

This commit is contained in:
Florian Bruhin 2016-08-04 12:46:44 +02:00
commit 6021dc6394
11 changed files with 229 additions and 29 deletions

View File

@ -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.

View File

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

View File

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

View File

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

View File

@ -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.

View File

@ -0,0 +1,13 @@
<html>
<head>
<title>test case for issue 1725</title>
</head>
<body>
<p>Using <code>:prompt-open-download</code> with a file that has a loooooong filename</p>
<p>
<a href="/response-headers?Content-Disposition=download;%20filename%2A%3DUTF-8%27%27I%2527ve%2520actually%2520felt%2520slightly%2520uncomfortable%2520at%2520TED%2520for%2520the%2520last%2520two%2520days%252C%2520because%2520there%2527s%2520a%2520lot%2520of%2520vision%2520going%2520on%252C%2520right%253F%2520And%2520I%2520am%2520not%2520a%2520visionary.%2520I%2520do%2520not%2520have%2520a%2520five-year%2520plan.%2520I%2527m%2520an%2520engineer.%2520And%2520I%2520think%2520it%2527s%2520really%2520--%2520I%2520mean%2520--%2520I%2527m%2520perfectly%2520happy%2520with%2520all%2520the%2520people%2520who%2520are%2520walking%2520around%2520and%2520just%2520staring%2520at%2520the%2520clouds%2520and%2520looking%2520at%2520the%2520stars%2520and%2520saying%252C%2520%2522I%2520want%2520to%2520go%2520there.%2522%2520But%2520I%2527m%2520looking%2520at%2520the%2520ground%252C%2520and%2520I%2520want%2520to%2520fix%2520the%2520pothole%2520that%2527s%2520right%2520in%2520front%2520of%2520me%2520before%2520I%2520fall%2520in.%2520This%2520is%2520the%2520kind%2520of%2520person%2520I%2520am.txt">
Download me!
</a>
</p>
</body>
</html>

View File

@ -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 <qutebrowser.utils.usertypes.Question default=* mode=<PromptMode.download: 5> 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

View File

@ -18,6 +18,8 @@
# along with qutebrowser. If not, see <http://www.gnu.org/licenses/>.
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))

View File

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

View File

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

View File

@ -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', [