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

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

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

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.

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

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.utils import (message, usertypes, qtutils, urlutils,
-from qutebrowser.commands import cmdexc, cmdutils
+                               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)
+    def get_by_qurl(self, url):
-    @cmdutils.argument('name',
+        """Look up a quickmark by QUrl, returning its name.
-                       completion=usertypes.Completion.quickmark_by_name)
-    def quickmark_del(self, name):
-        """Delete a quickmark.
 
-        Args:
+        Takes O(n) time, where n is the number of quickmarks.
-            name: The name of the quickmark to delete.
+        Use a name instead where possible.
         """
+        qtutils.ensure_valid(url)
+        urlstr = url.toString(QUrl.RemovePassword | QUrl.FullyEncoded)
+
         try:
-            self.delete(name)
+            index = list(self.marks.values()).index(urlstr)
-        except KeyError:
+            key = list(self.marks.keys())[index]
-            raise cmdexc.CommandError("Quickmark '{}' not found!".format(name))
+        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))

tests/end2end/data/numbers/15.txt

@@ -0,0 +1 @@
+fifteen

tests/end2end/data/numbers/16.txt

@@ -0,0 +1 @@
+sixteen

tests/end2end/data/numbers/17.txt

@@ -0,0 +1 @@
+seventeen

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
+        When I run :quickmark-add http://localhost:(port)/data/numbers/7.txt seven
-        Then the quickmark file should contain "six http://localhost:*/data/numbers/6.txt"
-
-    Scenario: Saving a quickmark (:quickmark-save)
-        When I open data/numbers/7.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 "<Enter>"
         Then the quickmark file should contain "seven http://localhost:*/data/numbers/7.txt"
 
-    Scenario: Saving a duplicate quickmark (without override)
+    Scenario: Saving a quickmark (:quickmark-save)
-        When I run :quickmark-add http://localhost:(port)/data/numbers/8.txt eight
+        When I open data/numbers/8.txt
-        And I run :quickmark-add http://localhost:(port)/data/numbers/8_2.txt eight
+        And I run :quickmark-save
-        And I wait for "Entering mode KeyMode.yesno (reason: question asked)" in the log
+        And I wait for "Entering mode KeyMode.prompt (reason: question asked)" in the log
-        And I run :prompt-no
+        And I press the keys "eight"
+        And I press the keys "<Enter>"
         Then the quickmark file should contain "eight http://localhost:*/data/numbers/8.txt"
 
-    Scenario: Saving a duplicate quickmark (with override)
+    Scenario: Saving a duplicate quickmark (without 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
         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 "nine http://localhost:*/data/numbers/9.txt"
+
+    Scenario: Saving a duplicate quickmark (with override)
+        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
+        When I run :quickmark-add http://localhost:(port)/data/numbers/11.txt eleven
-        And I run :quickmark-load ten
+        And I run :quickmark-load eleven
-        Then data/numbers/10.txt should be loaded
+        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-add http://localhost:(port)/data/numbers/12.txt twelve
-        And I run :quickmark-load -t eleven
+        And I run :quickmark-load -t twelve
-        Then data/numbers/11.txt should be loaded
+        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-add http://localhost:(port)/data/numbers/13.txt thirteen
-        And I run :quickmark-load -b twelve
+        And I run :quickmark-load -b thirteen
-        Then data/numbers/12.txt should be loaded
+        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-add http://localhost:(port)/data/numbers/14.txt fourteen
-        And I run :quickmark-load -w thirteen
+        And I run :quickmark-load -w fourteen
-        And I wait until data/numbers/13.txt is loaded
+        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-add http://localhost:(port)/data/numbers/15.txt fifteen
-        When I run :quickmark-load -t -b fourteen
+        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
+        When I run :quickmark-add http://localhost:(port)/data/numbers/16.txt sixteen
-        And I run :quickmark-del fifteen
+        And I run :quickmark-del sixteen
-        Then the quickmark file should not contain "fourteen http://localhost:*/data/numbers/15.txt "
+        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