From c016e8a4cf293df17fc6ca3ed7e6a046bf86ba1f Mon Sep 17 00:00:00 2001
From: Florian Bruhin <git@the-compiler.org>
Date: Wed, 29 Jul 2015 12:34:53 +0200
Subject: [PATCH] Improve error handling for quick-/bookmarks.

---
 qutebrowser/browser/commands.py | 19 +++++++++----
 qutebrowser/browser/urlmarks.py | 48 +++++++++++++++++++++++++--------
 2 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py
index 8b863743d..dfbb0592b 100644
--- a/qutebrowser/browser/commands.py
+++ b/qutebrowser/browser/commands.py
@@ -38,7 +38,7 @@ import pygments.formatters
 
 from qutebrowser.commands import userscripts, cmdexc, cmdutils, runners
 from qutebrowser.config import config, configexc
-from qutebrowser.browser import webelem, inspector
+from qutebrowser.browser import webelem, inspector, urlmarks
 from qutebrowser.keyinput import modeman
 from qutebrowser.utils import (message, usertypes, log, qtutils, urlutils,
                                objreg, utils)
@@ -1054,15 +1054,20 @@ class CommandDispatcher:
             bg: Load the quickmark in a new background tab.
             window: Load the quickmark in a new window.
         """
-        url = objreg.get('quickmark-manager').get(name)
+        try:
+            url = objreg.get('quickmark-manager').get(name)
+        except urlmarks.Error as e:
+            raise cmdexc.CommandError(str(e))
         self._open(url, tab, bg, window)
 
     @cmdutils.register(instance='command-dispatcher', scope='window')
     def bookmark_add(self):
         """Save the current page as a bookmark."""
         bookmark_manager = objreg.get('bookmark-manager')
-        bookmark_manager.add(self._win_id, self._current_url(),
-                             self._current_title())
+        try:
+            bookmark_manager.add(self._current_url(), self._current_title())
+        except urlmarks.Error as e:
+            raise cmdexc.CommandError(str(e))
 
     @cmdutils.register(instance='command-dispatcher', scope='window',
                        maxsplit=0,
@@ -1076,7 +1081,11 @@ class CommandDispatcher:
             bg: Load the bookmark in a new background tab.
             window: Load the bookmark in a new window.
         """
-        self._open(urlutils.fuzzy_url(url), tab, bg, window)
+        try:
+            url = urlutils.fuzzy_url(url)
+        except urlutils.FuzzyUrlError as e:
+            raise cmdexc.CommandError(e)
+        self._open(url, tab, bg, window)
 
     @cmdutils.register(instance='command-dispatcher', hide=True,
                        scope='window')
diff --git a/qutebrowser/browser/urlmarks.py b/qutebrowser/browser/urlmarks.py
index e83732c6b..735f05e13 100644
--- a/qutebrowser/browser/urlmarks.py
+++ b/qutebrowser/browser/urlmarks.py
@@ -37,6 +37,34 @@ from qutebrowser.commands import cmdexc, cmdutils
 from qutebrowser.misc import lineparser
 
 
+class Error(Exception):
+
+    """Base class for all errors in this module."""
+
+    pass
+
+
+class InvalidUrlError(Error):
+
+    """Exception emitted when a URL is invalid."""
+
+    pass
+
+
+class DoesNotExistError(Error):
+
+    """Exception emitted when a given URL does not exist."""
+
+    pass
+
+
+class AlreadyExistsError(Error):
+
+    """Exception emitted when a given URL does already exist."""
+
+    pass
+
+
 class UrlMarkManager(QObject):
 
     """Base class for BookmarkManager and QuickmarkManager.
@@ -192,18 +220,14 @@ class QuickmarkManager(UrlMarkManager):
     def get(self, name):
         """Get the URL of the quickmark named name as a QUrl."""
         if name not in self.marks:
-            raise cmdexc.CommandError(
+            raise DoesNotExistError(
                 "Quickmark '{}' does not exist!".format(name))
         urlstr = self.marks[name]
         try:
             url = urlutils.fuzzy_url(urlstr, do_search=False)
         except urlutils.FuzzyUrlError as e:
-            if e.url is None or not e.url.errorString():
-                errstr = ''
-            else:
-                errstr = ' ({})'.format(e.url.errorString())
-            raise cmdexc.CommandError("Invalid URL for quickmark {}: "
-                                      "{}{}".format(name, urlstr, errstr))
+            raise InvalidUrlError(
+                "Invalid URL for quickmark {}: {}".format(name, str(e)))
         return url
 
 
@@ -238,23 +262,25 @@ class BookmarkManager(UrlMarkManager):
         elif len(parts) == 1:
             self.marks[parts[0]] = ''
 
-    def add(self, win_id, url, title):
+    def add(self, url, title):
         """Add a new bookmark.
 
         Args:
-            win_id: The window ID to display the errors in.
             url: The url to add as bookmark.
             title: The title for the new bookmark.
         """
+        if not url.isValid():
+            errstr = urlutils.get_errstring(url)
+            raise InvalidUrlError(errstr)
+
         urlstr = url.toString(QUrl.RemovePassword | QUrl.FullyEncoded)
 
         if urlstr in self.marks:
-            message.error(win_id, "Bookmark already exists!")
+            raise AlreadyExistsError("Bookmark already exists!")
         else:
             self.marks[urlstr] = title
             self.changed.emit()
             self.added.emit(title, urlstr)
-            message.info(win_id, "Bookmark added")
 
     @cmdutils.register(instance='bookmark-manager', maxsplit=0,
                        completion=[usertypes.Completion.bookmark_by_url])