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" - - 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 "" + 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 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 - 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/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 "" 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 - 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