Go with a subclass of z.i18nmid.Message with the args reordered as a compromise.

Make get_translation always return something.

2 files changed, 147 insertions, 42 deletions

diff --git a/repoze/bfg/i18n.py b/repoze/bfg/i18n.py
index 7fc4c0f87..433dfed5a 100644
--- a/repoze/bfg/i18n.py
+++ b/repoze/bfg/i18n.py
@@ -17,10 +17,7 @@ import re
 from zope.interface import implements
 from zope.interface import classProvides
 
-from zope.i18nmessageid.message import Message
-from zope.i18nmessageid import MessageFactory
-
-msg = MessageFactory('bfg')
+from zope.i18nmessageid import Message
 
 from repoze.bfg.interfaces import ITranslator
 from repoze.bfg.interfaces import ITranslatorFactory
@@ -29,7 +26,69 @@ from repoze.bfg.interfaces import IChameleonTranslate
 from repoze.bfg.threadlocal import get_current_registry
 from repoze.bfg.threadlocal import get_current_request
 
-TranslationString = Message
+class TranslationString(Message):
+    """ The constructor for a :term:`translation string`.  This
+    constructor accepts one required argument named ``text``.
+    ``text`` must be the default text of the translation string,
+    optionally including replacement markers such as ``${foo}``.
+
+    Optional keyword arguments to the TranslationString constructor
+    include ``msgid``, ``mapping`` and ``domain``.
+
+    ``mapping``, if supplied, must be a dictionarylike object which
+    represents the replacement values for any replacement markers
+    found within the ``text`` value of this
+
+    ``msgid`` represents an explicit :term:`message identifier` for
+    this translation string.  Usually, the ``text`` of a translation
+    string serves as its message identifier.  However, using this
+    option you can pass an explicit message identifier, usually a
+    simple string.  This is useful when the ``text`` of a translation
+    string is too complicated or too long to be used as a translation
+    key. If ``msgid`` is ``None`` (the default), the ``msgid`` value
+    used by this translation string will be assumed to be the value of
+    ``text``.
+
+    ``domain`` represents the :term:`translation domain`.  By default,
+    the translation domain is ``None``, indicating that this
+    translation string is associated with no translation domain.
+
+    After a translation string is constructed, its ``text`` value is
+    available as the ``default`` attribute of the object, the
+    ``msgid`` is available as the ``msgid`` attribute of the object,
+    the ``domain`` is available as the ``domain`` attribute, and the
+    ``mapping`` is available as the ``mapping`` attribute.
+    """
+    def __new__(cls, text, mapping=None, msgid=None, domain=None):
+        if msgid is None:
+            msgid = text
+        return Message.__new__(cls, msgid, domain=domain, default=text,
+                               mapping=mapping)
+
+class TranslationStringFactory(object):
+    """ Create a factory which will generate translation strings
+    without requiring that each call to the factory be passed a
+    ``domain`` value.  The ``domain`` value passed to this class'
+    constructor will be used as the ``domain`` values of
+    :class:`repoze.bfg.i18n.TranslationString` objects generated by
+    the ``__call__`` of this class.  The ``text``, ``mapping``, and
+    ``msgid`` values provided to ``__call__`` have the meaning as
+    described by the constructor of the
+    :class:`repoze.bfg.i18n.TranslationString`"""
+    def __init__(self, domain):
+        self.domain = domain
+
+    def __call__(self, text, mapping=None, msgid=None):
+        return TranslationString(text, mapping=mapping, msgid=msgid,
+                                 domain=self.domain)
+                               
+bfg_tstr = TranslationStringFactory('bfg')
+bfg_tstr.__doc__ = """\
+    A :class:`repoze.bfg.i18n.TranslationStringFactory` instance with
+    a default ``domain`` value of ``bfg``.  This object may be called
+    with the values ``text``, ``mapping``, and ``msgid`` as per the
+    documentation of the
+    :class:`repoze.bfg.i18n.TranslationStringFactory` class."""
 
 def get_translator(request, translator_factory=None):
     """ Return a :term:`translator` for the given request based on the
@@ -37,34 +96,30 @@ def get_translator(request, translator_factory=None):
     and the :term:`request` passed in as the request object.  If no
     translator factory was sent to the
     :class:`repoze.bfg.configuration.Configurator` constructor at
-    application startup, this function will return ``None``.
+    application startup, this function will return a very simple
+    default 'interpolation only' translator.
 
-    Note that the translation factory will only be called once per
-    request instance.
+    Note that the translation factory will only be constructed once
+    per request instance.
     """
     
     translator = getattr(request, '_bfg_translator', None)
 
