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

Make get_translation always return something.

6 files changed, 373 insertions, 85 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 5021093bf..ea6e17f90 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -74,7 +74,12 @@ Dependencies
 - A new install-time dependency on the ``venusian`` distribution was
   added.
 
-- Chameleon 1.2.2 or better is now required (internationalization).
+- Chameleon 1.2.3 or better is now required (internationalization and
+  per-template debug settings).
+
+- Add an explicit direct dependency on ``zope.i18nmessageid``.  This
+  distribution was already a transitive dependency, but now we're
+  relying on it directly within ``repoze.bfg.i18n``.
 
 Internal
 --------
diff --git a/docs/api/i18n.rst b/docs/api/i18n.rst
index 604bf7cd1..a67350b64 100644
--- a/docs/api/i18n.rst
+++ b/docs/api/i18n.rst
@@ -5,15 +5,15 @@
 
 .. automodule:: repoze.bfg.i18n
 
-  .. autofunction:: get_translator(request)
+  .. autoclass:: TranslationString
+ 
+  .. autoclass:: TranslationStringFactory
+
+  .. autoinstance:: bfg_tstr
 
-  .. function:: msg(text, domain=None, default=None, mapping=None)
+  .. autofunction:: get_translator(request)
 
-     Create a :class:`repoze.bfg.i18n.TranslationString` instance in
-     the ``bfg`` domain.  If this is a "technical" translation, use a
-     opaque string for ``text``, and make the ``default`` the format
-     string.  if this is a nontechnical translation, pass ``text``
-     without a ``default``.
+  .. autofunction:: interpolate
 
   .. autoclass:: InterpolationOnlyTranslator
 
diff --git a/docs/narr/i18n.rst b/docs/narr/i18n.rst
index 9eac31621..45bbab342 100644
--- a/docs/narr/i18n.rst
+++ b/docs/narr/i18n.rst
@@ -7,12 +7,219 @@
 Using Internationalization
 ==========================
 
-Setting Up Translation
+:mod:`repoze.bfg` offers an internationalization (i18n) subsystem that
+can be used to translate the text of buttons, the text of error
+messages and other software-defined values into the native language of
+aq user of your :mod:`repoze-bfg` driven website.
+
+Activating Translation
 ----------------------
 
-Pass a :term:`translator factory` object to your application's
+By default, a :mod:`repoze.bfg` application performs no translation
+without explicitly configuring a :term:`translator factory`.  To make
+any translation at all happen, you must pass a translator factory
+object to your application's
 :mod:`repoze.bfg.configuration.Configurator` by supplying it with a
