Merge branch 'mlochbaum-mark-del-no-args'

9 files changed, 126 insertions, 61 deletions

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 6ebc25e6d..f0bd07795 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -44,6 +44,8 @@ Changed
   rather than the current page.
 - New `taskadd` userscript to add a taskwarrior task annotated with the
   current URL.
+- `:bookmark-del` and `:quickmark-del` now delete the current page's URL if none
+  is given.
 
 Fixed
 -----
diff --git a/README.asciidoc b/README.asciidoc
index acf7fbc09..422b87db8 100644
--- a/README.asciidoc
+++ b/README.asciidoc
@@ -165,6 +165,7 @@ Contributors, sorted by the number of commits in descending order:
 * Kevin Velghe
 * Austin Anderson
 * Jimmy
+* Marshall Lochbaum
 * Alexey "Averrin" Nabrodov
 * avk
 * ZDarian
diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc
index 713b75d60..eea3f0d7f 100644
--- a/doc/help/commands.asciidoc
+++ b/doc/help/commands.asciidoc
@@ -128,12 +128,13 @@ If no url and title are provided, then save the current page as a bookmark. If a
 
 [[bookmark-del]]
 === bookmark-del
-Syntax: +:bookmark-del 'url'+
+Syntax: +:bookmark-del ['url']+
 
 Delete a bookmark.
 
 ==== positional arguments
-* +'url'+: The URL of the bookmark to delete.
+* +'url'+: The url of the bookmark to delete. If not given, use the current page's url.
+
 
 ==== note
 * This command does not split arguments after the last argument and handles quotes literally.
@@ -520,12 +521,14 @@ You can view all saved quickmarks on the link:qute://bookmarks[bookmarks page].
 
 [[quickmark-del]]
 === quickmark-del
-Syntax: +:quickmark-del 'name'+
+Syntax: +:quickmark-del ['name']+
 
 Delete a quickmark.
 
 ==== positional arguments
-* +'name'+: The name of the quickmark to delete.
+* +'name'+: The name of the quickmark to delete. If not given, delete the quickmark for the current page (choosing one arbitrarily
+ if there are more than one).
+
 
 ==== note
 * This command does not split arguments after the last argument and handles quotes literally.
diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py
index 42b91f737..49eaa5dcb 100644
--- a/qutebrowser/browser/commands.py
+++ b/qutebrowser/browser/commands.py
@@ -1095,6 +1095,30 @@ class CommandDispatcher:
             raise cmdexc.CommandError(str(e))
         self._open(url, tab, bg, window)
 