-    if translator is False:
-        return None
-
     if translator is None:
+
         if translator_factory is None:
             try:
                 reg = request.registry
             except AttributeError:
                 reg = get_current_registry()
-            if reg is not None: # pragma: no cover
-                translator_factory = reg.queryUtility(ITranslatorFactory)
+            translator_factory = reg.queryUtility(
+                ITranslatorFactory,
+                default=InterpolationOnlyTranslator)
 
-        if translator_factory is None:
-            request_value = False
-        else:
-            translator = translator_factory(request)
-            request_value = translator
+        translator = translator_factory(request)
 
         try:
-            request._bfg_translator = request_value
+            request._bfg_translator = translator
         except AttributeError: # pragma: no cover
             pass # it's only a cache
 
@@ -92,6 +147,9 @@ class InterpolationOnlyTranslator(object):
         return interpolate(message, mapping)
 
 class ChameleonTranslate(object):
+    """ Registered as a Chameleon translate function 'under the hood'
+    to allow our ITranslator and ITranslatorFactory to drive template
+    translation."""
     implements(IChameleonTranslate)
     def __init__(self, translator_factory):
         self.translator_factory = translator_factory
@@ -100,21 +158,24 @@ class ChameleonTranslate(object):
                  target_language=None, default=None):
         if text is None:
             return None
-        translator = None
-        request = get_current_request()
-        if request is not None:
-            request.chameleon_target_language = target_language
-            translator = get_translator(request, self.translator_factory)
         if default is None:
             default = text
         if mapping is None:
             mapping = {}
-        if translator is None:
-            return interpolate(unicode(default), mapping)
         if not hasattr(text, 'mapping'):
-            text = TranslationString(text, domain=domain, default=default,
-                                     mapping=mapping)
+            text = TranslationString(default, mapping=mapping, msgid=text, 
+                                     domain=domain)
+        translator = self.make_translator(target_language)
         return translator(text)
+
+    def make_translator(self, target_language):
+        translator = None
+        request = get_current_request()
+        if request is not None:
+            translator = get_translator(request, self.translator_factory)
+        if translator is None:
+            translator = InterpolationOnlyTranslator(request)
+        return translator
         
 NAME_RE = r"[a-zA-Z][-a-zA-Z0-9_]*"
 
@@ -122,6 +183,10 @@ _interp_regex = re.compile(r'(?<!\$)(\$(?:(%(n)s)|{(%(n)s)}))'
     % ({'n': NAME_RE}))
     
 def interpolate(text, mapping=None):
+    """ Interpolate a string with one or more *replacement markers*
+    (``${foo}`` or ``${bar}``).  Note that if a :term:`translation
+    string` is passed to this function, it will be implicitly
+    converted back to the Unicode object."""
     def replace(match):
         whole, param1, param2 = match.groups()
         return unicode(mapping.get(param1 or param2, whole))
diff --git a/repoze/bfg/tests/test_i18n.py b/repoze/bfg/tests/test_i18n.py
index f2799e5e2..7d17b4551 100644
--- a/repoze/bfg/tests/test_i18n.py
+++ b/repoze/bfg/tests/test_i18n.py
@@ -1,20 +1,70 @@
 import unittest
 
