qutebrowser / qutebrowser

A keyboard-driven, vim-like browser based on Python and Qt.
https://www.qutebrowser.org/
GNU General Public License v3.0
9.62k stars 1k forks source link

Selecting downloads with :download-* #2384

Open al-caveman opened 7 years ago

al-caveman commented 7 years ago

I suggest to modify the :download-delete command to take the optional arguments ITEMS_LIST and --items-num NUM as follows:


Update: sorry, I think --items-num NUM is redundant, and having ITEMS_LIST is enough. E.g. this is the behaviour that I suggest:

pkillnine commented 7 years ago

What is the use-case of deleting the last x downloaded files?

I think deleting all downloaded files would be better with a --all flag as --items-num 0 isn't obvious and also is more likely to be done on accident.

al-caveman commented 7 years ago

Sorry, I updated my suggestion with an alternative one. What do you think about it now?

The-Compiler commented 7 years ago

This should be true for other :download-* commands as well - I adjusted the title accordingly.

What do you mean with:

  • download-delete 1-5 will delete the past 5 downloaded items.

though? Wouldn't that just delete the downloads with ID 1-5?

al-caveman commented 7 years ago

Sorry you are right. I corrected that line.

lahwaacz commented 7 years ago

Is 1 the newest or oldest download? What about using negative numbers for indexing from the other end?

The-Compiler commented 7 years ago

1 is the download with an 1: next to it, i.e. the oldest one. Not sure if indexing from the other end really is needed, since you can just look at the numbers and do 5-last. With something like -10--5 this might get funny to parse :wink:

lahwaacz commented 7 years ago

Aha, there are actually numbers in the download bar! :+1:

The-Compiler commented 7 years ago

Abandoned patch from #2493:

From 380105836aabd302d946ee089e3d9d5789e48288 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Fri, 31 Mar 2017 15:26:56 +0400
Subject: [PATCH 01/17] fixes #2384 as per [1]

fixes it only as per the suggestion from pkill-nine here [1] by adding the
argument --all/-a to download-delete. but does not add the fancy selection
thingy that i asked for in #2384.

[1] https://github.com/qutebrowser/qutebrowser/issues/2384#issuecomment-283448617
---
 qutebrowser/browser/downloads.py | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index 51ccdbfebb..11d2a278a3 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -911,23 +911,30 @@ def download_cancel(self, all_=False, count=0):

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_delete(self, count=0):
+    def download_delete(self, all_=False, count=0):
         """Delete the last/[count]th download from disk.

         Args:
+            all: Delete all listed downloads.
             count: The index of the download to delete.
         """
-        try:
-            download = self[count - 1]
-        except IndexError:
-            self._raise_no_download(count)
-        if not download.successful:
-            if not count:
-                count = len(self)
-            raise cmdexc.CommandError("Download {} is not done!".format(count))
-        download.delete()
-        download.remove()
-        log.downloads.debug("deleted download {}".format(download))
+
+        repeat = count+1
+        if all_:
+            repeat = len(self)
+
+        for i in range(0, repeat):
+            try:
+                download = self[count - 1]
+            except IndexError:
+                self._raise_no_download(count)
+            if not download.successful:
+                if not count:
+                    count = len(self)
+                raise cmdexc.CommandError("Download {} is not done!".format(count))
+            download.delete()
+            download.remove()
+            log.downloads.debug("deleted download {}".format(download))

     @cmdutils.register(instance='download-model', scope='window', maxsplit=0)
     @cmdutils.argument('count', count=True)

From e2c38fa2c160f84b0978d188279792399071edc8 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Fri, 31 Mar 2017 18:34:37 +0400
Subject: [PATCH 02/17] fixes #2384 (fully; except testing code)

removes argument --all/-a from download-delete, and introduces the more general
[indexset] to specify ranges of indexes such as: 1,5,7-last,3.

it also alters one default warning behaviour: the function only warns if there is:
  - a download that is incomplete, in the set of to-be-deleted downloads,
  - or, if the downloads list is empty,
  - or, if the indexset does not match any download.

for example, if one deletes a set of downloads, such that:
  1. all of them are completed downloads.
  2. and some of them deleted, but some others are not deleted (because they
  did not exist),
  3. then, the whatever that is deletable is deleted, and the none-existent
  downloads are ignored silently. the user is expected to notice that some
  items were gone from the downloads list, and that some are still there.
---
 qutebrowser/browser/downloads.py | 53 +++++++++++++++++++++++++++++++---------
 qutebrowser/utils/utils.py       | 32 ++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index 11d2a278a3..a69021f286 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -911,27 +911,56 @@ def download_cancel(self, all_=False, count=0):

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_delete(self, all_=False, count=0):
+    def download_delete(self, indexset=None, count=0):
         """Delete the last/[count]th download from disk.

         Args:
-            all: Delete all listed downloads.
+            indexset: Specify the set of download indexes to delete. E.g.
+                '2,5,6-last,3', where 'last' is special keyword that denotes
+                that latest download index. If you specify numbers smaller than
+                1, or greater than 'last', then those numbers will be silently
+                ignored.
             count: The index of the download to delete.
         """

-        repeat = count+1
-        if all_:
-            repeat = len(self)
+        # if there are no downloads, raise an exception
+        if len(self) == 0:
+            self._raise_no_download(count)

-        for i in range(0, repeat):
+        # get list of indexes to delete
+        if indexset == None:
+            indexes_to_del = [count]
+        else:
             try:
-                download = self[count - 1]
+                indexes_to_del = [i for i in utils.parse_number_sets(indexset, 1, len(self))]
+            except ValueError:
+                raise cmdexc.CommandError("Invalid index set '{}'!".format(indexset))
+
+        # get list of downloaded items to delete
+        download_to_del = []
+        for i in indexes_to_del:
+            j = i - 1
+            try:
+                download = self[j]
+                if download.successful:
+                    download_to_del.append(download)
+                else:
+                    raise cmdexc.CommandError("Download {} is not done!".format(i))
             except IndexError:
-                self._raise_no_download(count)
-            if not download.successful:
-                if not count:
-                    count = len(self)
-                raise cmdexc.CommandError("Download {} is not done!".format(count))
+                # silently ignore the case when a particular download index 'i'
+                # does not exist. in caveman's view, there is no need to be too
+                # chatty, and that it is maybe better to calm down, and
+                # complain later on only if there is absolutely nothing to
+                # delete
+                pass
+
+        # complain only if there is absolutely nothing to delete. caveman
+        # thinks this is a good idea.
+        if len(download_to_del) == 0:
+            raise cmdexc.CommandError("None of the downloaded items matched!")
+
+        # delete downloads
+        for download in download_to_del:
             download.delete()
             download.remove()
             log.downloads.debug("deleted download {}".format(download))
diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index fd4466e2a9..310b87a980 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -845,3 +845,35 @@ def open_file(filename, cmdline=None):
 def unused(_arg):
     """Function which does nothing to avoid pylint complaining."""
     pass
+
+
+def parse_number_sets(txtset, nummin, nummax):
+    """Parse the given numbers set in text format, and return it as a list of
+    numbers, such that the numbers are never smaller than nummin, and never
+    greater than nummax.
+
+    Args:
+        txtset: Numbers set specification. E.g. '1,5,8-last,32,2', where 'last'
+        is a special keyword that denotes that maximum allowable number.
+        nummin: Minimum allowable number.
+        nummax: Maximum allowable number.
+    """
+    txtset = txtset.lower().replace('last', '%d' % (nummax))
+    numset = set()
+    parts = txtset.split(',')
+    for part in parts:
+        if part.find('-') != -1:
+            [start, end] = part.split('-')
+            start = int(start)
+            end = int(end)
+            if start < nummin:
+                start = nummin
+            if end > nummax:
+                end = nummax
+            for i in range(int(start), int(end)+1):
+                numset.add(i)
+        else:
+            i = int(part)
+            if i >= nummin and i <= nummax:
+                numset.add(i)
+    return sorted(numset)

From f32d8a990252e64a09319a3d8ec01db3054d1e22 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Fri, 31 Mar 2017 19:30:08 +0400
Subject: [PATCH 03/17] fixes #2384 (complains about incomplete downloads after
 deleting whatever is deletable)

probably really dirty code. needs rethinking. i'll see if i can apply
The-Compiler's advise in creating a cleaner generic method that makes this
feature consistently usable by all download-* commands.
---
 qutebrowser/browser/downloads.py | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index a69021f286..37845f6606 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -938,6 +938,7 @@ def download_delete(self, indexset=None, count=0):

         # get list of downloaded items to delete
         download_to_del = []
+        download_incomplete = []
         for i in indexes_to_del:
             j = i - 1
             try:
@@ -945,16 +946,15 @@ def download_delete(self, indexset=None, count=0):
                 if download.successful:
                     download_to_del.append(download)
                 else:
-                    raise cmdexc.CommandError("Download {} is not done!".format(i))
+                    download_incomplete.append(i)
             except IndexError:
-                # silently ignore the case when a particular download index 'i'
-                # does not exist. in caveman's view, there is no need to be too
-                # chatty, and that it is maybe better to calm down, and
-                # complain later on only if there is absolutely nothing to
-                # delete
+                # i (caveman) think, there is no need to be too
+                # interactive/online. we probably better delete whatever is
+                # deletable first, and then complain about various errors later
+                # on. so for now we do nothing
                 pass

-        # complain only if there is absolutely nothing to delete. caveman
+        # complain if there is absolutely nothing to delete. caveman
         # thinks this is a good idea.
         if len(download_to_del) == 0:
             raise cmdexc.CommandError("None of the downloaded items matched!")
@@ -965,6 +965,14 @@ def download_delete(self, indexset=None, count=0):
             download.remove()
             log.downloads.debug("deleted download {}".format(download))

+        # raise exception for those downloads that were incomplete
+        if len(download_incomplete) > 0:
+            plural = 's'
+            if (download_incomplete) == 1:
+                plural = ''
+            raise cmdexc.CommandError("Download%s %s is not done!" % (plural,
+            ', '.join([str(i) for i in download_incomplete])))
+
     @cmdutils.register(instance='download-model', scope='window', maxsplit=0)
     @cmdutils.argument('count', count=True)
     def download_open(self, cmdline: str=None, count=0):

From 38279523226340d60cba4ce4b8d14824759f6981 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Tue, 11 Apr 2017 06:09:28 +0400
Subject: [PATCH 04/17] fixes #2384

all download_* commands have indexset text argument, that allows users to
select download indexes based on descriptions, such as "1,5,3,7-last" (without
double quotes).

