better propertyerror message

test/test_option_setting.py

@@ -4,6 +4,8 @@ do_autopath()
 
 from py.test import raises
 
+from tiramisu.i18n import _
+from tiramisu.error import display_list
 from tiramisu.setting import owners, groups
 from tiramisu.config import Config
 from tiramisu.option import ChoiceOption, BoolOption, IntOption, FloatOption, \
@@ -471,3 +473,65 @@ def test_reset_properties_force_store_value():
     setting.reset(all_properties=True)
     assert setting._p_.get_modified_properties() == {}
     raises(ValueError, 'setting.reset(all_properties=True, opt=option)')
+
+
+def test_pprint():
+    msg_error = _('cannot access to {} {} because has {} {}')
+    msg_is = _("the value of {0} is {1}")
+    msg_is_not = _("the value of {0} is not {1}")
+
+    s = StrOption("string", "", default=["string"], default_multi="string", multi=True, properties=('hidden', 'disabled'))
+    s2 = StrOption("string2", "", default="string")
+    s3 = StrOption("string3", "", default=["string"], default_multi="string", multi=True, properties=('hidden',))
+    intoption = IntOption('int', 'Test int option', default=0)
+    stroption = StrOption('str', 'Test string option', default="abc",
+                          requires=[{'option': intoption, 'expected': 2, 'action': 'hidden', 'inverse': True},
+                    {'option': intoption, 'expected': 3, 'action': 'hidden', 'inverse': True},
+                    {'option': intoption, 'expected': 4, 'action': 'hidden', 'inverse': True},
+                    {'option': intoption, 'expected': 1, 'action': 'disabled'},
+                    {'option': s2, 'expected': 'string', 'action': 'disabled'}])
+
+    val2 = StrOption('val2', "")
+    descr2 = OptionDescription("options", "", [val2], requires=[{'option': intoption, 'expected': 1, 'action': 'hidden'}])
+
+    val3 = StrOption('val3', "", requires=[{'option': stroption, 'expected': '2', 'action': 'hidden', 'inverse': True}])
+
+    descr = OptionDescription("options", "", [s, s2, s3, intoption, stroption, descr2, val3])
+    config = Config(descr)
+    setting = config.cfgimpl_get_settings()
+    config.read_write()
+    config.int = 1
+    try:
+        config.str
+    except Exception, err:
+        pass
+
+    assert str(err) ==  msg_error.format('option', 'str', 'properties', display_list(['disabled (' + display_list([msg_is.format('int', '1'), msg_is.format('string2', 'string')]) + ')', 'hidden (' + msg_is_not.format('int', display_list([2, 3, 4], 'or')) + ')']))
+
+    try:
+        config.options.val2
+    except Exception, err:
+        pass
+
+    assert str(err) == msg_error.format('optiondescription', 'options', 'property', 'hidden (' + msg_is.format('int', 1) + ')')
+
+    try:
+        config.val3
+    except Exception, err:
+        pass
+
+    assert str(err) == msg_error.format('option', 'val3', 'property', 'hidden (' + display_list([msg_is.format('int', 1), msg_is.format('string2', 'string'), msg_is_not.format('int', display_list([2, 3, 4], 'or'))]) + ')')
+
+    try:
+        config.string
+    except Exception, err:
+        pass
+
+    assert str(err) == msg_error.format('option', 'string', 'properties', display_list(['disabled', 'hidden']))
+
+    try:
+        config.string3
+    except Exception, err:
+        pass
+
+    assert str(err) == msg_error.format('option', 'string3', 'property', 'hidden')

tiramisu/error.py

@@ -15,15 +15,45 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 # ____________________________________________________________
 "user defined exceptions"
+from .i18n import _
+
+
+def display_list(lst, separator='and'):
+    if len(lst) == 1:
+        return lst[0]
+    else:
+        lst_ = []
+        for l in lst[:-1]:
+            lst_.append(str(l))
+        return ', '.join(lst_) + _(' {} ').format(separator) + str(lst[-1])
 
 
 # Exceptions for an Option
 class PropertiesOptionError(AttributeError):
     "attempt to access to an option with a property that is not allowed"
-    def __init__(self, msg, proptype):
+    def __init__(self, msg, proptype, settings, datas, option_type):
         self.proptype = proptype