+    @cmdutils.register(instance='command-dispatcher', scope='window',
+                       maxsplit=0)
+    @cmdutils.argument('name',
+                       completion=usertypes.Completion.quickmark_by_name)
+    def quickmark_del(self, name=None):
+        """Delete a quickmark.
+
+        Args:
+            name: The name of the quickmark to delete. If not given, delete the
+                  quickmark for the current page (choosing one arbitrarily
+                  if there are more than one).
+        """
+        quickmark_manager = objreg.get('quickmark-manager')
+        if name is None:
+            url = self._current_url()
+            try:
+                name = quickmark_manager.get_by_qurl(url)
+            except urlmarks.DoesNotExistError as e:
+                raise cmdexc.CommandError(str(e))
+        try:
+            quickmark_manager.delete(name)
+        except KeyError:
+            raise cmdexc.CommandError("Quickmark '{}' not found!".format(name))
+
     @cmdutils.register(instance='command-dispatcher', scope='window')
     def bookmark_add(self, url=None, title=None):
         """Save the current page as a bookmark, or a specific url.
@@ -1150,6 +1174,25 @@ class CommandDispatcher:
             raise cmdexc.CommandError(e)
         self._open(url, tab, bg, window)
 
+    @cmdutils.register(instance='command-dispatcher', scope='window',
+                       maxsplit=0)
+    @cmdutils.argument('url', completion=usertypes.Completion.bookmark_by_url)
+    def bookmark_del(self, url=None):
+        """Delete a bookmark.
+
+        Args:
+            url: The url of the bookmark to delete. If not given, use the
+                 current page's url.
+        """
+        if url is None:
+            url = self._current_url().toString(QUrl.RemovePassword
+                                             | QUrl.FullyEncoded)
+        try:
+            objreg.get('bookmark-manager').delete(url)
+        except KeyError:
+            raise cmdexc.CommandError("Bookmark '{}' not found!".format(url))
+
+
     @cmdutils.register(instance='command-dispatcher', hide=True,
                        scope='window')
     def follow_selected(self, *, tab=False):
diff --git a/qutebrowser/browser/urlmarks.py b/qutebrowser/browser/urlmarks.py
index e7b78c310..89bec3d9a 100644
--- a/qutebrowser/browser/urlmarks.py
+++ b/qutebrowser/browser/urlmarks.py
@@ -32,8 +32,9 @@ import collections
 
 from PyQt5.QtCore import pyqtSignal, QUrl, QObject
 
-from qutebrowser.utils import message, usertypes, urlutils, standarddir, objreg
-from qutebrowser.commands import cmdexc, cmdutils
+from qutebrowser.utils import (message, usertypes, qtutils, urlutils,
+                               standarddir, objreg)
+from qutebrowser.commands import cmdutils
 from qutebrowser.misc import lineparser
 
 
@@ -207,19 +208,22 @@ class QuickmarkManager(UrlMarkManager):
         else:
             set_mark()
 
-    @cmdutils.register(instance='quickmark-manager', maxsplit=0)
-    @cmdutils.argument('name',
-                       completion=usertypes.Completion.quickmark_by_name)
-    def quickmark_del(self, name):
-        """Delete a quickmark.
+    def get_by_qurl(self, url):
+        """Look up a quickmark by QUrl, returning its name.
 
-        Args:
-            name: The name of the quickmark to delete.
+        Takes O(n) time, where n is the number of quickmarks.
+        Use a name instead where possible.
         """
+        qtutils.ensure_valid(url)
+        urlstr = url.toString(QUrl.RemovePassword | QUrl.FullyEncoded)
+
         try:
-            self.delete(name)
-        except KeyError:
-            raise cmdexc.CommandError("Quickmark '{}' not found!".format(name))
+            index = list(self.marks.values()).index(urlstr)
+            key = list(self.marks.keys())[index]
+        except ValueError:
+            raise DoesNotExistError(
+                "Quickmark for '{}' not found!".format(urlstr))
+        return key
 
     def get(self, name):
         """Get the URL of the quickmark named name as a QUrl."""
@@ -287,16 +291,3 @@ class BookmarkManager(UrlMarkManager):
             self.marks[urlstr] = title
             self.changed.emit()
             self.added.emit(title, urlstr)
-
-    @cmdutils.register(instance='bookmark-manager', maxsplit=0)
-    @cmdutils.argument('url', completion=usertypes.Completion.bookmark_by_url)
-    def bookmark_del(self, url):
-        """Delete a bookmark.
-
-        Args:
-            url: The URL of the bookmark to delete.
-        """
-        try:
-            self.delete(url)
-        except KeyError:
-            raise cmdexc.CommandError("Bookmark '{}' not found!".format(url))
diff --git a/tests/end2end/data/numbers/15.txt b/tests/end2end/data/numbers/15.txt
new file mode 100644
index 000000000..e228c03ca
--- /dev/null
+++ b/tests/end2end/data/numbers/15.txt
@@ -0,0 +1 @@
+fifteen
diff --git a/tests/end2end/data/numbers/16.txt b/tests/end2end/data/numbers/16.txt
new file mode 100644
index 000000000..64b765f18
--- /dev/null
+++ b/tests/end2end/data/numbers/16.txt
@@ -0,0 +1 @@
+sixteen
diff --git a/tests/end2end/data/numbers/17.txt b/tests/end2end/data/numbers/17.txt
new file mode 100644
index 000000000..5615fadf4
--- /dev/null
+++ b/tests/end2end/data/numbers/17.txt
@@ -0,0 +1 @@
+seventeen
diff --git a/tests/end2end/features/urlmarks.feature b/tests/end2end/features/urlmarks.feature
index 010d87f90..3490cc4a0 100644
--- a/tests/end2end/features/urlmarks.feature
+++ b/tests/end2end/features/urlmarks.feature
@@ -86,33 +86,44 @@ Feature: quickmarks and bookmarks
         And I run :bookmark-del http://localhost:(port)/data/numbers/5.txt
         Then the bookmark file should not contain "http://localhost:*/data/numbers/5.txt "
 
