From d99f9a3a20a7905d49abf9683511f7b862896840 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <git@the-compiler.org>
Date: Tue, 10 Nov 2015 22:09:36 +0100
Subject: [PATCH] Improve :set error messages.

---
 qutebrowser/config/config.py           | 23 +++++++++++++++--------
 tests/integration/features/set.feature | 16 ++++++++++------
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py
index 7f1727924..09cde457e 100644
--- a/qutebrowser/config/config.py
+++ b/qutebrowser/config/config.py
@@ -29,6 +29,7 @@ import sys
 import os.path
 import functools
 import configparser
+import contextlib
 import collections
 import collections.abc
 
@@ -666,6 +667,18 @@ class ConfigManager(QObject):
             newval = val.typ.transform(newval)
         return newval
 
+    @contextlib.contextmanager
+    def _handle_config_error(self):
+        """Catch errors in set_command and raise CommandError."""
+        try:
+            yield
+        except (configexc.NoOptionError, configexc.NoSectionError,
+                configexc.ValidationError) as e:
+            raise cmdexc.CommandError("set: {}".format(e))
+        except (configexc.Error, configparser.Error) as e:
+            raise cmdexc.CommandError("set: {} - {}".format(
+                e.__class__.__name__, e))
+
     @cmdutils.register(name='set', instance='config', win_id='win_id',
                        completion=[Completion.section, Completion.option,
                                    Completion.value])
@@ -703,7 +716,7 @@ class ConfigManager(QObject):
             option = option[:-1]
             print_ = True
         else:
-            try:
+            with self._handle_config_error():
                 if option.endswith('!') and option != '!' and value is None:
                     option = option[:-1]
                     val = self.get(section_, option)
@@ -719,16 +732,10 @@ class ConfigManager(QObject):
                 else:
                     raise cmdexc.CommandError("set: The following arguments "
                                               "are required: value")
-            except (configexc.Error, configparser.Error) as e:
-                raise cmdexc.CommandError("set: {} - {}".format(
-                    e.__class__.__name__, e))
 
         if print_:
-            try:
+            with self._handle_config_error():
                 val = self.get(section_, option, transformed=False)
-            except configexc.Error as e:
-                raise cmdexc.CommandError("set: {} - {}".format(
-                    e.__class__.__name__, e))
             message.info(win_id, "{} {} = {}".format(
                 section_, option, val), immediately=True)
 
diff --git a/tests/integration/features/set.feature b/tests/integration/features/set.feature
index 92c6c07bf..a0c3a8c55 100644
--- a/tests/integration/features/set.feature
+++ b/tests/integration/features/set.feature
@@ -17,11 +17,11 @@ Feature: Setting settings.
 
     Scenario: Invalid section
         When I run :set blah blub foo
-        Then the error "set: NoSectionError - Section 'blah' does not exist!" should be shown.
+        Then the error "set: Section 'blah' does not exist!" should be shown.
 
     Scenario: Invalid option
         When I run :set general blub foo
-        Then the error "set: NoOptionError - No option 'blub' in section 'general'" should be shown.
+        Then the error "set: No option 'blub' in section 'general'" should be shown.
 
     Scenario: Toggling an option
         When I run :set general auto-save-config false
@@ -46,6 +46,10 @@ Feature: Setting settings.
         And I run :set -p general auto-save-config!
         Then the message "general auto-save-config = True" should be shown.
 
+    Scenario: Setting an invalid value
+        When I run :set general auto-save-config blah
+        Then the error "set: Invalid value 'blah' - must be a boolean!" should be shown.
+
     Scenario: Setting a temporary option
         # We don't actually check if the option is temporary as this isn't easy
         # to check.
@@ -74,11 +78,11 @@ Feature: Setting settings.
 
     Scenario: Invalid option with ? (issue 1109)
         When I run :set general foo?
-        Then the error "set: NoOptionError - No option 'foo' in section 'general'" should be shown.
+        Then the error "set: No option 'foo' in section 'general'" should be shown.
 
     Scenario: Invalid section/option with ? (issue 1109)
         When I run :set blah foo ?
-        Then the error "set: NoSectionError - Section 'blah' does not exist!" should be shown.
+        Then the error "set: Section 'blah' does not exist!" should be shown.
 
     Scenario: Empty option with !
         When I run :set general !
@@ -90,8 +94,8 @@ Feature: Setting settings.
 
     Scenario: Invalid option with !
         When I run :set general foo!
-        Then the error "set: NoOptionError - No option 'foo' in section 'general'" should be shown.
+        Then the error "set: No option 'foo' in section 'general'" should be shown.
 
     Scenario: Invalid section/option with !
         When I run :set blah foo !
-        Then the error "set: NoSectionError - Section 'blah' does not exist!" should be shown.
+        Then the error "set: Section 'blah' does not exist!" should be shown.