-``translator_factory`` argument.  
+``translator_factory`` argument.  For example:
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.configuration import Configurator
+   from repoze.bfg.i18n import InterpolationOnlyTranslator
+   config = Configurator(translator_factory=InterpolationOnlyTranslator)
+
+.. note:: At the time of this writing, only one (very weak) translator
+   factory named :class:`repoze.bfg.i18n.InterpolationOnlyTranslator`
+   ships as part of the :mod:`repoze.bfg` software.  This class only
+   does basic interpolation of mapping values; it does not actually do
+   any language translation.
+
+Creating a Translation String
+-----------------------------
+
+While you write your software, you can insert specialized markup into
+your Python code that makes it possible for the system to translate
+text values into the languages used by your application's users.  This
+markup generates a :term:`translation string`.  A translation string
+is an object that behave mostly like a normal Unicode object, except
+that it also carries around extra information related to its job as
+part of :mod:`repoze.bfg` the translation machinery.
+
+Using The ``TranslationString`` Class
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+One way to create a translation string is to use the
+:class:`repoze.bfg.i18n.TranslationString` callable:
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.i18n import TranslationString
+   ts = TranslationString('Add')
+
+This creates a Unicode-like object that is a TranslationString.
+
+The first argument to :class:`repoze.bfg.i18n.TranslationString` is
+the ``text``; it is required.  The ``text`` value acts as a default
+value for the translation string if a translation to the user's
+language cannot be found at translation time.  The ``text`` argument
+must be a Unicode object or an ASCII string.  The text may optionally
+contain *replacement markers*.  For instance:
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.i18n import TranslationString
+   ts = TranslationString('Add ${number}')
+
+Within the string above, ``${stuff}`` is a replacement marker.  It
+will be replaced by whatever is in the *mapping* for a translation
+string.  The mapping may be supplied at the same time as the
+replacement marker:
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.i18n import TranslationString
+   ts = TranslationString('Add ${number}', mapping={'number':1})
+
+Any number of replacement markers can be present in th text value, any
+number of times.  Only markers which can be replaced by the values in
+the *mapping* will be replaced at translation time.  The others will
+not be interpolated and will be output literally.
+
+A translation string should also usually carry a *domain*.  The domain
+represents a translation category to disambiguate it from other
+translations of the same msgid, in case they conflict.
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.i18n import TranslationString
+   ts = TranslationString('Add ${number}', mapping={'number':1}, 
+                          domain='form')
+
+The above translation string named a domain of "form".  A
+:term:`translator` function will often use the domain to locate a file
+on the filesystem which contains translations for a given context.  In
+this case, if it were trying to translate to our msgid to German, it
+might try to find a translation from a :term:`gettext` file like this
+one::
+
+   locale/de/LC_MESSAGES/form.mo
+
+In other words, it would want to take translations from the "form.mo"
+translation file in the German language.
+
+Domain translation support is dependent upon the :term:`translator
+factory` in use.  Not all translator factories use domain information
+that is associated with a translation string.  However, it is always
+safe to associate a given translation string with a domain; the
+information is ignored by translators that don't support it.
+
+Finally, the TranslationString constructor accepts a ``msgid``
+argument.  If a ``msgid`` argument is supplied, it is used as the
+*message identifier* for the translation string.  When ``msgid`` is
+``None``, the ``text`` value passed to a TranslationString is used as
+an implicit message identifier.  Message identifiers are matched with
+translations in translation files, so it is often useful to create
+translation strings with "opaque" message identifiers unrelated to
+their default text:
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.i18n import TranslationString
+   ts = TranslationString('Add ${number}', msgid='add-number',
+                           domain='form', mapping={'number':1})
+
+Using the ``bfg_tstr`` Translation String Factory
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Another way to generate a translation string is to use the
+:attr:`repoze.bfg.i18n.bfg_tstr` object.  This object is a
+*translation string factory*.  Basically a translation string factory
+presets the ``domain`` value of any :term:`translation string`
+generated by using it.  For example:
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.i18n import bfg_tstr as _
+   ts = _('Add ${number}', msgid='add-number', mapping={'number':1})
+
+.. note:: We imported ``bfg_tstr`` as the name ``_``.  This is a
+   convention which will be supported by translation file generation
+   tools.
+
+The result of calling ``bfg_tstr`` is a
+:class:`repoze.bfg.i18n.TranslationString` instance.  Even though a
+``domain`` value was not passed to bfg_tstr (as would have been
+necessary if the :class:`repoze.bfg.i18n.TranslationString`
+constructor were used instead of a translation string factory), the
+``domain`` attribute of the resulting translation string will be
+``bfg``.  As a result, the previous code example is completely
+equivalent (except for spelling) to:
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.i18n import TranslationString as _
+   ts = _('Add ${number}', msgid='add-number', mapping={'number':1}, 
+          domain='form')
+
+Using the ``TranslationStringFactory`` Class
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+You can set up your own translation string factory much like the one
+provided as :mod:`repoze.bfg.i18n.bfg_tstr` by using the
+:class:`repoze.bfg.i18n.TranslationStringFactory` class.  For example,
+if you'd like to create a translation string factory which presets the
+``domain`` value of generated translation strings to ``form``, you'd
+do something like this:
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.i18n import TranslationStringFactory
+   _ = TranslationStringFactory('form')
+   ts = _('Add ${number}', msgid='add-number', mapping={'number':1})
+
+.. note:: We created this factory with the name ``_``.  This is a
+   convention which will be supported by translation file generation
+   tools.
+
+Performing a Translation by Hand
+--------------------------------
+
+If you need to perform translation "by hand" in an application, use
+the :func:`repoze.bfg.i18n.get_translator` function to obtain a
+:term:`translator` .  :func:`repoze.bfg.i18n.get_translator` will
+return either the current translator defined by the
+``translator_factory`` passed to the Configurator at startup or a
+default translator if no explicit translator factory has been
+registered.
+
+Remember that a translator is a callable which accepts either a
+:term:`translation string` and which returns a Unicode object
+representing the translation.  So, generating a translation in a view
+component of your application might look like so:
+
+.. code-block:: python
+   :linenos:
+
+   from repoze.bfg.i18n import get_translator
+
+   from repoze.bfg.i18n import bfg_tstr as _
+   ts = _('Add ${number}', mapping={'number':1})
+
+   def aview(request):
+       translator = get_translator(request)
+       translated = translator(ts)
+
+A translator may be called any number of times after being retrieved
+from the ``get_translator`` function.
 
 Defining A Translator Factory
 -----------------------------
@@ -28,8 +235,7 @@ expanded translation of the translation string.
 A simplistic implementation of both a translator factory and a
 translator (via its constructor and ``__call__`` methods respecively)
 named :class:`repoze.bfg.i18n.InterpolationOnlyTranslator` is defined.
-This class only does basic interpolation of mapping values; it does
-not actually do any language translations.  Here it is:
+Here it is:
 
 .. code-block:: python
    :linenos:
@@ -57,32 +263,3 @@ constructor.
    from repoze.bfg.i18n import InterpolationOnlyTranslator
    config = Configurator(translator_factory=InterpolationOnlyTranslator, ...)
 
-Obtaining the Current Translator via :func:`repoze.bfg.i18n.get_translator`
----------------------------------------------------------------------------
-
-If you need to perform translation "by hand" in an application, use
-the :func:`repoze.bfg.i18n.get_translator` function to obtain the
-translator.  :func:`repoze.bfg.i18n.get_translator` will return either
-the current :term:`translator` or ``None`` if no translator is
-defined.
-
-Remeber that a translator is a callable which accepts either a
-:term:`translation string` and which returns a Unicode object
-representing the translation.
-
-Creating a Translation String The Hard Way
-------------------------------------------
-
-Use the :class:`repoze.bfg.i18n.TranslationString` constructor to
-create a translation string.
-
-.. code-block:: python
-   :linenos:
-
-   from repoze.bfg.i18n import TranslationString
-   ts = TranslationString('abc')
-
-  
-
-
-
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):
diff --git a/setup.py b/setup.py
index e5cf8d80b..cae2656eb 100644
--- a/setup.py
+++ b/setup.py
@@ -27,7 +27,7 @@ except IOError:
     README = CHANGES = ''
 
 install_requires=[
-    'Chameleon >= 1.2.2',
+    'Chameleon >= 1.2.3',
     'Paste > 1.7', # temp version pin to prevent PyPi install failure :-(
     'PasteDeploy',
     'PasteScript',
@@ -39,6 +39,7 @@ install_requires=[
     'zope.deprecation',
     'zope.interface >= 3.5.1',  # 3.5.0 comment: "allow to bootstrap on jython"
     'venusian >= 0.2',
+    'zope.i18nmessageid',
     ]
 
 if sys.version_info[:2] < (2, 6):