From 28842c90b606eb0300add8cbd2a03cd4ceb8349b Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Fri, 22 Jul 2016 22:46:30 -0400 Subject: [PATCH 1/4] Add --toggle flag to bookmark-add (fixes #1667) --- qutebrowser/browser/commands.py | 10 +++++++--- qutebrowser/browser/urlmarks.py | 14 ++++++++++++-- tests/end2end/features/urlmarks.feature | 6 ++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 7838e0138..f86faa0da 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1120,7 +1120,8 @@ 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): + @cmdutils.argument('toggle', flag='t') + 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 +1135,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 +1152,13 @@ class CommandDispatcher: if not title: title = self._current_title() try: - bookmark_manager.add(url, title) + if_added = bookmark_manager.add(url, title, toggle) except urlmarks.Error as e: raise cmdexc.CommandError(str(e)) else: + mes = "Bookmarked {}!" if if_added else "Removed bookmark {}!" message.info(self._win_id, - "Bookmarked {}!".format(url.toDisplayString())) + mes.format(url.toDisplayString())) @cmdutils.register(instance='command-dispatcher', scope='window', maxsplit=0) diff --git a/qutebrowser/browser/urlmarks.py b/qutebrowser/browser/urlmarks.py index 89bec3d9a..9889c8536 100644 --- a/qutebrowser/browser/urlmarks.py +++ b/qutebrowser/browser/urlmarks.py @@ -272,12 +272,17 @@ 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. + Return True if the bookmark was added, and False if it was + removed (which only happens if toggle is True). + 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. """ if not url.isValid(): errstr = urlutils.get_errstring(url) @@ -286,8 +291,13 @@ class BookmarkManager(UrlMarkManager): urlstr = url.toString(QUrl.RemovePassword | QUrl.FullyEncoded) if urlstr in self.marks: - raise AlreadyExistsError("Bookmark already exists!") + 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 diff --git a/tests/end2end/features/urlmarks.feature b/tests/end2end/features/urlmarks.feature index 3490cc4a0..383cb492a 100644 --- a/tests/end2end/features/urlmarks.feature +++ b/tests/end2end/features/urlmarks.feature @@ -97,6 +97,12 @@ 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) From ae7fe2ee33f466f592ae42e0de91e46a5ecb8dcb Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Tue, 26 Jul 2016 09:14:51 -0400 Subject: [PATCH 2/4] Update number files --- tests/end2end/data/numbers/18.txt | 1 + tests/end2end/features/urlmarks.feature | 80 ++++++++++++------------- 2 files changed, 41 insertions(+), 40 deletions(-) create mode 100644 tests/end2end/data/numbers/18.txt diff --git a/tests/end2end/data/numbers/18.txt b/tests/end2end/data/numbers/18.txt new file mode 100644 index 000000000..63affa473 --- /dev/null +++ b/tests/end2end/data/numbers/18.txt @@ -0,0 +1 @@ +eighteen diff --git a/tests/end2end/features/urlmarks.feature b/tests/end2end/features/urlmarks.feature index 383cb492a..4054e3f15 100644 --- a/tests/end2end/features/urlmarks.feature +++ b/tests/end2end/features/urlmarks.feature @@ -106,30 +106,30 @@ Feature: quickmarks and bookmarks ## quickmarks Scenario: Saving a quickmark (:quickmark-add) - 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/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 "" + When I run :quickmark-add http://localhost:(port)/data/numbers/8.txt eight 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/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 + Scenario: Saving a quickmark (:quickmark-save) + When I open data/numbers/9.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 "nine" + And I press the keys "" 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 "" @@ -141,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 - And I run :quickmark-load eleven - Then data/numbers/11.txt should be loaded + When I run :quickmark-add http://localhost:(port)/data/numbers/12.txt twelve + And I run :quickmark-load twelve + 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-load -t 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 -t thirteen + 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-load -b thirteen - Then data/numbers/13.txt should be loaded + And I run :quickmark-add http://localhost:(port)/data/numbers/14.txt fourteen + And I run :quickmark-load -b fourteen + 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-load -w fourteen - And I wait until data/numbers/14.txt is loaded + And I run :quickmark-add http://localhost:(port)/data/numbers/15.txt fifteen + And I run :quickmark-load -w fifteen + And I wait until data/numbers/15.txt is loaded Then the session should look like: windows: - tabs: @@ -184,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-load -t -b fifteen + When I run :quickmark-add http://localhost:(port)/data/numbers/16.txt sixteen + 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 @@ -200,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 - And I run :quickmark-del sixteen - Then the quickmark file should not contain "sixteen http://localhost:*/data/numbers/16.txt " + When I run :quickmark-add http://localhost:(port)/data/numbers/17.txt seventeen + And I run :quickmark-del seventeen + 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 @@ -210,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 - And I run :quickmark-add http://localhost:(port)/data/numbers/17.txt seventeen + When I open data/numbers/18.txt + 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 From d9247c15a4172d812b3a51b8b2b10df294dba16e Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Tue, 26 Jul 2016 09:48:35 -0400 Subject: [PATCH 3/4] Coding style fixes --- qutebrowser/browser/commands.py | 7 +++---- qutebrowser/browser/urlmarks.py | 9 +++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index f86faa0da..3b0e0bc51 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1120,7 +1120,6 @@ class CommandDispatcher: raise cmdexc.CommandError("Quickmark '{}' not found!".format(name)) @cmdutils.register(instance='command-dispatcher', scope='window') - @cmdutils.argument('toggle', flag='t') def bookmark_add(self, url=None, title=None, toggle=False): """Save the current page as a bookmark, or a specific url. @@ -1152,13 +1151,13 @@ class CommandDispatcher: if not title: title = self._current_title() try: - if_added = bookmark_manager.add(url, title, toggle) + was_added = bookmark_manager.add(url, title, toggle=toggle) except urlmarks.Error as e: raise cmdexc.CommandError(str(e)) else: - mes = "Bookmarked {}!" if if_added else "Removed bookmark {}!" + msg = "Bookmarked {}!" if was_added else "Removed bookmark {}!" message.info(self._win_id, - mes.format(url.toDisplayString())) + msg.format(url.toDisplayString())) @cmdutils.register(instance='command-dispatcher', scope='window', maxsplit=0) diff --git a/qutebrowser/browser/urlmarks.py b/qutebrowser/browser/urlmarks.py index 9889c8536..4280f0cd9 100644 --- a/qutebrowser/browser/urlmarks.py +++ b/qutebrowser/browser/urlmarks.py @@ -272,17 +272,18 @@ class BookmarkManager(UrlMarkManager): elif len(parts) == 1: self.marks[parts[0]] = '' - def add(self, url, title, toggle=False): + def add(self, url, title, *, toggle=False): """Add a new bookmark. - Return True if the bookmark was added, and False if it was - removed (which only happens if toggle is True). - 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) From 29fd292aa43b00d5549b7a28be53f6ad92f17e66 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 26 Jul 2016 17:30:07 +0200 Subject: [PATCH 4/4] Update docs --- CHANGELOG.asciidoc | 11 ++++++++++- README.asciidoc | 2 +- doc/help/commands.asciidoc | 6 +++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 9367f50d0..33f4d1c3b 100644 --- a/CHANGELOG.asciidoc +++ b/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 ` argument for `:print` which can be used to generate a +- New `--pdf ` argument for `:print` WHICH can be used to generate a PDF without a dialog. Changed diff --git a/README.asciidoc b/README.asciidoc index 422b87db8..a1a6bffa5 100644 --- a/README.asciidoc +++ b/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 diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index af9089135..b82dc9ac7 100644 --- a/doc/help/commands.asciidoc +++ b/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']+