Merge branch 'mlochbaum-bookmark-add-toggle'

CHANGELOG.asciidoc

@@ -14,6 +14,15 @@ This project adheres to http://semver.org/[Semantic Versioning].
 // `Fixed` for any bug fixes.
 // `Security` to invite users to upgrade in case of vulnerabilities.
 
+v0.9.0
+------
+
+Changed
+~~~~~~~
+
+- `:bookmark-add` now has a `--toggle` flag which deletes the bookmark if it
+  already exists.
+
 v0.8.0
 ------
 
@@ -33,7 +42,7 @@ Added
   `$QUTE_DOWNLOAD_DIR` available for userscripts.
 - New option `ui` -> `status-position` to configure the position of the
   status bar (top/bottom).
-- New `--pdf <filename>` argument for `:print` which can be used to generate a
+- New `--pdf <filename>` argument for `:print` WHICH can be used to generate a
   PDF without a dialog.
 
 Changed

README.asciidoc

@@ -164,8 +164,8 @@ Contributors, sorted by the number of commits in descending order:
 * Thorsten Wißmann
 * Kevin Velghe
 * Austin Anderson
-* Jimmy
 * Marshall Lochbaum
+* Jimmy
 * Alexey "Averrin" Nabrodov
 * avk
 * ZDarian

doc/help/commands.asciidoc

@@ -116,7 +116,7 @@ Bind a key to a command.
 
 [[bookmark-add]]
 === bookmark-add
-Syntax: +:bookmark-add ['url'] ['title']+
+Syntax: +:bookmark-add [*--toggle*] ['url'] ['title']+
 
 Save the current page as a bookmark, or a specific url.
 
@@ -126,6 +126,10 @@ If no url and title are provided, then save the current page as a bookmark. If a
 * +'url'+: url to save as a bookmark. If None, use url of current page.
 * +'title'+: title of the new bookmark.
 
+==== optional arguments
+* +*-t*+, +*--toggle*+: remove the bookmark instead of raising an error if it already exists.
+
+
 [[bookmark-del]]
 === bookmark-del
 Syntax: +:bookmark-del ['url']+

qutebrowser/browser/commands.py

@@ -1120,7 +1120,7 @@ class CommandDispatcher:
             raise cmdexc.CommandError("Quickmark '{}' not found!".format(name))
 
     @cmdutils.register(instance='command-dispatcher', scope='window')
-    def bookmark_add(self, url=None, title=None):
+    def bookmark_add(self, url=None, title=None, toggle=False):
         """Save the current page as a bookmark, or a specific url.
 
         If no url and title are provided, then save the current page as a
@@ -1134,6 +1134,8 @@ class CommandDispatcher:
         Args:
             url: url to save as a bookmark. If None, use url of current page.
             title: title of the new bookmark.
+            toggle: remove the bookmark instead of raising an error if it
+                    already exists.
         """
         if url and not title:
             raise cmdexc.CommandError('Title must be provided if url has '
@@ -1149,12 +1151,13 @@ class CommandDispatcher:
         if not title:
             title = self._current_title()
         try:
-            bookmark_manager.add(url, title)
+            was_added = bookmark_manager.add(url, title, toggle=toggle)
         except urlmarks.Error as e:
             raise cmdexc.CommandError(str(e))
         else:
+            msg = "Bookmarked {}!" if was_added else "Removed bookmark {}!"
             message.info(self._win_id,
-                         "Bookmarked {}!".format(url.toDisplayString()))
+                         msg.format(url.toDisplayString()))
 
     @cmdutils.register(instance='command-dispatcher', scope='window',
                        maxsplit=0)

qutebrowser/browser/urlmarks.py

@@ -272,12 +272,18 @@ class BookmarkManager(UrlMarkManager):
         elif len(parts) == 1:
             self.marks[parts[0]] = ''
 
-    def add(self, url, title):
+    def add(self, url, title, *, toggle=False):
         """Add a new bookmark.
 
         Args:
             url: The url to add as bookmark.
             title: The title for the new bookmark.
+            toggle: remove the bookmark instead of raising an error if it
+                    already exists.
+
+        Return:
+            True if the bookmark was added, and False if it was
+            removed (only possible if toggle is True).
         """
         if not url.isValid():
             errstr = urlutils.get_errstring(url)
@@ -286,8 +292,13 @@ class BookmarkManager(UrlMarkManager):
         urlstr = url.toString(QUrl.RemovePassword | QUrl.FullyEncoded)
 
         if urlstr in self.marks:
+            if toggle:
+                del self.marks[urlstr]
+                return False
+            else:
                 raise AlreadyExistsError("Bookmark already exists!")
         else:
             self.marks[urlstr] = title
             self.changed.emit()
             self.added.emit(title, urlstr)
+            return True

tests/end2end/data/numbers/18.txt

@@ -0,0 +1 @@
+eighteen

tests/end2end/features/urlmarks.feature

@@ -97,33 +97,39 @@ Feature: quickmarks and bookmarks
         And I run :bookmark-del
         Then the bookmark file should not contain "http://localhost:*/data/numbers/6.txt "
 
