From f0779f8cc043d68874660e5b3f7e03a077a01a3a Mon Sep 17 00:00:00 2001 From: Joel Torstensson Date: Fri, 26 Dec 2014 21:58:45 +0100 Subject: [PATCH 1/4] User now asked if it wants to overwrite existing file. Fix #318 --- qutebrowser/browser/downloads.py | 46 ++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 66154f6d7..40c1fa744 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -171,6 +171,7 @@ class DownloadItem(QObject): _read_timer: A QTimer which reads the QNetworkReply into self._buffer periodically. _retry_info: A RetryInfo instance. + _win_id: The window ID the DownloadItem runs in. Signals: data_changed: The downloads metadata changed. @@ -217,6 +218,7 @@ class DownloadItem(QObject): self.fileobj = None self._filename = None self.init_reply(reply) + self._win_id = parent._win_id def __repr__(self): return utils.get_repr(self, basename=self.basename) @@ -256,6 +258,25 @@ class DownloadItem(QObject): name=self.basename, speed=speed, remaining=remaining, perc=perc, down=down, total=total, errmsg=errmsg)) + def _create_fileobj(self): + """Creates a file object using the internal filename.""" + try: + fileobj = open(self._filename, 'wb') + except OSError as e: + self._die(e.strerror) + else: + self.set_fileobj(fileobj) + + def _create_overwrite_question(self): + """Create a Question object to be asked.""" + q = usertypes.Question(self) + q.text = " already exists. Overwrite? (y/n)" + q.mode = usertypes.PromptMode.yesno + q.answered_yes.connect(self._create_fileobj) + q.answered_no.connect(functools.partial(self.cancel, False)) + q.cancelled.connect(functools.partial(self.cancel, False)) + return q + def _die(self, msg): """Abort the download and emit an error.""" assert not self.successful @@ -312,8 +333,12 @@ class DownloadItem(QObject): return utils.interpolate_color( start, stop, self.stats.percentage(), system) - def cancel(self): - """Cancel the download.""" + def cancel(self, remove_data=True): + """Cancel the download. + + Args: + remove_data: Whether to remove the downloaded data. + """ log.downloads.debug("cancelled") self._read_timer.stop() self.cancelled.emit() @@ -325,7 +350,8 @@ class DownloadItem(QObject): if self.fileobj is not None: self.fileobj.close() try: - if self._filename is not None and os.path.exists(self._filename): + if (self._filename is not None and os.path.exists(self._filename) + and remove_data): os.remove(self._filename) except OSError: log.downloads.exception("Failed to remove partial file") @@ -376,12 +402,16 @@ class DownloadItem(QObject): self._filename = os.path.join(download_dir, filename) self.basename = filename log.downloads.debug("Setting filename to {}".format(filename)) - try: - fileobj = open(self._filename, 'wb') - except OSError as e: - self._die(e.strerror) + if os.path.isfile(self._filename): + # The file already exists, so ask the user if it should be + # overwritten. + q = self._create_overwrite_question() + q.text = self._filename + q.text + message_bridge = objreg.get('message-bridge', scope='window', + window=self._win_id) + message_bridge.ask(q, blocking=False) else: - self.set_fileobj(fileobj) + self._create_fileobj() def set_fileobj(self, fileobj): """"Set the file object to write the download to. From 6c6ae4e465f36db1f42206689c5b023249182a65 Mon Sep 17 00:00:00 2001 From: Joel Torstensson Date: Sat, 27 Dec 2014 00:50:52 +0100 Subject: [PATCH 2/4] Refactored question logic. --- qutebrowser/browser/downloads.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 40c1fa744..079eabe3b 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -267,15 +267,17 @@ class DownloadItem(QObject): else: self.set_fileobj(fileobj) - def _create_overwrite_question(self): + def _ask_overwrite_question(self): """Create a Question object to be asked.""" q = usertypes.Question(self) - q.text = " already exists. Overwrite? (y/n)" + q.text = self._filename + " already exists. Overwrite? (y/n)" q.mode = usertypes.PromptMode.yesno q.answered_yes.connect(self._create_fileobj) q.answered_no.connect(functools.partial(self.cancel, False)) q.cancelled.connect(functools.partial(self.cancel, False)) - return q + message_bridge = objreg.get('message-bridge', scope='window', + window=self._win_id) + message_bridge.ask(q, blocking=False) def _die(self, msg): """Abort the download and emit an error.""" @@ -405,11 +407,7 @@ class DownloadItem(QObject): if os.path.isfile(self._filename): # The file already exists, so ask the user if it should be # overwritten. - q = self._create_overwrite_question() - q.text = self._filename + q.text - message_bridge = objreg.get('message-bridge', scope='window', - window=self._win_id) - message_bridge.ask(q, blocking=False) + self._ask_overwrite_question() else: self._create_fileobj() From ed253f23c60cb1e0332f8cc9213e6b38e943c818 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 28 Dec 2014 02:00:31 +0100 Subject: [PATCH 3/4] Pass window id to DownloadItem. --- qutebrowser/browser/downloads.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 079eabe3b..bd4527601 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -194,7 +194,7 @@ class DownloadItem(QObject): redirected = pyqtSignal(QNetworkRequest, QNetworkReply) do_retry = pyqtSignal('QNetworkReply') - def __init__(self, reply, parent=None): + def __init__(self, reply, win_id, parent=None): """Constructor. Args: @@ -218,7 +218,7 @@ class DownloadItem(QObject): self.fileobj = None self._filename = None self.init_reply(reply) - self._win_id = parent._win_id + self._win_id = win_id def __repr__(self): return utils.get_repr(self, basename=self.basename) @@ -694,7 +694,7 @@ class DownloadManager(QAbstractListModel): _inline, suggested_filename = http.parse_content_disposition(reply) log.downloads.debug("fetch: {} -> {}".format(reply.url(), suggested_filename)) - download = DownloadItem(reply, self) + download = DownloadItem(reply, self._win_id, self) download.cancelled.connect( functools.partial(self.remove_item, download)) if config.get('ui', 'remove-finished-downloads') or auto_remove: From 553a2fbcbde270385e97eb89dc4e14a2e7d7df21 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sun, 28 Dec 2014 02:01:59 +0100 Subject: [PATCH 4/4] Regenerate authors --- README.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/README.asciidoc b/README.asciidoc index edb4c3df1..759c8a719 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -133,6 +133,7 @@ Contributors, sorted by the number of commits in descending order: * Mathias Fussenegger * Larry Hynes * Johannes Altmanninger +* Joel Torstensson * Regina Hug * Peter Vilim * Helen Sherwood-Taylor