+class TestTranslationString(unittest.TestCase):
+    def _makeOne(self, text, **kw):
+        from repoze.bfg.i18n import TranslationString
+        return TranslationString(text, **kw)
+
+    def test_msgid_None(self):
+        inst = self._makeOne('text')
+        self.assertEqual(inst, 'text')
+        self.assertEqual(inst.default, 'text')
+
+    def test_msgid_not_None(self):
+        inst = self._makeOne('text', msgid='msgid')
+        self.assertEqual(inst, 'msgid')
+        self.assertEqual(inst.default, 'text')
+
+    def test_allargs(self):
+        inst = self._makeOne('text', msgid='msgid', mapping='mapping',
+                             domain='domain')
+        self.assertEqual(inst, 'msgid')
+        self.assertEqual(inst.default, 'text')
+        self.assertEqual(inst.mapping, 'mapping')
+        self.assertEqual(inst.domain, 'domain')
+
+class TestTranslationStringFactory(unittest.TestCase):
+    def _makeOne(self, domain):
+        from repoze.bfg.i18n import TranslationStringFactory
+        return TranslationStringFactory(domain)
+
+    def test_allargs(self):
+        factory = self._makeOne('budge')
+        inst = factory('text', mapping='mapping', msgid='msgid')
+        self.assertEqual(inst, 'msgid')
+        self.assertEqual(inst.domain, 'budge')
+        self.assertEqual(inst.mapping, 'mapping')
+        self.assertEqual(inst.default, 'text')
+
+class Test_bfg_tstr(unittest.TestCase):
+    def _callFUT(self, text, **kw):
+        from repoze.bfg.i18n import bfg_tstr
+        return bfg_tstr(text, **kw)
+
+    def test_allargs(self):
+        inst = self._callFUT('text', mapping='mapping', msgid='msgid')
+        self.assertEqual(inst, 'msgid')
+        self.assertEqual(inst.domain, 'bfg')
+        self.assertEqual(inst.mapping, 'mapping')
+        self.assertEqual(inst.default, 'text')
+
 class Test_get_translator(unittest.TestCase):
     def _callFUT(self, request):
         from repoze.bfg.i18n import get_translator
         return get_translator(request)
 
     def test_no_ITranslatorFactory(self):
+        from repoze.bfg.i18n import InterpolationOnlyTranslator
         request = DummyRequest()
         request.registry = DummyRegistry()
         translator = self._callFUT(request)
-        self.assertEqual(translator, None)
+        self.assertEqual(translator.__class__, InterpolationOnlyTranslator)
 
     def test_no_registry_on_request(self):
+        from repoze.bfg.i18n import InterpolationOnlyTranslator
         request = DummyRequest()
         translator = self._callFUT(request)
-        self.assertEqual(translator, None)
+        self.assertEqual(translator.__class__, InterpolationOnlyTranslator)
 
     def test_with_ITranslatorFactory_from_registry(self):
         request = DummyRequest()
@@ -30,13 +80,6 @@ class Test_get_translator(unittest.TestCase):
         translator = self._callFUT(request)
         self.assertEqual(translator, 'abc')
 
-    def test_with_ITranslatorFactory_from_request_neg_cache(self):
-        request = DummyRequest()
-        request.registry = DummyRegistry()
-        request._bfg_translator = False
-        translator = self._callFUT(request)
-        self.assertEqual(translator, None)
-
 class TestInterpolationOnlyTranslator(unittest.TestCase):
     def _makeOne(self, request):
         from repoze.bfg.i18n import InterpolationOnlyTranslator
@@ -78,7 +121,6 @@ class TestChameleonTranslate(unittest.TestCase):
         trans = self._makeOne(None)
         result = trans('text')
         self.assertEqual(result, 'text')
-        self.assertEqual(self.request.chameleon_target_language, None)
 
     def test_with_current_request_and_translator(self):
         from repoze.bfg.interfaces import ITranslatorFactory
@@ -88,7 +130,6 @@ class TestChameleonTranslate(unittest.TestCase):
         trans = self._makeOne(None)
         result = trans('text')
         self.assertEqual(result, 'text')
-        self.assertEqual(self.request.chameleon_target_language, None)
         self.assertEqual(result.domain, None)
         self.assertEqual(result.default, 'text')
         self.assertEqual(result.mapping, {})
@@ -102,7 +143,6 @@ class TestChameleonTranslate(unittest.TestCase):
         result = trans('text', domain='domain', mapping={'a':'1'},
                        context=None, target_language='lang',
                        default='default')
-        self.assertEqual(self.request.chameleon_target_language, 'lang')
         self.assertEqual(result, 'text')
         self.assertEqual(result.domain, 'domain')
         self.assertEqual(result.default, 'default')
@@ -159,8 +199,8 @@ class DummyRegistry(object):
     def __init__(self, tfactory=None):
         self.tfactory = tfactory
 
-    def queryUtility(self, iface):
-        return self.tfactory
+    def queryUtility(self, iface, default=None):
+        return self.tfactory or default
 
 class DummyTranslator(object):
     def __call__(self, message):