+    Scenario: Toggling a bookmark
+        When I open data/numbers/7.txt
+        And I run :bookmark-add
+        And I run :bookmark-add --toggle
+        Then the bookmark file should not contain "http://localhost:*/data/numbers/7.txt "
+
     ## quickmarks
 
     Scenario: Saving a quickmark (:quickmark-add)
-        When I run :quickmark-add http://localhost:(port)/data/numbers/7.txt seven
+        When I run :quickmark-add http://localhost:(port)/data/numbers/8.txt eight
-        Then the quickmark file should contain "seven http://localhost:*/data/numbers/7.txt"
-
-    Scenario: Saving a quickmark (:quickmark-save)
-        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 "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 (without override)
+    Scenario: Saving a quickmark (:quickmark-save)
-        When I run :quickmark-add http://localhost:(port)/data/numbers/9.txt nine
+        When I open data/numbers/9.txt
-        And I run :quickmark-add http://localhost:(port)/data/numbers/9_2.txt nine
+        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 "nine"
+        And I press the keys "<Enter>"
         Then the quickmark file should contain "nine http://localhost:*/data/numbers/9.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/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-no
+        Then the quickmark file should contain "ten http://localhost:*/data/numbers/10.txt"
+
+    Scenario: Saving a duplicate quickmark (with override)
+        When I run :quickmark-add http://localhost:(port)/data/numbers/11.txt eleven
+        And I run :quickmark-add http://localhost:(port)/data/numbers/11_2.txt eleven
+        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 "ten http://localhost:*/data/numbers/10_2.txt"
+        Then the quickmark file should contain "eleven http://localhost:*/data/numbers/11_2.txt"
 
     Scenario: Adding a quickmark with an empty name
         When I run :quickmark-add about:blank ""
@@ -135,38 +141,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/11.txt eleven
+        When I run :quickmark-add http://localhost:(port)/data/numbers/12.txt twelve
-        And I run :quickmark-load eleven
+        And I run :quickmark-load twelve
-        Then data/numbers/11.txt should be loaded
+        Then data/numbers/12.txt should be loaded
         And the following tabs should be open:
-            - data/numbers/11.txt (active)
+            - data/numbers/12.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/12.txt twelve
+        And I run :quickmark-add http://localhost:(port)/data/numbers/13.txt thirteen
-        And I run :quickmark-load -t twelve
+        And I run :quickmark-load -t 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
-            - data/numbers/12.txt (active)
+            - data/numbers/13.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/13.txt thirteen
+        And I run :quickmark-add http://localhost:(port)/data/numbers/14.txt fourteen
-        And I run :quickmark-load -b thirteen
+        And I run :quickmark-load -b fourteen
-        Then data/numbers/13.txt should be loaded
+        Then data/numbers/14.txt should be loaded
         And the following tabs should be open:
             - about:blank (active)
-            - data/numbers/13.txt
+            - data/numbers/14.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/14.txt fourteen
+        And I run :quickmark-add http://localhost:(port)/data/numbers/15.txt fifteen
-        And I run :quickmark-load -w fourteen
+        And I run :quickmark-load -w fifteen
-        And I wait until data/numbers/14.txt is loaded
+        And I wait until data/numbers/15.txt is loaded
         Then the session should look like:
             windows:
             - tabs:
@@ -178,15 +184,15 @@ Feature: quickmarks and bookmarks
               - active: true
                 history:
                 - active: true
-                  url: http://localhost:*/data/numbers/14.txt
+                  url: http://localhost:*/data/numbers/15.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/15.txt fifteen
+        When I run :quickmark-add http://localhost:(port)/data/numbers/16.txt sixteen
-        When I run :quickmark-load -t -b fifteen
+        When I run :quickmark-load -t -b sixteen
         Then the error "Only one of -t/-b/-w can be given!" should be shown
 
     Scenario: Deleting a quickmark which does not exist
@@ -194,9 +200,9 @@ 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/16.txt sixteen
+        When I run :quickmark-add http://localhost:(port)/data/numbers/17.txt seventeen
-        And I run :quickmark-del sixteen
+        And I run :quickmark-del seventeen
-        Then the quickmark file should not contain "sixteen http://localhost:*/data/numbers/16.txt "
+        Then the quickmark file should not contain "seventeen http://localhost:*/data/numbers/17.txt "
 
     Scenario: Deleting the current page's quickmark if it has none
         When I open about:blank
@@ -204,10 +210,10 @@ Feature: quickmarks and bookmarks
         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
+        When I open data/numbers/18.txt
-        And I run :quickmark-add http://localhost:(port)/data/numbers/17.txt seventeen
+        And I run :quickmark-add http://localhost:(port)/data/numbers/18.txt eighteen
         And I run :quickmark-del
-        Then the quickmark file should not contain "seventeen http://localhost:*/data/numbers/17.txt"
+        Then the quickmark file should not contain "eighteen http://localhost:*/data/numbers/18.txt"
 
     Scenario: Listing quickmarks
         When I run :quickmark-add http://localhost:(port)/data/numbers/15.txt fifteen