additionally, to simplify the code (as suggested by The-Compiler in PR #2493),
this selection of downloads based on indexset texts is generalized and
implemented in a method named _download_select(...), and used by all download_*
commands.

finally, utils/utils.py is simplified compared to the previous patch, so that
it is more stupid, and does only one simple task, without extra voodoo (like
putting lower/upper bounds on numbers that indexset texts expand do)
---
 qutebrowser/browser/downloads.py | 359 ++++++++++++++++++++++++++-------------
 qutebrowser/utils/utils.py       |  16 +-
 2 files changed, 248 insertions(+), 127 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index 6adb67a521..4480bf5234 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -894,154 +894,274 @@ def _on_data_changed(self, idx, *, webengine):
             qtutils.ensure_valid(end_index)
         self.dataChanged.emit(start_index, end_index)

-    def _raise_no_download(self, count):
-        """Raise an exception that the download doesn't exist.
+    def _raise_error_download(self, successfuls, notsuccessfuls, dones, notdones,
+                             nonexistents):
+        """Raise exception for each category of downloads. if a category should
+        not have any exceptions raised for, then pass an empty list for the
+        category.

         Args:
-            count: The index of the download
+            successfuls     : The indexes of the successful downloads
+            notsuccessfuls  : The indexes of the downloads that are not
+                              successful
+            dones           : The indexes of the downloads that are done
+            notdones        : The indexes of the downloads that are not
+                              done 
+            nonexistents    : The indexes of the nonexistent downloads
         """
-        if not count:
-            raise cmdexc.CommandError("There's no download!")
-        raise cmdexc.CommandError("There's no download {}!".format(count))

-    @cmdutils.register(instance='download-model', scope='window')
-    @cmdutils.argument('count', count=True)
-    def download_cancel(self, all_=False, count=0):
-        """Cancel the last/[count]th download.
-
-        Args:
-            all_: Cancel all running downloads
-            count: The index of the download to cancel.
-        """
-        downloads = self._all_downloads()
-        if all_:
-            for download in downloads:
-                if not download.done:
-                    download.cancel()
-        else:
-            try:
-                download = downloads[count - 1]
-            except IndexError:
-                self._raise_no_download(count)
-            if download.done:
-                if not count:
-                    count = len(self)
-                raise cmdexc.CommandError("Download {} is already done!"
-                                        .format(count))
-            download.cancel()
-
-    @cmdutils.register(instance='download-model', scope='window')
-    @cmdutils.argument('count', count=True)
-    def download_delete(self, indexset=None, count=0):
-        """Delete the last/[count]th download from disk.
+        msg_error = ''
+
+        if len(successfuls):
+            if msg_error == '':
+                msg_error += ', '
+            msg_error += "{} are successful!".format(','.join(
+                            [str(i[0]) for i in successfuls]
+                         ))
+
+        if len(notsuccessfuls):
+            if msg_error == '':
+                msg_error += ', '
+            msg_error += "{} are not successful!".format(','.join(
+                            [str(i[0]) for i in notsuccessfuls]
+                         ))
+
+        if len(dones):
+            if msg_error == '':
+                msg_error += ', '
+            msg_error += "{} are done!".format(','.join(
+                            [str(i[0]) for i in dones]
+                         ))
+
+        if len(notdones):
+            if msg_error == '':
+                msg_error += ', '
+            msg_error += "{} are not done!".format(','.join(
+                            [str(i[0]) for i in notdones]
+                         ))
+
+        if len(nonexistents):
+            if msg_error == '':
+                msg_error += ', '
+            msg_error += "{} are nonexistent!".format(','.join(
+                            [str(i) for i in nonexistents if i != '']
+                         ))
+
+        print('ERROR: successfulls: %s' % successfuls)
+        print('ERROR: notsuccessfulls: %s' % notsuccessfuls)
+        print('ERROR: dones: %s' % dones)
+        print('ERROR: notdones: %s' % notdones)
+        print('ERROR: nonexistents: %s' % nonexistents)
+        
+
+        if msg_error != '':
+            raise cmdexc.CommandError("Downloads {}".format(msg_error))
+
+    def _download_select(self, count, indexset):
+        """Select download objects that match the selection criterion, and then
+        classify them as completed, incomplete, and nonexistent downloads.

         Args:
+            count: Single index item to pick (for compatibility).
             indexset: Specify the set of download indexes to delete. E.g.
                 '2,5,6-last,3', where 'last' is special keyword that denotes
                 that latest download index. If you specify numbers smaller than
                 1, or greater than 'last', then those numbers will be silently
                 ignored.
-            count: The index of the download to delete.
         """

-        # if there are no downloads, raise an exception
-        if len(self) == 0:
-            self._raise_no_download(count)
+        indexset_p = []

-        # get list of indexes to delete
+        # parse set of indexes to delete (parsed based on count, or the
+        # indexset)
         if indexset == None:
-            indexes_to_del = [count]
+            indexset_p.append(count)
         else:
             try:
-                indexes_to_del = [i for i in utils.parse_number_sets(indexset, 1, len(self))]
+                for i in utils.parse_number_sets(indexset, len(self)):
+                    indexset_p.add(i)
             except ValueError:
-                raise cmdexc.CommandError("Invalid index set '{}'!".format(indexset))
+                raise cmdexc.CommandError(
+                        "Invalid index set '{}'!".format(indexset)
+                      )

         # get list of downloaded items to delete
-        download_to_del = []
-        download_incomplete = []
-        for i in indexes_to_del:
-            j = i - 1
+        successfuls = []
+        notsuccessfuls = []
+        dones = []
+        notdones = []
+        nonexistents = []
+        for i in sorted(indexset_p, reverse=True):
+            j = i - 1   # humans usually count from 1, but python usually counts
+                        # from 0. let's make python's life easier, cause
+                        # happier python also means happier programmer. let's
+                        # face it, a developer, unlike a pure end-user, needs
+                        # to make both happy. so this is the point where we
+                        # (devs) try to work to make sure everyone is happy
+                        # (pure end user, and python) by making this
+                        # translation of how indexes are defined.
+                        #
+                        # however, to be honest, i think humans, including pure
+                        # end-users, should count from zero *shrug*. maybe one
+                        # day we should expect that pure end-users also count
+                        # from 0?
             try:
                 download = self[j]
                 if download.successful:
-                    download_to_del.append(download)
+                    successfuls.append([i, download])
+                else:
+                    notsuccessfuls.append([i, download])
+                if download.done:
+                    dones.append([i, download])
                 else:
-                    download_incomplete.append(i)
+                    notdones.append([i, download])
             except IndexError:
-                # i (caveman) think, there is no need to be too
-                # interactive/online. we probably better delete whatever is
-                # deletable first, and then complain about various errors later
-                # on. so for now we do nothing
-                pass
-
-        # complain if there is absolutely nothing to delete. caveman
-        # thinks this is a good idea.
-        if len(download_to_del) == 0:
-            raise cmdexc.CommandError("None of the downloaded items matched!")
-
-        # delete downloads
-        for download in download_to_del:
-            download.delete()
-            download.remove()
-            log.downloads.debug("deleted download {}".format(download))
-
-        # raise exception for those downloads that were incomplete
-        if len(download_incomplete) > 0:
-            plural = 's'
-            if (download_incomplete) == 1:
-                plural = ''
-            raise cmdexc.CommandError("Download%s %s is not done!" % (plural,
-            ', '.join([str(i) for i in download_incomplete])))
+                nonexistents.append(i)
+
+        print('SELECT: successfulls: %s' % successfuls)
+        print('SELECT: notsuccessfulls: %s' % notsuccessfuls)
+        print('SELECT: dones: %s' % dones)
+        print('SELECT: notdones: %s' % notdones)
+        print('SELECT: nonexistents: %s' % nonexistents)
+        
+        return (successfuls, notsuccessfuls, dones, notdones, nonexistents)
+
+    @cmdutils.register(instance='download-model', scope='window')
+    @cmdutils.argument('count', count=True)
+    def download_cancel(self, indexset=None, count=0):
+        """Cancel the last/[count]th download.
+
+        Args:
+            indexset: Specify the set of download indexes to delete. E.g.
+                      '2,5,6-last,3', where 'last' is special keyword that
+                      denotes that latest download index. If you specify
+                      numbers smaller than 1, or greater than 'last', then
+                      those numbers will be silently ignored.
+            count: The index of the download to cancel.
+        """
+
+        # get categorized lists of downloads that match the selected criteria
+        # (count and indexset)
+        (
+            successfuls,
+            notsuccessfuls,
+            dones,
+            notdones,
+            nonexistents
+        ) = self._download_select(count, indexset)
+
+        # cancel all that's cancelable
+        for (i,d) in notdones:
+            download.cancel()
+
+        # tell the user about what wasn't cancelable
+        self._raise_error_download([], [], dones, [], nonexistents)
+
+    @cmdutils.register(instance='download-model', scope='window')
+    @cmdutils.argument('count', count=True)
+    def download_delete(self, indexset=None, count=0):
+        """Delete the last/[count]th download from disk.
+
+        Args:
+            indexset: Specify the set of download indexes to delete. E.g.
+                      '2,5,6-last,3', where 'last' is special keyword that
+                      denotes that latest download index. If you specify
+                      numbers smaller than 1, or greater than 'last', then
+                      those numbers will be silently ignored.
+            count: The index of the download to delete.
+        """
+
+        # get categorized lists of downloads that match the selected criteria
+        # (count and indexset)
+        (
+            successfuls,
+            notsuccessfuls,
+            dones,
+            notdones,
+            nonexistents
+        ) = self._download_select(count, indexset)
+
+        # delete all that are deletable (completed downloads)
+        for (i,d) in successfuls:
+            d.delete()
+            d.remove()
+            log.downloads.debug("deleted download {}".format(i))
+
+        # raise an exception to tell the user about downloads that were
+        # undeletable. namely, those that were incomplelte or nonexistent.
+        self._raise_error_download([], notsuccessfuls, [], [], nonexistents)

     @cmdutils.register(instance='download-model', scope='window', maxsplit=0)
     @cmdutils.argument('count', count=True)
-    def download_open(self, cmdline: str=None, count=0):
+    def download_open(self, indexset=None, 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:
+            indexset: Specify the set of download indexes to delete. E.g.
+                      '2,5,6-last,3', where 'last' is special keyword that
+                      denotes that latest download index. If you specify
+                      numbers smaller than 1, or greater than 'last', then
+                      those numbers will be silently ignored.
             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:
-            download = self[count - 1]
-        except IndexError:
-            self._raise_no_download(count)
-        if not download.successful:
-            if not count:
-                count = len(self)
-            raise cmdexc.CommandError("Download {} is not done!".format(count))
-        download.open_file(cmdline)
+
+        # get categorized lists of downloads that match the selected criteria
+        # (count and indexset)
+        (
+            successfuls,
+            notsuccessfuls,
+            dones,
+            notdones,
+            nonexistents
+        ) = self._download_select(count, indexset)
+
+        # open all that are openable (completed downloads)
+        for (i,d) in successfuls:
+            d.open_file(cmdline)
+
+        # raise an exception to tell the user about downloads that were
+        # unopenable. namely, those that were incomplelte or nonexistent.
+        self._raise_error_download([], notsuccessfuls, [], [], nonexistents)

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_retry(self, count=0):
+    def download_retry(self, indexset=None, count=0):
         """Retry the first failed/[count]th download.

         Args:
+            indexset: Specify the set of download indexes to delete. E.g.
+                      '2,5,6-last,3', where 'last' is special keyword that
+                      denotes that latest download index. If you specify
+                      numbers smaller than 1, or greater than 'last', then
+                      those numbers will be silently ignored.
             count: The index of the download to retry.
         """
-        if count:
-            try:
-                download = self[count - 1]
-            except IndexError:
-                self._raise_no_download(count)
-            if download.successful or not download.done:
-                raise cmdexc.CommandError("Download {} did not fail!".format(
-                    count))
-        else:
-            to_retry = [d for d in self if d.done and not d.successful]
-            if not to_retry:
-                raise cmdexc.CommandError("No failed downloads!")
-            else:
-                download = to_retry[0]
-        download.try_retry()
+
+        # get categorized lists of downloads that match the selected criteria
+        # (count and indexset)
+        (
+            successfuls,
+            notsuccessfuls,
+            dones,
+            notdones,
+            nonexistents
+        ) = self._download_select(count, indexset)
+
+        # retry all that are retryable (i.e. those that are, simultaneously, done and
+        # notsuccessful)
+        for (i,d) in [dt for dt in dones if dt in notsuccessfuls]:
+            d.try_retry()
+
+        # raise an exception to tell the user about downloads that were
+        # not retryable. namely, those that were successful or nonexistent.
+        self._raise_error_download(successfuls, [], [], notdones, nonexistents)

     def can_clear(self):
         """Check if there are finished downloads to clear."""
@@ -1056,26 +1176,35 @@ def download_clear(self):

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_remove(self, all_=False, count=0):
+    def download_remove(self, indexset=None, count=0):
         """Remove the last/[count]th download from the list.

         Args:
-            all_: Remove all finished downloads.
+            indexset: Specify the set of download indexes to delete. E.g.
+                      '2,5,6-last,3', where 'last' is special keyword that
+                      denotes that latest download index. If you specify
+                      numbers smaller than 1, or greater than 'last', then
+                      those numbers will be silently ignored.
             count: The index of the download to remove.
         """
-        if all_:
-            self.download_clear()
-        else:
-            try:
-                download = self[count - 1]
-            except IndexError:
-                self._raise_no_download(count)
-            if not download.done:
-                if not count:
-                    count = len(self)
-                raise cmdexc.CommandError("Download {} is not done!"
-                                          .format(count))
-            download.remove()
+
+        # get categorized lists of downloads that match the selected criteria
+        # (count and indexset)
+        (
+            successfuls,
+            notsuccessfuls,
+            dones,
+            notdones,
+            nonexistents
+        ) = self._download_select(count, indexset)
+
+        # remove what's removable
+        for (i,d) in dones:
+            d.remove()
+
+        # raise an exception to tell the user about downloads that were
+        # not removable. namely, those that were not done or nonexistent.
+        self._raise_error_download([], [], [], notdones, nonexistents)

     def running_downloads(self):
         """Return the amount of still running downloads.
diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index dd42e37053..5a5ac5c46f 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -864,16 +864,14 @@ def unused(_arg):
     pass

-def parse_number_sets(txtset, nummin, nummax):
+def parse_number_sets(txtset, nummax):
     """Parse the given numbers set in text format, and return it as a list of
-    numbers, such that the numbers are never smaller than nummin, and never
-    greater than nummax.
+    numbers.

     Args:
         txtset: Numbers set specification. E.g. '1,5,8-last,32,2', where 'last'
         is a special keyword that denotes that maximum allowable number.
-        nummin: Minimum allowable number.
-        nummax: Maximum allowable number.
+        nummax: The number that will be used to replace 'last'
     """
     txtset = txtset.lower().replace('last', '%d' % (nummax))
     numset = set()
@@ -883,16 +881,10 @@ def parse_number_sets(txtset, nummin, nummax):
             [start, end] = part.split('-')
             start = int(start)
             end = int(end)
-            if start < nummin:
-                start = nummin
-            if end > nummax:
-                end = nummax
             for i in range(int(start), int(end)+1):
                 numset.add(i)
         else:
-            i = int(part)
-            if i >= nummin and i <= nummax:
-                numset.add(i)
+            numset.add(int(part))
     return sorted(numset)

From 2066548567b85298713f9879387be8b64b93a945 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Tue, 11 Apr 2017 06:23:52 +0400
Subject: [PATCH 05/17] removed sorted(...). i don't think it was needed

---
 qutebrowser/browser/downloads.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index 4480bf5234..aa1ec143e2 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -991,7 +991,7 @@ def _download_select(self, count, indexset):
         dones = []
         notdones = []
         nonexistents = []
-        for i in sorted(indexset_p, reverse=True):
+        for i in indexset_p:
             j = i - 1   # humans usually count from 1, but python usually counts
                         # from 0. let's make python's life easier, cause
                         # happier python also means happier programmer. let's

From cb76a87a81b6342c8fcf8e54aa11622503fb0739 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Tue, 11 Apr 2017 10:37:38 +0400
Subject: [PATCH 06/17] 1) added --all/-a to all download-* commands (including
 download-delete,    which didn't have this before).

2) added function utils.parse_list_into_numsettxt(...) in utils.py, which
   summarizes number lists into list descriptors, such as '1,3-100'.

3) that function is used when printing error messages of download-*. for
   example, if you say ":download-delete 1-1000", and you only have 1-10
   downloads, the warning will show "Downloads 11-1000 don't exist"
---
 qutebrowser/browser/downloads.py | 112 +++++++++++++++++++++++----------------
 qutebrowser/utils/utils.py       |  45 +++++++++++++---
 2 files changed, 104 insertions(+), 53 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index aa1ec143e2..e739a8d4fc 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -913,46 +913,40 @@ def _raise_error_download(self, successfuls, notsuccessfuls, dones, notdones,
         msg_error = ''

         if len(successfuls):
-            if msg_error == '':
-                msg_error += ', '
-            msg_error += "{} are successful!".format(','.join(
-                            [str(i[0]) for i in successfuls]
-                         ))
+            if msg_error != '':
+                msg_error += ' '
+            msg_error += "{} are successful!".format(
+                            utils.parse_list_into_numsettxt(successfuls)
+                         )

         if len(notsuccessfuls):
-            if msg_error == '':
-                msg_error += ', '
-            msg_error += "{} are not successful!".format(','.join(
-                            [str(i[0]) for i in notsuccessfuls]
-                         ))
+            if msg_error != '':
+                msg_error += ' '
+            msg_error += "{} are not successful!".format(
+                            utils.parse_list_into_numsettxt(notsuccessfuls)
+                         )

         if len(dones):
-            if msg_error == '':
-                msg_error += ', '
-            msg_error += "{} are done!".format(','.join(
-                            [str(i[0]) for i in dones]
-                         ))
+            if msg_error != '':
+                msg_error += ' '
+            msg_error += "{} are done!".format(
+                            utils.parse_list_into_numsettxt(dones)
+                         )

         if len(notdones):
-            if msg_error == '':
-                msg_error += ', '
-            msg_error += "{} are not done!".format(','.join(
-                            [str(i[0]) for i in notdones]
-                         ))
+            if msg_error != '':
+                msg_error += ' '
+            msg_error += "{} are not done!".format(
+                            utils.parse_list_into_numsettxt(notdones)
+                         )

         if len(nonexistents):
-            if msg_error == '':
-                msg_error += ', '
-            msg_error += "{} are nonexistent!".format(','.join(
-                            [str(i) for i in nonexistents if i != '']
-                         ))
-
-        print('ERROR: successfulls: %s' % successfuls)
-        print('ERROR: notsuccessfulls: %s' % notsuccessfuls)
-        print('ERROR: dones: %s' % dones)
-        print('ERROR: notdones: %s' % notdones)
-        print('ERROR: nonexistents: %s' % nonexistents)
-        
+            if msg_error != '':
+                msg_error += ' '
+            msg_error += "{} are nonexistent!".format(
+                            utils.parse_list_into_numsettxt(nonexistents)
+                         )
+

         if msg_error != '':
             raise cmdexc.CommandError("Downloads {}".format(msg_error))
@@ -978,8 +972,8 @@ def _download_select(self, count, indexset):
             indexset_p.append(count)
         else:
             try:
-                for i in utils.parse_number_sets(indexset, len(self)):
-                    indexset_p.add(i)
+                for i in utils.parse_numsettxt_into_list(indexset, len(self)):
+                    indexset_p.append(i)
             except ValueError:
                 raise cmdexc.CommandError(
                         "Invalid index set '{}'!".format(indexset)
@@ -1017,18 +1011,12 @@ def _download_select(self, count, indexset):
                     notdones.append([i, download])
             except IndexError:
                 nonexistents.append(i)
-
-        print('SELECT: successfulls: %s' % successfuls)
-        print('SELECT: notsuccessfulls: %s' % notsuccessfuls)
-        print('SELECT: dones: %s' % dones)
-        print('SELECT: notdones: %s' % notdones)
-        print('SELECT: nonexistents: %s' % nonexistents)

         return (successfuls, notsuccessfuls, dones, notdones, nonexistents)

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_cancel(self, indexset=None, count=0):
+    def download_cancel(self, indexset=None, all_=False, count=0):
         """Cancel the last/[count]th download.

         Args:
@@ -1037,9 +1025,15 @@ def download_cancel(self, indexset=None, count=0):
                       denotes that latest download index. If you specify
                       numbers smaller than 1, or greater than 'last', then
                       those numbers will be silently ignored.
+            all_: Cancels all downloads. This is a shortcut to using the list
+                  specification '1-last'.
             count: The index of the download to cancel.
         """

+        # support for --all,-a
+        if all_ == True:
+            indexset = '1-last'
+
         # get categorized lists of downloads that match the selected criteria
         # (count and indexset)
         (
@@ -1059,7 +1053,7 @@ def download_cancel(self, indexset=None, count=0):

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_delete(self, indexset=None, count=0):
+    def download_delete(self, indexset=None, all_=False, count=0):
         """Delete the last/[count]th download from disk.

         Args:
@@ -1068,9 +1062,15 @@ def download_delete(self, indexset=None, count=0):
                       denotes that latest download index. If you specify
                       numbers smaller than 1, or greater than 'last', then
                       those numbers will be silently ignored.
+            all_: Cancels all downloads. This is a shortcut to using the list
+                  specification '1-last'.
             count: The index of the download to delete.
         """

+        # support for --all,-a
+        if all_ == True:
+            indexset = '1-last'
+
         # get categorized lists of downloads that match the selected criteria
         # (count and indexset)
         (
@@ -1093,7 +1093,7 @@ def download_delete(self, indexset=None, count=0):

     @cmdutils.register(instance='download-model', scope='window', maxsplit=0)
     @cmdutils.argument('count', count=True)
-    def download_open(self, indexset=None, cmdline: str=None, count=0):
+    def download_open(self, indexset=None, all_=False, cmdline: str=None, count=0):
         """Open the last/[count]th download.

         If no specific command is given, this will use the system's default
@@ -1105,6 +1105,8 @@ def download_open(self, indexset=None, cmdline: str=None, count=0):
                       denotes that latest download index. If you specify
                       numbers smaller than 1, or greater than 'last', then
                       those numbers will be silently ignored.
+            all_: Cancels all downloads. This is a shortcut to using the list
+                  specification '1-last'.
             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
@@ -1112,6 +1114,10 @@ def download_open(self, indexset=None, cmdline: str=None, count=0):
             count: The index of the download to open.
         """

+        # support for --all,-a
+        if all_ == True:
+            indexset = '1-last'
+
         # get categorized lists of downloads that match the selected criteria
         # (count and indexset)
         (
@@ -1132,7 +1138,7 @@ def download_open(self, indexset=None, cmdline: str=None, count=0):

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_retry(self, indexset=None, count=0):
+    def download_retry(self, indexset=None, all_=False, count=0):
         """Retry the first failed/[count]th download.

         Args:
@@ -1141,9 +1147,15 @@ def download_retry(self, indexset=None, count=0):
                       denotes that latest download index. If you specify
                       numbers smaller than 1, or greater than 'last', then
                       those numbers will be silently ignored.
+            all_: Cancels all downloads. This is a shortcut to using the list
+                  specification '1-last'.
             count: The index of the download to retry.
         """

+        # support for --all,-a
+        if all_ == True:
+            indexset = '1-last'
+
         # get categorized lists of downloads that match the selected criteria
         # (count and indexset)
         (
@@ -1154,8 +1166,8 @@ def download_retry(self, indexset=None, count=0):
             nonexistents
         ) = self._download_select(count, indexset)

-        # retry all that are retryable (i.e. those that are, simultaneously, done and
-        # notsuccessful)
+        # retry all that are retryable (i.e. those that are, simultaneously,
+        # done and notsuccessful)
         for (i,d) in [dt for dt in dones if dt in notsuccessfuls]:
             d.try_retry()

@@ -1176,7 +1188,7 @@ def download_clear(self):

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_remove(self, indexset=None, count=0):
+    def download_remove(self, indexset=None, all_=False, count=0):
         """Remove the last/[count]th download from the list.

         Args:
@@ -1185,9 +1197,15 @@ def download_remove(self, indexset=None, count=0):
                       denotes that latest download index. If you specify
                       numbers smaller than 1, or greater than 'last', then
                       those numbers will be silently ignored.
+            all_: Cancels all downloads. This is a shortcut to using the list
+                  specification '1-last'.
             count: The index of the download to remove.
         """

+        # support for --all,-a
+        if all_ == True:
+            indexset = '1-last'
+
         # get categorized lists of downloads that match the selected criteria
         # (count and indexset)
         (
diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index 5a5ac5c46f..a66665826b 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -864,17 +864,18 @@ def unused(_arg):
     pass

-def parse_number_sets(txtset, nummax):
+def parse_numsettxt_into_list(txtset, nummax):
     """Parse the given numbers set in text format, and return it as a list of
     numbers.

     Args:
         txtset: Numbers set specification. E.g. '1,5,8-last,32,2', where 'last'
-        is a special keyword that denotes that maximum allowable number.
+                is a special keyword that denotes that maximum allowable
+                number.
         nummax: The number that will be used to replace 'last'
     """
     txtset = txtset.lower().replace('last', '%d' % (nummax))
-    numset = set()
+    l = set()
     parts = txtset.split(',')
     for part in parts:
         if part.find('-') != -1:
@@ -882,12 +883,44 @@ def parse_number_sets(txtset, nummax):
             start = int(start)
             end = int(end)
             for i in range(int(start), int(end)+1):
-                numset.add(i)
+                l.add(i)
         else:
-            numset.add(int(part))
-    return sorted(numset)
+            l.add(int(part))
+    return sorted(l)

+def parse_list_into_numsettxt(l):
+    """Summarise list of numbers into a numbers set description in text.
+    Basically undoes the effect of parse_numsettxt_into_list(...). For example,
+    l = [1,5,8,9,10,11,35] will be summarized into the string '1,5,8-11,35'.
+
+    Args:
+        l: Numbers list to be summarized back into text set descriptors. 
+    """
+
+    state = 'start'
+    for i in sorted(l):
+        if state == 'start':
+            txtset = str(i)
+            state = 'inside'
+        elif state == 'inside':
+            diff = i - last_i
+            if diff > 1:
+                txtset += ',{}'.format(i)
+            else:
+                txtset += '-'.format(i)
+                state = 'inside-dashed'
+        elif state == 'inside-dashed':
+            diff = i - last_i
+            if diff > 1:
+                txtset += '{},{}'.format(last_i,i)
+                state = 'inside'
+        last_i = i
+    if state == 'inside-dashed':
+        txtset += '{}'.format(i)
+
+    return txtset
+
 def expand_windows_drive(path):
     r"""Expand a drive-path like E: into E:\.

From 7887913c51ebe6fe27c8473e18464125ae0c1f8b Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Tue, 11 Apr 2017 10:50:37 +0400
Subject: [PATCH 07/17] fixed error text in _raise_error_download(...)_

---
 qutebrowser/browser/downloads.py | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index e739a8d4fc..31276c43eb 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -916,28 +916,36 @@ def _raise_error_download(self, successfuls, notsuccessfuls, dones, notdones,
             if msg_error != '':
                 msg_error += ' '
             msg_error += "{} are successful!".format(
-                            utils.parse_list_into_numsettxt(successfuls)
+                            utils.parse_list_into_numsettxt(
+                                [i[0] for i in successfuls]
+                            )
                          )

         if len(notsuccessfuls):
             if msg_error != '':
                 msg_error += ' '
             msg_error += "{} are not successful!".format(
-                            utils.parse_list_into_numsettxt(notsuccessfuls)
+                            utils.parse_list_into_numsettxt(
+                                [i[0] for i in notsuccessfuls]
+                            )
                          )

         if len(dones):
             if msg_error != '':
                 msg_error += ' '
             msg_error += "{} are done!".format(
-                            utils.parse_list_into_numsettxt(dones)
+                            utils.parse_list_into_numsettxt(
+                                [i[0] for i in dones]
+                            )
                          )

         if len(notdones):
             if msg_error != '':
                 msg_error += ' '
             msg_error += "{} are not done!".format(
-                            utils.parse_list_into_numsettxt(notdones)
+                            utils.parse_list_into_numsettxt(
+                                [i[0] for i in notdones]
+                            )
                          )

         if len(nonexistents):

From c0dd571c3cb5f19ddbec6e4e820e5a3ffdbaa035 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Fri, 14 Apr 2017 01:11:46 +0400
Subject: [PATCH 08/17] made the code less cryptic. and found a bug.

the bug that i found seems outside of my code. what makes me feel that
this is a bug, is that this crash only happens with `download-retry`.

the bug:
--------

    the command `:download-retry -a` (or any variant that causes
    multiple downloads to be retried), will cause qutebrowser to
    crash, if there are multiple failed downloads.

    to create multiple failed downloads, i simply used the command
    `:download lol` knowing that host `lol` doesn't exist. Similarly
    with host `haha`.

below is the crash message:
---------------------------

    01:11:00 ERROR: Download error: Host lol not found
    01:11:00 ERROR: Download error: Host haha not found
    01:11:04 ERROR: Uncaught exception
    Traceback (most recent call last):
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/app.py", line 882, in eventFilter
        return handler(event)
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/app.py", line 842, in _handle_key_event
        return man.eventFilter(event)
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/keyinput/modeman.py", line 337, in eventFilter
        return self._eventFilter_keypress(event)
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/keyinput/modeman.py", line 168, in _eventFilter_keypress
        handled = parser.handle(event)
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/keyinput/basekeyparser.py", line 307, in handle
        handled = self._handle_special_key(e)
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/keyinput/basekeyparser.py", line 136, in _handle_special_key
        self.execute(cmdstr, self.Type.special, count)
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/keyinput/keyparser.py", line 44, in execute
        self._commandrunner.run(cmdstr, count)
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/commands/runners.py", line 275, in run
        result.cmd.run(self._win_id, args, count=count)
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/commands/command.py", line 525, in run
        self.handler(*posargs, **kwargs)
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/mainwindow/prompt.py", line 372, in prompt_accept
        question.done()
      File "/home/caveman/torabora/dev/qutebrowser/qutebrowser/utils/usertypes.py", line 339, in done
        self.answered.emit(self.answer)
    RuntimeError: wrapped C/C++ object of type Question has been deleted
---
 qutebrowser/browser/downloads.py | 176 ++++++++++++++++++++++-----------------
 1 file changed, 101 insertions(+), 75 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index 34adfce678..c93816e9ea 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -894,10 +894,9 @@ def _on_data_changed(self, idx, *, webengine):
             qtutils.ensure_valid(end_index)
         self.dataChanged.emit(start_index, end_index)

-    def _selective_dos(self, count, all_, indexset, criterion, dos,
-    err_postfix):
-        """Apply actions in dos against downloads specified by indexset that
-        satisfy the criterion.
+    def _select_downloads(self, count, all_, indexset, criterion):
+        """Return selected downloads specified by indexset that satisfy the
+        criterion.

         Args:
             count: count of download to get the actions applied against.
@@ -908,10 +907,6 @@ def _selective_dos(self, count, all_, indexset, criterion, dos,
                 1, or greater than 'last', then those numbers will be silently
                 ignored.
             criterion: A function that returns True only when satisfied.
-            dos: A set of functions that apply actions against satisfying
-                 downloads.
-            err_postfix: The text to print after printing the list of indexes
-                         that don't match the criterion.
         """

         # support --all/-a
@@ -951,23 +946,7 @@ def _selective_dos(self, count, all_, indexset, criterion, dos,
             except IndexError:
                 noexists.append(i)

-        # apply actions on matched downloads
-        for d in matchs:
-            for do in dos:
-                do(d)
-
-        # display errors - downloads that didn't match the criterion
-        if len(nomatchs):
-            raise cmdexc.CommandError("Downloads {} {}!".format(
-                    ", ".join([str(i) for i in nomatchs]),
-                    err_postfix
-                ))
-
-        # display errors - downloads that don't exist
-        if len(noexists):
-            raise cmdexc.CommandError("Downloads {} don't exist!".format(
-                    ", ".join([str(i) for i in noexists])
-                ))
+        return (matchs, nomatchs, noexists)

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
@@ -985,15 +964,25 @@ def download_cancel(self, indexset=None, all_=False, count=0):
             count: The index of the download to cancel.
         """

-        # cancel all downloads that are matched
-        self._selective_dos(
-            count,                  # support count
-            all_,                   # support --all/-a
-            indexset,               # select downloads
-            lambda d: d.done,       # that match this criteria
-            [lambda d: d.cancel()], # to do these against
-            "are not done yet"      # error postfix
-        )
+        # find matched downloads
+        (matchs, nomatchs, noexists) = self._select_downloads(
+                                           count,
+                                           all_,
+                                           indexset,
+                                           lambda d: not d.done
+                                       )
+
+        # cancel them
+        for d in matchs:
+            d.cancel()
+
+        # show errors if needed
+        if len(nomatchs):
+            message.error("Downloads {} are already done!".format(
+            utils.parse_list_into_numsettxt(nomatchs)))
+        if len(noexists):
+            message.error("Downloads {} don't exist!".format(
+            utils.parse_list_into_numsettxt(noexists)))

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
@@ -1011,20 +1000,27 @@ def download_delete(self, indexset=None, all_=False, count=0):
             count: The index of the download to delete.
         """

-        # cancel all downloads that are matched
-        self._selective_dos(
-            count,                                # support count
-            all_,                                 # support --all/-a
-            indexset,                             # select downloads
-            lambda d: d.done,                     # matching this
-            [
-                lambda d: d.delete(),
-                lambda d: d.remove(),
-                lambda d: d.remove(),
-                lambda d: log.downloads.debug("deleted download {}".format(d))
-            ],                                    # to do these against
-            "are not successfully downloaded yet" # error postfix
-        )
+        # find matched downloads
+        (matchs, nomatchs, noexists) = self._select_downloads(
+                                           count,
+                                           all_,
+                                           indexset,
+                                           lambda d: d.successful
+                                       )
+
+        # delete them
+        for d in matchs:
+            d.delete(),
+            d.remove(),
+            log.downloads.debug("Deleted download {}".format(d))
+
+        # show errors if needed
+        if len(nomatchs):
+            message.error("Downloads {} are not successful yet!".format(
+            utils.parse_list_into_numsettxt(nomatchs)))
+        if len(noexists):
+            message.error("Downloads {} don't exist!".format(
+            utils.parse_list_into_numsettxt(noexists)))

     @cmdutils.register(instance='download-model', scope='window', maxsplit=0)
     @cmdutils.argument('count', count=True)
@@ -1049,15 +1045,25 @@ def download_open(self, indexset=None, all_=False, cmdline: str=None, count=0):
             count: The index of the download to open.
         """

-        # cancel all downloads that are matched
-        self._selective_dos(
-            count,                                # support count
-            all_,                                 # support --all/-a
-            indexset,                             # select downloads
-            lambda d: d.done,                     # matching this
-            [lambda d: d.open_file(cmdline)],     # to do these against
-            "are not successfully downloaded yet" # error postfix (not done)
-        )
+        # find matched downloads
+        (matchs, nomatchs, noexists) = self._select_downloads(
+                                           count,
+                                           all_,
+                                           indexset,
+                                           lambda d: d.successful
+                                       )
+
+        # open them
+        for d in matchs:
+            d.open_file(cmdline)
+
+        # show errors if needed
+        if len(nomatchs):
+            message.error("Downloads {} are not successful!".format(
+            utils.parse_list_into_numsettxt(nomatchs)))
+        if len(noexists):
+            message.error("Downloads {} don't exist!".format(
+            utils.parse_list_into_numsettxt(noexists)))

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
@@ -1075,15 +1081,25 @@ def download_retry(self, indexset=None, all_=False, count=0):
             count: The index of the download to retry.
         """

-        # cancel all downloads that are matched
-        self._selective_dos(
-            count,                      # support count
-            all_,                       # support --all/-a
-            indexset,                   # select downloads
-            lambda d: d.done and not d.successful, # matching this
-            [lambda d: d.try_retry()],  # to do these against
-            "are fine"                  # error postfix (not done)
-        )
+        # find matched downloads
+        (matchs, nomatchs, noexists) = self._select_downloads(
+                                        count,
+                                        all_,
+                                        indexset,
+                                        lambda d: d.done and not d.successful
+                                    )
+
+        # retry them
+        for d in matchs:
+            d.try_retry()
+
+        # show errors if needed
+        if len(nomatchs):
+            message.error("Downloads {} are fine!".format(
+            utils.parse_list_into_numsettxt(nomatchs)))
+        if len(noexists):
+            message.error("Downloads {} don't exist!".format(
+            utils.parse_list_into_numsettxt(noexists)))

     def can_clear(self):
         """Check if there are finished downloads to clear."""
@@ -1112,15 +1128,25 @@ def download_remove(self, indexset=None, all_=False, count=0):
             count: The index of the download to remove.
         """

-        # cancel all downloads that are matched
-        self._selective_dos(
-            count,                  # support count
-            all_,                   # support --all/-a
-            indexset,               # select downloads
-            lambda d: d.done,       # matching this
-            [lambda d: d.remove()], # to do these against
-            "are not done yet"      # error postfix (not done)
-        )
+        # find matche downloads 
+        (matchs, nomatchs, noexists) = self._select_downloads(
+                                           count,
+                                           all_,
+                                           indexset,
+                                           lambda d: d.done
+                                       )
+
+        # remove them
+        for d in matchs:
+            d.remove()
+
+        # show errors if needed
+        if len(nomatchs):
+            message.error("Downloads {} are in progress!".format(
+            utils.parse_list_into_numsettxt(nomatchs)))
+        if len(noexists):
+            message.error("Downloads {} don't exist!".format(
+            utils.parse_list_into_numsettxt(noexists)))

     def running_downloads(self):
         """Return the amount of still running downloads.

From 855be26c2f41545da89b2ee91d5832d7163ec814 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Fri, 14 Apr 2017 22:35:08 +0400
Subject: [PATCH 09/17] applied all The-Compiler comments, except:     1.
 comment on line 947 (why not simply show error here?)     2. comment on line
 981 (remove all erorrs in _select_downloads?)

my reasons are mentioned in the comments. basically, i applied parts
that i understood, and raised questions about parts htat i don't
undrestand.
---
 qutebrowser/browser/downloads.py | 147 ++++++++++++++++++++-------------------
 qutebrowser/utils/utils.py       |  25 +++----
 2 files changed, 85 insertions(+), 87 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index c93816e9ea..27d0f2fac9 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -908,13 +908,15 @@ def _select_downloads(self, count, all_, indexset, criterion):
                 ignored.
             criterion: A function that returns True only when satisfied.
         """
-
         # support --all/-a
         if all_ == True:
             indexset = '1-last'
+            message.warning(
+                "'--all/-a' will be deprecated. Kindly use '0-last' instead."
+            )

         # support count
-        if indexset == None:
+        if indexset is None:
             if count == 0:
                 count = len(self)
             indexset = str(count)
@@ -933,10 +935,7 @@ def _select_downloads(self, count, all_, indexset, criterion):
         nomatchs = []
         noexists = []
         for i in indexset_p:
-            j = i-1 # j is the download item when counting from 0, i is the one
-                    # when counting from 1 - do we need this? maybe one day we
-                    # should count downloads, everywhere, from 0, and get away
-                    # with this?
+            j = i - 1
             try:
                 d = self[j]
                 if criterion(d):
@@ -946,7 +945,15 @@ def _select_downloads(self, count, all_, indexset, criterion):
             except IndexError:
                 noexists.append(i)

-        return (matchs, nomatchs, noexists)
+        # tell the user about nonexistent downloads
+        if len(noexists):
+            message.error("Download{} {} {} exist!".format(
+                "s" if len(noexists) > 1 else "",
+                utils.parse_list_into_numsettxt(nomatchs),
+                "don't" if len(noexists) > 1 else "doesn't"
+            ))
+
+        return (matchs, nomatchs)

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
@@ -965,12 +972,12 @@ def download_cancel(self, indexset=None, all_=False, count=0):
         """

         # find matched downloads
-        (matchs, nomatchs, noexists) = self._select_downloads(
-                                           count,
-                                           all_,
-                                           indexset,
-                                           lambda d: not d.done
-                                       )
+        (matchs, nomatchs) = self._select_downloads(
+                                 count,
+                                 all_,
+                                 indexset,
+                                 lambda d: not d.done
+                             )

         # cancel them
         for d in matchs:
@@ -978,15 +985,15 @@ def download_cancel(self, indexset=None, all_=False, count=0):

         # show errors if needed
         if len(nomatchs):
-            message.error("Downloads {} are already done!".format(
-            utils.parse_list_into_numsettxt(nomatchs)))
-        if len(noexists):
-            message.error("Downloads {} don't exist!".format(
-            utils.parse_list_into_numsettxt(noexists)))
+            message.error("Download{} {} {} already done!".format(
+                "s" if len(nomatchs) > 1 else "",
+                utils.parse_list_into_numsettxt(nomatchs),
+                "are" if len(nomatchs) > 1 else "is"
+            ))

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_delete(self, indexset=None, all_=False, count=0):
+    def download_delete(self, indexset=None, count=0):
         """Delete the last/[count]th download from disk.

         Args:
@@ -995,36 +1002,34 @@ def download_delete(self, indexset=None, all_=False, count=0):
                       denotes that latest download index. If you specify
                       numbers smaller than 1, or greater than 'last', then
                       those numbers will be silently ignored.
-            all_: Cancels all downloads. This is a shortcut to using the list
-                  specification '1-last'.
             count: The index of the download to delete.
         """

         # find matched downloads
-        (matchs, nomatchs, noexists) = self._select_downloads(
-                                           count,
-                                           all_,
-                                           indexset,
-                                           lambda d: d.successful
-                                       )
+        (matchs, nomatchs) = self._select_downloads(
+                                 count,
+                                 False,
+                                 indexset,
+                                 lambda d: d.successful
+                             )

         # delete them
         for d in matchs:
-            d.delete(),
-            d.remove(),
+            d.delete()
+            d.remove()
             log.downloads.debug("Deleted download {}".format(d))

         # show errors if needed
         if len(nomatchs):
-            message.error("Downloads {} are not successful yet!".format(
-            utils.parse_list_into_numsettxt(nomatchs)))
-        if len(noexists):
-            message.error("Downloads {} don't exist!".format(
-            utils.parse_list_into_numsettxt(noexists)))
+            message.error("Download{} {} {} not successful yet!".format(
+                "s" if len(nomatchs) > 1 else "",
+                utils.parse_list_into_numsettxt(nomatchs),
+                "are" if len(nomatchs) > 1 else "is"
+            ))

     @cmdutils.register(instance='download-model', scope='window', maxsplit=0)
     @cmdutils.argument('count', count=True)
-    def download_open(self, indexset=None, all_=False, cmdline: str=None, count=0):
+    def download_open(self, indexset=None, cmdline: str=None, count=0):
         """Open the last/[count]th download.

         If no specific command is given, this will use the system's default
@@ -1036,8 +1041,6 @@ def download_open(self, indexset=None, all_=False, cmdline: str=None, count=0):
                       denotes that latest download index. If you specify
                       numbers smaller than 1, or greater than 'last', then
                       those numbers will be silently ignored.
-            all_: Cancels all downloads. This is a shortcut to using the list
-                  specification '1-last'.
             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
@@ -1046,12 +1049,12 @@ def download_open(self, indexset=None, all_=False, cmdline: str=None, count=0):
         """

         # find matched downloads
-        (matchs, nomatchs, noexists) = self._select_downloads(
-                                           count,
-                                           all_,
-                                           indexset,
-                                           lambda d: d.successful
-                                       )
+        (matchs, nomatchs) = self._select_downloads(
+                                 count,
+                                 False,
+                                 indexset,
+                                 lambda d: d.successful
+                             )

         # open them
         for d in matchs:
@@ -1059,15 +1062,15 @@ def download_open(self, indexset=None, all_=False, cmdline: str=None, count=0):

         # show errors if needed
         if len(nomatchs):
-            message.error("Downloads {} are not successful!".format(
-            utils.parse_list_into_numsettxt(nomatchs)))
-        if len(noexists):
-            message.error("Downloads {} don't exist!".format(
-            utils.parse_list_into_numsettxt(noexists)))
+            message.error("Download{} {} {} not successful yet!".format(
+                "s" if len(nomatchs) > 1 else "",
+                utils.parse_list_into_numsettxt(nomatchs),
+                "are" if len(nomatchs) > 1 else "is"
+            ))

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
-    def download_retry(self, indexset=None, all_=False, count=0):
+    def download_retry(self, indexset=None, count=0):
         """Retry the first failed/[count]th download.

         Args:
@@ -1076,18 +1079,16 @@ def download_retry(self, indexset=None, all_=False, count=0):
                       denotes that latest download index. If you specify
                       numbers smaller than 1, or greater than 'last', then
                       those numbers will be silently ignored.
-            all_: Cancels all downloads. This is a shortcut to using the list
-                  specification '1-last'.
             count: The index of the download to retry.
         """

         # find matched downloads
-        (matchs, nomatchs, noexists) = self._select_downloads(
-                                        count,
-                                        all_,
-                                        indexset,
-                                        lambda d: d.done and not d.successful
-                                    )
+        (matchs, nomatchs) = self._select_downloads(
+                                 count,
+                                 False,
+                                 indexset,
+                                 lambda d: d.done and not d.successful
+                             )

         # retry them
         for d in matchs:
@@ -1095,11 +1096,11 @@ def download_retry(self, indexset=None, all_=False, count=0):

         # show errors if needed
         if len(nomatchs):
-            message.error("Downloads {} are fine!".format(
-            utils.parse_list_into_numsettxt(nomatchs)))
-        if len(noexists):
-            message.error("Downloads {} don't exist!".format(
-            utils.parse_list_into_numsettxt(noexists)))
+            message.error("Download{} {} {} fine!".format(
+                "s" if len(nomatchs) > 1 else "",
+                utils.parse_list_into_numsettxt(nomatchs),
+                "are" if len(nomatchs) > 1 else "is"
+            ))

     def can_clear(self):
         """Check if there are finished downloads to clear."""
@@ -1129,12 +1130,12 @@ def download_remove(self, indexset=None, all_=False, count=0):
         """

         # find matche downloads 
-        (matchs, nomatchs, noexists) = self._select_downloads(
-                                           count,
-                                           all_,
-                                           indexset,
-                                           lambda d: d.done
-                                       )
+        (matchs, nomatchs) = self._select_downloads(
+                                 count,
+                                 all_,
+                                 indexset,
+                                 lambda d: d.done
+                             )

         # remove them
         for d in matchs:
@@ -1142,11 +1143,11 @@ def download_remove(self, indexset=None, all_=False, count=0):

         # show errors if needed
         if len(nomatchs):
-            message.error("Downloads {} are in progress!".format(
-            utils.parse_list_into_numsettxt(nomatchs)))
-        if len(noexists):
-            message.error("Downloads {} don't exist!".format(
-            utils.parse_list_into_numsettxt(noexists)))
+            message.error("Download{} {} {} in progress!".format(
+                "s" if len(nomatchs) > 1 else "",
+                utils.parse_list_into_numsettxt(nomatchs),
+                "are" if len(nomatchs) > 1 else "is"
+            ))

     def running_downloads(self):
         """Return the amount of still running downloads.
diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index a66665826b..208e245f76 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -874,32 +874,29 @@ def parse_numsettxt_into_list(txtset, nummax):
                 number.
         nummax: The number that will be used to replace 'last'
     """
-    txtset = txtset.lower().replace('last', '%d' % (nummax))
-    l = set()
-    parts = txtset.split(',')
-    for part in parts:
-        if part.find('-') != -1:
-            [start, end] = part.split('-')
-            start = int(start)
-            end = int(end)
+    txtset = txtset.lower().replace('last', str(nummax))
+    numbers = set()
+    for part in txtset.split(','):
+        if '-' in part:
+            start, end = part.split('-')
             for i in range(int(start), int(end)+1):
-                l.add(i)
+                numbers.add(i)
         else:
-            l.add(int(part))
-    return sorted(l)
+            numbers.add(int(part))
+    return sorted(numbers)

-def parse_list_into_numsettxt(l):
+def parse_list_into_numsettxt(numbers):
     """Summarise list of numbers into a numbers set description in text.
     Basically undoes the effect of parse_numsettxt_into_list(...). For example,
     l = [1,5,8,9,10,11,35] will be summarized into the string '1,5,8-11,35'.

     Args:
-        l: Numbers list to be summarized back into text set descriptors. 
+        numbers: Numbers list to be summarized back into text set descriptors. 
     """

     state = 'start'
-    for i in sorted(l):
+    for i in sorted(numbers):
         if state == 'start':
             txtset = str(i)
             state = 'inside'

From 1ebd37a664cc52414a8a783767df4d8f2f7c3b4f Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Fri, 14 Apr 2017 22:57:14 +0400
Subject: [PATCH 10/17] small bug fix that i introduced while applying some of
 the comments from PR #2493 (put a list in place of another)

---
 qutebrowser/browser/downloads.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index 27d0f2fac9..8e6ce32a06 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -949,7 +949,7 @@ def _select_downloads(self, count, all_, indexset, criterion):
         if len(noexists):
             message.error("Download{} {} {} exist!".format(
                 "s" if len(noexists) > 1 else "",
-                utils.parse_list_into_numsettxt(nomatchs),
+                utils.parse_list_into_numsettxt(noexists),
                 "don't" if len(noexists) > 1 else "doesn't"
             ))

From d62d208ba196518319ca40422a639b731950a56c Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Fri, 14 Apr 2017 23:33:02 +0400
Subject: [PATCH 11/17] removed needless ()s

---
 qutebrowser/browser/downloads.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index 8e6ce32a06..4526f7d4d8 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -972,12 +972,12 @@ def download_cancel(self, indexset=None, all_=False, count=0):
         """

         # find matched downloads
-        (matchs, nomatchs) = self._select_downloads(
+        matchs, nomatchs = self._select_downloads(
                                  count,
                                  all_,
                                  indexset,
                                  lambda d: not d.done
-                             )
+                           )

         # cancel them
         for d in matchs:
@@ -1006,12 +1006,12 @@ def download_delete(self, indexset=None, count=0):
         """

         # find matched downloads
-        (matchs, nomatchs) = self._select_downloads(
+        matchs, nomatchs = self._select_downloads(
                                  count,
                                  False,
                                  indexset,
                                  lambda d: d.successful
-                             )
+                           )

         # delete them
         for d in matchs:
@@ -1049,12 +1049,12 @@ def download_open(self, indexset=None, cmdline: str=None, count=0):
         """

         # find matched downloads
-        (matchs, nomatchs) = self._select_downloads(
+        matchs, nomatchs = self._select_downloads(
                                  count,
                                  False,
                                  indexset,
                                  lambda d: d.successful
-                             )
+                           )

         # open them
         for d in matchs:
@@ -1083,12 +1083,12 @@ def download_retry(self, indexset=None, count=0):
         """

         # find matched downloads
-        (matchs, nomatchs) = self._select_downloads(
+        matchs, nomatchs = self._select_downloads(
                                  count,
                                  False,
                                  indexset,
                                  lambda d: d.done and not d.successful
-                             )
+                           )

         # retry them
         for d in matchs:
@@ -1130,12 +1130,12 @@ def download_remove(self, indexset=None, all_=False, count=0):
         """

         # find matche downloads 
-        (matchs, nomatchs) = self._select_downloads(
+        matchs, nomatchs = self._select_downloads(
                                  count,
                                  all_,
                                  indexset,
                                  lambda d: d.done
-                             )
+                           )

         # remove them
         for d in matchs:

From 6093202f5c14a87200e2ba46fad9f545c7fc4272 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Sat, 15 Apr 2017 13:10:57 +0400
Subject: [PATCH 12/17] made _select_downloads() independent of the
 user-specified set size

---
 qutebrowser/browser/downloads.py | 169 ++++++++++++++++++---------------------
 qutebrowser/utils/utils.py       |  74 ++++++++---------
 2 files changed, 114 insertions(+), 129 deletions(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index 4526f7d4d8..f3d6a9f23a 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -894,7 +894,7 @@ def _on_data_changed(self, idx, *, webengine):
             qtutils.ensure_valid(end_index)
         self.dataChanged.emit(start_index, end_index)

-    def _select_downloads(self, count, all_, indexset, criterion):
+    def _select_downloads(self, count, all_, indexset, criterion, errmsg):
         """Return selected downloads specified by indexset that satisfy the
         criterion.

@@ -907,7 +907,15 @@ def _select_downloads(self, count, all_, indexset, criterion):
                 1, or greater than 'last', then those numbers will be silently
                 ignored.
             criterion: A function that returns True only when satisfied.
+            errmsg: The postfix of the error message that is shown alongside
+                    list of downloads that do not match the criterion.
         """
+        # test if there are any downloads at all
+        if len(self) == 0:
+            raise cmdexc.CommandError(
+                    "No downloads!".format(indexset)
+                  )
+
         # support --all/-a
         if all_ == True:
             indexset = '1-last'
@@ -921,39 +929,49 @@ def _select_downloads(self, count, all_, indexset, criterion):
                 count = len(self)
             indexset = str(count)

-        # parse the indexset into numbers set
+        # parse the indexset into number intervals list
         try:
-            indexset_p = utils.parse_numsettxt_into_list(indexset, len(self))
+            indexset_p = utils.parse_numsettxt_into_numints(indexset, len(self))
         except ValueError:
             raise cmdexc.CommandError(
                     "Invalid index set '{}'!".format(indexset)
                   )

-        # identify downloads that match the criterion, as well as those that
-        # don't exist
+        # identify downloads indexes that fall in the indexset
+        ins = utils.which_nums_in_numints(
+                range(1, len(self) + 1),
+                indexset_p
+              )
+
+        # tell the user if no downloads matched his query
+        if len(ins) == 0:
+            raise cmdexc.CommandError(
+                    "No downloads with indexes that match '{}'!".format(indexset)
+                  )
+
+        # identify which of those that are in the indexset, match the criteria,
+        # don't match the criteria.
         matchs = []
         nomatchs = []
-        noexists = []
-        for i in indexset_p:
+        for i in ins:
             j = i - 1
-            try:
-                d = self[j]
-                if criterion(d):
-                    matchs.append(d)
-                else:
-                    nomatchs.append(i)
-            except IndexError:
-                noexists.append(i)
-
-        # tell the user about nonexistent downloads
-        if len(noexists):
-            message.error("Download{} {} {} exist!".format(
-                "s" if len(noexists) > 1 else "",
-                utils.parse_list_into_numsettxt(noexists),
-                "don't" if len(noexists) > 1 else "doesn't"
+            d = self[j]
+            if criterion(d):
+                matchs.append(d)
+            else:
+                nomatchs.append(i)
+
+        # tell the user about the downloads that exist, but didn't match the
+        # criteria
+        if len(nomatchs):
+            message.error("Download{} {} {} {}!".format(
+                "s" if len(nomatchs) > 1 else "",
+                ','.join([str(i) for i in nomatchs]),
+                "are" if len(nomatchs) > 1 else "is",
+                errmsg
             ))

-        return (matchs, nomatchs)
+        return matchs

     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
@@ -972,25 +990,18 @@ def download_cancel(self, indexset=None, all_=False, count=0):
         """

         # find matched downloads
-        matchs, nomatchs = self._select_downloads(
-                                 count,
-                                 all_,
-                                 indexset,
-                                 lambda d: not d.done
-                           )
+        matchs = self._select_downloads(
+                       count,
+                       all_,
+                       indexset,
+                       lambda d: not d.done,
+                       "already done"
+                 )

         # cancel them
         for d in matchs:
             d.cancel()

-        # show errors if needed
-        if len(nomatchs):
-            message.error("Download{} {} {} already done!".format(
-                "s" if len(nomatchs) > 1 else "",
-                utils.parse_list_into_numsettxt(nomatchs),
-                "are" if len(nomatchs) > 1 else "is"
-            ))
-
     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
     def download_delete(self, indexset=None, count=0):
@@ -1006,12 +1017,13 @@ def download_delete(self, indexset=None, count=0):
         """

         # find matched downloads
-        matchs, nomatchs = self._select_downloads(
-                                 count,
-                                 False,
-                                 indexset,
-                                 lambda d: d.successful
-                           )
+        matchs = self._select_downloads(
+                       count,
+                       False,
+                       indexset,
+                       lambda d: d.successful,
+                       "not successful yet"
+                 )

         # delete them
         for d in matchs:
@@ -1019,14 +1031,6 @@ def download_delete(self, indexset=None, count=0):
             d.remove()
             log.downloads.debug("Deleted download {}".format(d))

-        # show errors if needed
-        if len(nomatchs):
-            message.error("Download{} {} {} not successful yet!".format(
-                "s" if len(nomatchs) > 1 else "",
-                utils.parse_list_into_numsettxt(nomatchs),
-                "are" if len(nomatchs) > 1 else "is"
-            ))
-
     @cmdutils.register(instance='download-model', scope='window', maxsplit=0)
     @cmdutils.argument('count', count=True)
     def download_open(self, indexset=None, cmdline: str=None, count=0):
@@ -1049,25 +1053,18 @@ def download_open(self, indexset=None, cmdline: str=None, count=0):
         """

         # find matched downloads
-        matchs, nomatchs = self._select_downloads(
-                                 count,
-                                 False,
-                                 indexset,
-                                 lambda d: d.successful
-                           )
+        matchs = self._select_downloads(
+                       count,
+                       False,
+                       indexset,
+                       lambda d: d.successful,
+                       "not successful yet"
+                 )

         # open them
         for d in matchs:
             d.open_file(cmdline)

-        # show errors if needed
-        if len(nomatchs):
-            message.error("Download{} {} {} not successful yet!".format(
-                "s" if len(nomatchs) > 1 else "",
-                utils.parse_list_into_numsettxt(nomatchs),
-                "are" if len(nomatchs) > 1 else "is"
-            ))
-
     @cmdutils.register(instance='download-model', scope='window')
     @cmdutils.argument('count', count=True)
     def download_retry(self, indexset=None, count=0):
@@ -1083,25 +1080,18 @@ def download_retry(self, indexset=None, count=0):
         """

         # find matched downloads
-        matchs, nomatchs = self._select_downloads(
-                                 count,
-                                 False,
-                                 indexset,
-                                 lambda d: d.done and not d.successful
-                           )
+        matchs = self._select_downloads(
+                       count,
+                       False,
+                       indexset,
+                       lambda d: d.done and not d.successful,
+                       "fine"
+                 )

         # retry them
         for d in matchs:
             d.try_retry()

-        # show errors if needed
-        if len(nomatchs):
-            message.error("Download{} {} {} fine!".format(
-                "s" if len(nomatchs) > 1 else "",
-                utils.parse_list_into_numsettxt(nomatchs),
-                "are" if len(nomatchs) > 1 else "is"
-            ))
-
     def can_clear(self):
         """Check if there are finished downloads to clear."""
         return any(download.done for download in self)
@@ -1130,25 +1120,18 @@ def download_remove(self, indexset=None, all_=False, count=0):
         """

         # find matche downloads 
-        matchs, nomatchs = self._select_downloads(
-                                 count,
-                                 all_,
-                                 indexset,
-                                 lambda d: d.done
-                           )
+        matchs = self._select_downloads(
+                       count,
+                       all_,
+                       indexset,
+                       lambda d: d.done,
+                       "in progress"
+                 )

         # remove them
         for d in matchs:
             d.remove()

-        # show errors if needed
-        if len(nomatchs):
-            message.error("Download{} {} {} in progress!".format(
-                "s" if len(nomatchs) > 1 else "",
-                utils.parse_list_into_numsettxt(nomatchs),
-                "are" if len(nomatchs) > 1 else "is"
-            ))
-
     def running_downloads(self):
         """Return the amount of still running downloads.

diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index 208e245f76..d5b564db73 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -864,59 +864,61 @@ def unused(_arg):
     pass

-def parse_numsettxt_into_list(txtset, nummax):
+def parse_numsettxt_into_numints(txtset, nummax):
     """Parse the given numbers set in text format, and return it as a list of
-    numbers.
+    number intervals [[start1, end2], [start2, end2], ..., [startn, endn]].

     Args:
         txtset: Numbers set specification. E.g. '1,5,8-last,32,2', where 'last'
-                is a special keyword that denotes that maximum allowable
-                number.
+                is a special keyword that denotes that largest number.
         nummax: The number that will be used to replace 'last'
     """
     txtset = txtset.lower().replace('last', str(nummax))
-    numbers = set()
+    numints = []
     for part in txtset.split(','):
         if '-' in part:
             start, end = part.split('-')
-            for i in range(int(start), int(end)+1):
-                numbers.add(i)
+            numints.append((int(start), int(end)))
         else:
-            numbers.add(int(part))
-    return sorted(numbers)
+            numints.append((int(part), int(part)))
+    return numints

-def parse_list_into_numsettxt(numbers):
-    """Summarise list of numbers into a numbers set description in text.
-    Basically undoes the effect of parse_numsettxt_into_list(...). For example,
-    l = [1,5,8,9,10,11,35] will be summarized into the string '1,5,8-11,35'.
+def which_nums_in_numints(nums, numints):
+    """Identify which of the numbers in nums fall within the intervals in
+    numints.

     Args:
-        numbers: Numbers list to be summarized back into text set descriptors. 
+        nums: A list of numbers.
+        numints: A list of number intervals [[start1, end2], [start2,
+                 end2], ..., [startn, endn]].
     """
-
-    state = 'start'
-    for i in sorted(numbers):
-        if state == 'start':
-            txtset = str(i)
-            state = 'inside'
-        elif state == 'inside':
-            diff = i - last_i
-            if diff > 1:
-                txtset += ',{}'.format(i)
+    nums = sorted(nums)
+    numints = sorted(numints)
+
+    # identify which of the numbers fall in the intervals
+    ins    = []
+    i, j = 0, 0
+    done = False
+    while done == False:
+        n = nums[i]
+        int_start = numints[j][0]
+        int_end = numints[j][1]
+        if n >= int_start and n <= int_end:
+            ins.append(n)
+            if i < (len(nums) - 1):
+                i += 1
             else:
-                txtset += '-'.format(i)
-                state = 'inside-dashed'
-        elif state == 'inside-dashed':
-            diff = i - last_i
-            if diff > 1:
-                txtset += '{},{}'.format(last_i,i)
-                state = 'inside'
-        last_i = i
-    if state == 'inside-dashed':
-        txtset += '{}'.format(i)
-
-    return txtset
+                done = True
+        elif n < int_start and i < (len(nums) - 1):
+            i += 1
+        elif n > int_end and j < (len(numints) - 1):
+            j += 1
+        else:
+            done = True
+
+    return ins
+

 def expand_windows_drive(path):
     r"""Expand a drive-path like E: into E:\.

From e03956fba499a31a0e73f961ed31c06896ee8e71 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Sat, 15 Apr 2017 14:03:49 +0400
Subject: [PATCH 13/17] removed excess newlines

---
 qutebrowser/utils/utils.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index d5b564db73..745f57aa3f 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -895,8 +895,6 @@ def which_nums_in_numints(nums, numints):
     """
     nums = sorted(nums)
     numints = sorted(numints)
-
-    # identify which of the numbers fall in the intervals
     ins    = []
     i, j = 0, 0
     done = False
@@ -916,7 +914,6 @@ def which_nums_in_numints(nums, numints):
             j += 1
         else:
             done = True
-
     return ins

From e3efa52a4e73decfaf674df5e7e38061f972fd86 Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Fri, 21 Apr 2017 22:19:02 +0400
Subject: [PATCH 14/17] trying to add unit tests that i added (fixes #2384)

my first attempt for adding unit tests for
which_nums_in_numints(..) and
parse_numsettxt_into_numints(..).

first time i do such things. am i on track? :-p
---
 qutebrowser/utils/utils.py     |  2 +-
 tests/unit/utils/test_utils.py | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index 745f57aa3f..9b9bb53876 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -895,7 +895,7 @@ def which_nums_in_numints(nums, numints):
     """
     nums = sorted(nums)
     numints = sorted(numints)
-    ins    = []
+    ins = []
     i, j = 0, 0
     done = False
     while done == False:
diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py
index d42f7a971d..c9322e640f 100644
--- a/tests/unit/utils/test_utils.py
+++ b/tests/unit/utils/test_utils.py
@@ -926,6 +926,21 @@ def test_system_default_application(self, caplog, config_stub, mocker):
 def test_unused():
     utils.unused(None)

+@pytest.mark.parametrize('txtset, nummax, expected', [
+    ('0-5,9-100,last', 500, [(0, 5), (9, 100), (500, 500)]),
+    ('0-5,9-100,last-3', 500, [(0, 5), (9, 100), (500, 3)]),
+    ('0-5,9-100,last-7000', 500, [(0, 5), (9, 100), (500, 7000)]),
+])
+def test_parse_numsettxt_into_numints(txtset, nummax, expected):
+    assert utils.parse_numsettxt_into_numints(txtset, nummax) == expected
+
+@pytest.mark.parametrize('nums, numints, expected', [
+    ([501], [(0, 5), (9, 100), (500, 500)], []),
+    ([3, 6, 50], [(0, 5), (9, 100), (500, 3)], [3, 50]),
+    ([501, 0], [(0, 5), (9, 100), (500, 7000)], [0, 501]),
+])
+def which_nums_in_numints(nums, numints, expected):
+    assert utils.which_nums_in_numints(nums, numints) == expected

 @pytest.mark.parametrize('path, expected', [
     ('E:', 'E:\\'),

From 511b96bcf98214c7531dcc865161683ccdc0ed7e Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Sun, 11 Jun 2017 01:59:37 +0400
Subject: [PATCH 15/17] added more tests  (fixes #2384) as per Kingdread's
 comments, except that i did not add tests for when the input is malformed. my
 only reasoning is that such cases will raise an exception, and i'm not quite
 sure how to write tests for that. i'll change this as soon as i get educated.

---
 tests/unit/utils/test_utils.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py
index e6a05c8b35..635c8f4eee 100644
--- a/tests/unit/utils/test_utils.py
+++ b/tests/unit/utils/test_utils.py
@@ -929,6 +929,8 @@ def test_system_default_application(self, caplog, config_stub, mocker):
     ('0-5,9-100,last', 500, [(0, 5), (9, 100), (500, 500)]),
     ('0-5,9-100,last-3', 500, [(0, 5), (9, 100), (500, 3)]),
     ('0-5,9-100,last-7000', 500, [(0, 5), (9, 100), (500, 7000)]),
+    ('6-9,3-5', 500, [(6, 9), (3, 5)]),
+    ('5-7,0-10', 500, [(5, 7), (0, 10)]),
 ])
 def test_parse_numsettxt_into_numints(txtset, nummax, expected):
     assert utils.parse_numsettxt_into_numints(txtset, nummax) == expected
@@ -938,8 +940,10 @@ def test_parse_numsettxt_into_numints(txtset, nummax, expected):
     ([501], [(0, 5), (9, 100), (500, 500)], []),
     ([3, 6, 50], [(0, 5), (9, 100), (500, 3)], [3, 50]),
     ([501, 0], [(0, 5), (9, 100), (500, 7000)], [0, 501]),
+    ([501, 0, 6, 9, 3, 5, 8, 4], [(6, 9), (3, 5)], [6, 9, 3, 5, 8, 4]),
+    ([501, 0, 5, 7, 10, 5, 8, 4], [(5, 7), (0, 10)], [0, 5, 7, 10, 5, 8, 4]),
 ])
-def which_nums_in_numints(nums, numints, expected):
+def test_which_nums_in_numints(nums, numints, expected):
     assert utils.which_nums_in_numints(nums, numints) == expected

From d1a7c39ccb5747d64cd13e3119df3859368a357f Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Sun, 11 Jun 2017 02:39:16 +0400
Subject: [PATCH 16/17] changed '0-last' into '1-last' as per Kingdread's
 comment (fixes #2384)

---
 qutebrowser/browser/downloads.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py
index 099a8726b2..26c55b6fbb 100644
--- a/qutebrowser/browser/downloads.py
+++ b/qutebrowser/browser/downloads.py
@@ -920,7 +920,7 @@ def _select_downloads(self, count, all_, indexset, criterion, errmsg):
         if all_ == True:
             indexset = '1-last'
             message.warning(
-                "'--all/-a' will be deprecated. Kindly use '0-last' instead."
+                "'--all/-a' will be deprecated. Kindly use '1-last' instead."
             )

         # support count

From 0fd78133a1e13c0b2f64dc06a660511dc0cc294f Mon Sep 17 00:00:00 2001
From: caveman <toraboracaveman@gmail.com>
Date: Sun, 11 Jun 2017 04:38:57 +0400
Subject: [PATCH 17/17] added tests for malformed input for
 parse_numsettxt_into_numints(...)

---
 tests/unit/utils/test_utils.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py
index 635c8f4eee..856f3f9115 100644
--- a/tests/unit/utils/test_utils.py
+++ b/tests/unit/utils/test_utils.py
@@ -931,9 +931,15 @@ def test_system_default_application(self, caplog, config_stub, mocker):
     ('0-5,9-100,last-7000', 500, [(0, 5), (9, 100), (500, 7000)]),
     ('6-9,3-5', 500, [(6, 9), (3, 5)]),
     ('5-7,0-10', 500, [(5, 7), (0, 10)]),
+    ('-5', 500, 'malformed'),
+    ('0--5', 500, 'malformed'),
+    ('foo-bar', 500, 'malformed'),
 ])
 def test_parse_numsettxt_into_numints(txtset, nummax, expected):
-    assert utils.parse_numsettxt_into_numints(txtset, nummax) == expected
+    try:
+        assert utils.parse_numsettxt_into_numints(txtset, nummax) == expected
+    except ValueError:
+        assert expected == 'malformed'

 @pytest.mark.parametrize('nums, numints, expected', [
al-caveman commented 7 years ago

Just to document the latest code that addresses all requested changes (as far as I know), hopefully for later use (perhaps when the exams season is over, or maybe after the new configs format for qutebrowser 2.0 is out).

(ty @The-Compiler and @Kingdread for mentoring me, and good luck with your exams).

utils.py changes

class NumberSet:

    """Fancy number set class that elements of its objects are defined in text
    in some convenient notation, and such objects supports 'in' operator.
    Attributes:
        text: set representation in text. E.g. '1,5,8-last,32,2'.
        nummax: value to represent 'last'
    """

    def __init__(self, text, nummax):
        self.text = text.lower().replace('last', str(nummax))
        self.__parse_text_into_ranges()

    def __contains__(self, other):
        return any(other in r for r in self.ranges)

    def __parse_text_into_ranges(self):
        self.ranges = []
        for part in self.text.split(','):
            if '-' in part:
                start, end = part.split('-')
                self.ranges.append(range(int(start), int(end) + 1))
            else:
                self.ranges.append(range(int(part), int(part) + 1))

downloads.py changes

As done here. Known bugs:

test_utils.py changes

@pytest.mark.parametrize('txtset, nummax, expected', [
    ('5-0', 10, []),
    ('5-0,1-5', 10, [1,2,3,4,5]),
    ('5-0,1-5,last', 10, [1,2,3,4,5,10]),
    ('5-0,1,1-5,last', 10, [1,2,3,4,5,10]),
    ('last,5-0,1-5,last', 10, [1,2,3,4,5,10]),
    ('-5-0', 10, ValueError),
    ('-5-0,1-5', 10, ValueError),
    ('5-0,-1-5', 10, ValueError),
    ('5--0,1-5', 10, ValueError),
    ('5-0,-1-5,last', 10, ValueError),
    ('5-0,,1-5', 10, ValueError),
    ('5-0,1-5,,last', 10, ValueError),
    ('5-,0,1-5', 10, ValueError)
])
def test_NumberSet(txtset, nummax, expected):
    if expected is ValueError:
        with pytest.raises(ValueError):
            utils.NumberSet(txtset, nummax)
    else:
        numset = utils.NumberSet(txtset, nummax)
        assert [i for i in range(0, nummax + 1) if i in numset] == expected
The-Compiler commented 6 years ago

Somewhat relevant: https://nedbatchelder.com//blog/201802/a_python_gargoyle.html