+    Scenario: Deleting the current page's bookmark if it doesn't exist
+        When I open about:blank
+        And I run :bookmark-del
+        Then the error "Bookmark 'about:blank' not found!" should be shown
+
+    Scenario: Deleting the current page's bookmark
+        When I open data/numbers/6.txt
+        And I run :bookmark-add
+        And I run :bookmark-del
+        Then the bookmark file should not contain "http://localhost:*/data/numbers/6.txt "
+
     ## quickmarks
 
     Scenario: Saving a quickmark (:quickmark-add)
-        When I run :quickmark-add http://localhost:(port)/data/numbers/6.txt six
-        Then the quickmark file should contain "six http://localhost:*/data/numbers/6.txt"
+        When I run :quickmark-add http://localhost:(port)/data/numbers/7.txt seven
+        Then the quickmark file should contain "seven http://localhost:*/data/numbers/7.txt"
 
     Scenario: Saving a quickmark (:quickmark-save)
-        When I open data/numbers/7.txt
+        When I open data/numbers/8.txt
         And I run :quickmark-save
         And I wait for "Entering mode KeyMode.prompt (reason: question asked)" in the log
-        And I press the keys "seven"
+        And I press the keys "eight"
         And I press the keys "<Enter>"
-        Then the quickmark file should contain "seven http://localhost:*/data/numbers/7.txt"
+        Then the quickmark file should contain "eight http://localhost:*/data/numbers/8.txt"
 
     Scenario: Saving a duplicate quickmark (without override)
-        When I run :quickmark-add http://localhost:(port)/data/numbers/8.txt eight
-        And I run :quickmark-add http://localhost:(port)/data/numbers/8_2.txt eight
+        When I run :quickmark-add http://localhost:(port)/data/numbers/9.txt nine
+        And I run :quickmark-add http://localhost:(port)/data/numbers/9_2.txt nine
         And I wait for "Entering mode KeyMode.yesno (reason: question asked)" in the log
         And I run :prompt-no
-        Then the quickmark file should contain "eight http://localhost:*/data/numbers/8.txt"
+        Then the quickmark file should contain "nine http://localhost:*/data/numbers/9.txt"
 
     Scenario: Saving a duplicate quickmark (with override)
-        When I run :quickmark-add http://localhost:(port)/data/numbers/9.txt nine
-        And I run :quickmark-add http://localhost:(port)/data/numbers/9_2.txt nine
+        When I run :quickmark-add http://localhost:(port)/data/numbers/10.txt ten
+        And I run :quickmark-add http://localhost:(port)/data/numbers/10_2.txt ten
         And I wait for "Entering mode KeyMode.yesno (reason: question asked)" in the log
         And I run :prompt-yes
-        Then the quickmark file should contain "nine http://localhost:*/data/numbers/9_2.txt"
+        Then the quickmark file should contain "ten http://localhost:*/data/numbers/10_2.txt"
 
     Scenario: Adding a quickmark with an empty name
         When I run :quickmark-add about:blank ""
@@ -124,38 +135,38 @@ Feature: quickmarks and bookmarks
 
     Scenario: Loading a quickmark
         Given I have a fresh instance
-        When I run :quickmark-add http://localhost:(port)/data/numbers/10.txt ten
-        And I run :quickmark-load ten
-        Then data/numbers/10.txt should be loaded
+        When I run :quickmark-add http://localhost:(port)/data/numbers/11.txt eleven
+        And I run :quickmark-load eleven
+        Then data/numbers/11.txt should be loaded
         And the following tabs should be open:
-            - data/numbers/10.txt (active)
+            - data/numbers/11.txt (active)
 
     Scenario: Loading a quickmark in a new tab
         Given I open about:blank
         When I run :tab-only
-        And I run :quickmark-add http://localhost:(port)/data/numbers/11.txt eleven
-        And I run :quickmark-load -t eleven
-        Then data/numbers/11.txt should be loaded
+        And I run :quickmark-add http://localhost:(port)/data/numbers/12.txt twelve
+        And I run :quickmark-load -t twelve
+        Then data/numbers/12.txt should be loaded
         And the following tabs should be open:
             - about:blank
-            - data/numbers/11.txt (active)
+            - data/numbers/12.txt (active)
 
     Scenario: Loading a quickmark in a background tab
         Given I open about:blank
         When I run :tab-only
-        And I run :quickmark-add http://localhost:(port)/data/numbers/12.txt twelve
-        And I run :quickmark-load -b twelve
-        Then data/numbers/12.txt should be loaded
+        And I run :quickmark-add http://localhost:(port)/data/numbers/13.txt thirteen
+        And I run :quickmark-load -b thirteen
+        Then data/numbers/13.txt should be loaded
         And the following tabs should be open:
             - about:blank (active)
-            - data/numbers/12.txt
+            - data/numbers/13.txt
 
     Scenario: Loading a quickmark in a new window
         Given I open about:blank
         When I run :tab-only
-        And I run :quickmark-add http://localhost:(port)/data/numbers/13.txt thirteen
-        And I run :quickmark-load -w thirteen
-        And I wait until data/numbers/13.txt is loaded
+        And I run :quickmark-add http://localhost:(port)/data/numbers/14.txt fourteen
+        And I run :quickmark-load -w fourteen
+        And I wait until data/numbers/14.txt is loaded
         Then the session should look like:
             windows:
             - tabs:
@@ -167,15 +178,15 @@ Feature: quickmarks and bookmarks
               - active: true
                 history:
                 - active: true
-                  url: http://localhost:*/data/numbers/13.txt
+                  url: http://localhost:*/data/numbers/14.txt
 
     Scenario: Loading a quickmark which does not exist
         When I run :quickmark-load -b doesnotexist
         Then the error "Quickmark 'doesnotexist' does not exist!" should be shown
 
     Scenario: Loading a quickmark with -t and -b
-        When I run :quickmark-add http://localhost:(port)/data/numbers/14.txt fourteen
-        When I run :quickmark-load -t -b fourteen
+        When I run :quickmark-add http://localhost:(port)/data/numbers/15.txt fifteen
+        When I run :quickmark-load -t -b fifteen
         Then the error "Only one of -t/-b/-w can be given!" should be shown
 
     Scenario: Deleting a quickmark which does not exist
@@ -183,9 +194,20 @@ Feature: quickmarks and bookmarks
         Then the error "Quickmark 'doesnotexist' not found!" should be shown
 
     Scenario: Deleting a quickmark
-        When I run :quickmark-add http://localhost:(port)/data/numbers/15.txt fifteen
-        And I run :quickmark-del fifteen
-        Then the quickmark file should not contain "fourteen http://localhost:*/data/numbers/15.txt "
+        When I run :quickmark-add http://localhost:(port)/data/numbers/16.txt sixteen
+        And I run :quickmark-del sixteen
+        Then the quickmark file should not contain "sixteen http://localhost:*/data/numbers/16.txt "
+
+    Scenario: Deleting the current page's quickmark if it has none
+        When I open about:blank
+        And I run :quickmark-del
+        Then the error "Quickmark for 'about:blank' not found!" should be shown
+
+    Scenario: Deleting the current page's quickmark
+        When I open data/numbers/17.txt
+        And I run :quickmark-add http://localhost:(port)/data/numbers/17.txt seventeen
+        And I run :quickmark-del
+        Then the quickmark file should not contain "seventeen http://localhost:*/data/numbers/17.txt"
 
     Scenario: Listing quickmarks
         When I run :quickmark-add http://localhost:(port)/data/numbers/15.txt fifteen