+        self._settings = settings
+        self._datas = datas
+        self._type = option_type
         super(PropertiesOptionError, self).__init__(msg)
 
+    def __str__(self):
+        #this part is a bit slow, so only execute when display
+        req = self._settings.apply_requires(**self._datas)
+        if req != {}:
+            msg = []
+            for action, msg_ in req.items():
+                msg.append('{0} ({1})'.format(action, display_list(msg_)))
+            if len(req) == 1:
+                prop_msg = _('property')
+            else:
+                prop_msg = _('properties')
+            msg = display_list(msg)
+            return _('cannot access to {0} {1} because has {2} {3}').format(self._type, self._datas['path'], prop_msg, msg)
+        else:
+            return self.message
+
 
 #____________________________________________________________
 # Exceptions for a Config

tiramisu/option/baseoption.py

@@ -24,9 +24,10 @@ import warnings
 import sys
 
 from ..i18n import _
-from ..setting import log, undefined
+from ..setting import log, undefined, debug
 from ..autolib import carry_out_calculation
-from ..error import ConfigError, ValueWarning, PropertiesOptionError
+from ..error import (ConfigError, ValueWarning, PropertiesOptionError,
+    display_list)
 from ..storage import get_storages_option
 
 
@@ -38,13 +39,6 @@ forbidden_names = frozenset(['iter_all', 'iter_group', 'find', 'find_first',
                              'read_write', 'getowner', 'set_contexts'])
 
 
-def display_list(list_):
-    if len(list_) == 1:
-        return list_[0]
-    else:
-        return ', '.join(list_[:-1]) + _(' and ') + list_[-1]
-
-
 def valid_name(name):
     "an option's name is a str and does not start with 'impl' or 'cfgimpl'"
     if not isinstance(name, str):  # pragma: optional cover
@@ -417,6 +411,7 @@ class Option(OnlyOption):
                                                 returns_raise=True)
                     if isinstance(opt_value, Exception):
                         if isinstance(opt_value, PropertiesOptionError):
+                            if debug:
                                 log.debug('propertyerror in _launch_consistency: {0}'.format(opt_value))
                             if transitive:
                                 return opt_value
@@ -493,6 +488,7 @@ class Option(OnlyOption):
             err = self._validate(_value, context, current_opt,
                                  returns_raise=True)
             if err:
+                if debug:
                     log.debug('do_validation: value: {0}, index: {1}, '
                               'submulti_index: {2}'.format(_value, _index,
                                                            submulti_index),
@@ -512,6 +508,7 @@ class Option(OnlyOption):
             if not error:
                 error = self._second_level_validation(_value, self._is_warnings_only())
             if error:
+                if debug:
                     log.debug(_('do_validation for {0}: error in value').format(
                         self.impl_getname()), exc_info=True)
                 if self._is_warnings_only():
@@ -718,6 +715,7 @@ class Option(OnlyOption):
                         msg = _("value for {0} and {1} should be different")
                     else:
                         msg = _("value for {0} and {1} must be different")
+                    if debug:
                         log.debug('_cons_not_equal: {0} and {1} are not different'.format(val_inf, val_sup))
                     return ValueError(msg.format(opts[idx_inf].impl_getname(),
                                                  opts[idx_inf + idx_sup + 1].impl_getname()))

tiramisu/option/masterslave.py

@@ -20,7 +20,7 @@
 # the whole pypy projet is under MIT licence
 # ____________________________________________________________
 from ..i18n import _
-from ..setting import log, undefined
+from ..setting import log, undefined, debug
 from ..error import SlaveError, PropertiesOptionError
 from .baseoption import DynSymLinkOption, SymLinkOption, Option
 
@@ -278,6 +278,7 @@ class MasterSlaves(object):
 
     def validate_slave_length(self, masterlen, valuelen, name, opt, setitem=False):
         if valuelen > masterlen or (valuelen < masterlen and setitem):  # pragma: optional cover
+            if debug:
                 log.debug('validate_slave_length: masterlen: {0}, valuelen: {1}, '
                           'setitem: {2}'.format(masterlen, valuelen, setitem))
             raise SlaveError(_("invalid len for the slave: {0}"

tiramisu/setting.py

@@ -20,7 +20,7 @@ from copy import copy
 from logging import getLogger
 import weakref
 from .error import (RequirementError, PropertiesOptionError,
-                    ConstError, ConfigError)
+                    ConstError, ConfigError, display_list)
 from .i18n import _
 
 
@@ -113,6 +113,7 @@ log = getLogger('tiramisu')
 #FIXME
 #import logging
 #logging.basicConfig(format='%(levelname)s:%(message)s', level=logging.DEBUG)
+debug = False
 
 
 # ____________________________________________________________
@@ -394,7 +395,7 @@ class Settings(object):
                 if opt.impl_is_multi() and not opt.impl_is_master_slaves('slave'):
                     props.add('empty')
                 if apply_requires:
-                    requires = self.apply_requires(opt, path, setting_properties, index)
+                    requires = self.apply_requires(opt, path, setting_properties, index, False)
                     if requires != set([]):
                         props = copy(props)
                         props |= requires
@@ -441,7 +442,7 @@ class Settings(object):
                             value=None, force_permissive=False,
                             setting_properties=undefined,
                             self_properties=undefined,
-                            index=None):
+                            index=None, debug=False):
         """
         validation upon the properties related to `opt_or_descr`
 
@@ -496,22 +497,30 @@ class Settings(object):
         # at this point an option should not remain in properties
         if properties != frozenset():
             props = list(properties)
+            datas = {'opt': opt_or_descr, 'path': path, 'setting_properties': setting_properties,
+                     'index': index, 'debug': True}
+            if is_descr:
+                opt_type = 'optiondescription'
+            else:
+                opt_type = 'option'
             if 'frozen' in properties:
                 return PropertiesOptionError(_('cannot change the value for '
                                                'option {0} this option is'
                                                ' frozen').format(
                                                    opt_or_descr.impl_getname()),
-                                             props)
+                                             props, self, datas, opt_type)
             else:
-                if is_descr:
+                if len(props) == 1:
-                    opt_type = 'optiondescription'
+                    prop_msg = 'property'
                 else:
-                    opt_type = 'option'
+                    prop_msg = 'properties'
-                return PropertiesOptionError(_("trying to access to an {0} "
+                return PropertiesOptionError(_("cannot access to {0} {1} "
-                                               "named: {1} with properties {2}"
+                                               "because has {2} {3}"
                                                "").format(opt_type,
                                                           opt_or_descr._name,
-                                                          str(props)), props)
+                                                          prop_msg,
+                                                          display_list(props)), props,
+                                                          self, datas, opt_type)
 
     def setpermissive(self, permissive, opt=None, path=None):
         """
@@ -571,7 +580,7 @@ class Settings(object):
         else:
             self._p_.reset_all_cache()
 
-    def apply_requires(self, opt, path, setting_properties, index):
+    def apply_requires(self, opt, path, setting_properties, index, debug):
         """carries out the jit (just in time) requirements between options
 
         a requirement is a tuple of this form that comes from the option's
@@ -619,6 +628,9 @@ class Settings(object):
             return frozenset()
 
         # filters the callbacks
+        if debug:
+            calc_properties = {}
+        else:
             calc_properties = set()
         context = self._getcontext()
         for requires in opt.impl_getrequires():
@@ -650,16 +662,29 @@ class Settings(object):
                                                      "{1} {2}").format(opt._name,
                                                                        reqpath,
                                                                        properties))
+                        orig_value = value
                         # transitive action, force expected
                         value = expected[0]
                         inverse = False
                     else:
                         raise value
+                else:
+                    orig_value = value
                 if (not inverse and
                         value in expected or
                         inverse and value not in expected):
+                    if debug:
+                        if isinstance(orig_value, PropertiesOptionError):
+                            for act, msg in orig_value._settings.apply_requires(**orig_value._datas).items():
+                                calc_properties.setdefault(action, []).extend(msg)
+                        else:
+                            if not inverse:
+                                msg = _("the value of {0} is {1}")
+                            else:
+                                msg = _("the value of {0} is not {1}")
+                            calc_properties.setdefault(action, []).append(msg.format(reqpath, display_list(expected, 'or')))
+                    else:
                         calc_properties.add(action)
-                    # the calculation cannot be carried out
                         break
         return calc_properties