Merge lp:~jr/bzr/i18n-errors into lp:bzr

[Jonathan Riddell]

Turn on translations for errors.

Turn on translations for errors

[Jelmer Vernooij (jelmer)]

It seems like this could install the i18n support more than once, wouldn't it need to be protected against that, like in the command help ?

It would be nice to have a smoke test for this (and perhaps also the command help i18n support), especially given none of the core developers uses i18n.

review: Needs Fixing

[Jonathan Riddell (jr)]

>It seems like this could install the i18n support more than once, wouldn't it need to be protected against that, like in the command help ?

This is no longer the case now that this branch is in trunk https://code.launchpad.net/~jr/bzr/i18n-install-once

>It would be nice to have a smoke test for this (and perhaps also the command help i18n support), especially given none of the core developers uses i18n.

Added

[Jelmer Vernooij (jelmer)]

review: Approve

[Jelmer Vernooij (jelmer)]

sent to pqm by email

[Martin Packman (gz)]

Summarizing from IRC conversion, the error module has a lot of pre-existing problems that have been mostly safe to ignore because:

* All _fmt strings are ascii str, which can be upcast to either non-ascii bytes or unicode (but still break when trying to do both, see bug 273978 for instance).
* Many users have utf-8 consoles so printing utf-8 encoded error messages rather than encoding unicode appropriately appears fine.

In making the _fmt string a localised utf-8 message, these existing confusions with string types are exposed.

+ unicode_fmt = osutils.safe_unicode(fmt)
+ if type(fmt) == unicode:
+ trace.mutter("Unicode strings in error.fmt are deprecated")

I'd replace this with just unicode(fmt) and add a comment that all _fmt strings should be ascii.

+ return gettext(unicode_fmt).encode('utf-8')

Remove the encode here. That will fix exceptions that include paths, or any other unicode strings in the message. On the other hand, it breaks exceptions that think it's safe to interpolate arbitrary bytes into exception messages, but they're already bogus and need fixing anyway.

+ self.overrideAttr(i18n, '_translations', ZzzTranslations())
+ try:
+ workingtree.WorkingTree.open('./foo')
+ except errors.NotBranchError,e:
+ err = str(e)
+ self.assertContainsRe(err, "zz{{Not a branch: .*}}")

If we can make ZzzTranslations return more than just ascii it would be more realistic, and this test would then fail with a UnicodeError.

review: Needs Fixing

[Martin Packman (gz)]

Actually, NotBranchError has some special-casing to prevent breakage, but many other PathError subclasses do not.

[Jonathan Riddell (jr)]

sent to pqm by email

=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py	2011-09-05 10:28:30 +0000
+++ bzrlib/errors.py	2011-09-07 11:50:38 +0000
@@ -20,7 +20,10 @@
 from bzrlib import (
     osutils,
     symbol_versioning,
+    i18n,
+    trace,
     )
+from bzrlib.i18n import gettext
 from bzrlib.patches import (
     MalformedHunkHeader,
     MalformedLine,
@@ -140,7 +143,11 @@
         """Return format string for this exception or None"""
         fmt = getattr(self, '_fmt', None)
         if fmt is not None:
-            return fmt
+            i18n.install()
+            unicode_fmt = unicode(fmt) #_fmt strings should be ascii
+            if type(fmt) == unicode:
+                trace.mutter("Unicode strings in error.fmt are deprecated")
+            return gettext(unicode_fmt)
         fmt = getattr(self, '__doc__', None)
         if fmt is not None:
             symbol_versioning.warn("%s uses its docstring as a format, "
@@ -2792,7 +2799,7 @@
     _fmt = "Container has multiple records with the same name: %(name)s"
 
     def __init__(self, name):
-        self.name = name
+        self.name = name.decode("utf-8")
 
 
 class NoDestinationAddress(InternalBzrError):
 
=== modified file 'bzrlib/pack.py'
--- bzrlib/pack.py	2011-04-08 14:45:25 +0000
+++ bzrlib/pack.py	2011-09-07 11:50:38 +0000
@@ -333,7 +333,7 @@
                 # risk that the same unicode string has been encoded two
                 # different ways.
                 if name_tuple in all_names:
-                    raise errors.DuplicateRecordNameError(name_tuple)
+                    raise errors.DuplicateRecordNameError(name_tuple[0])
                 all_names.add(name_tuple)
         excess_bytes = self.reader_func(1)
         if excess_bytes != '':
 
=== modified file 'bzrlib/tests/test_i18n.py'
--- bzrlib/tests/test_i18n.py	2011-08-25 10:33:14 +0000
+++ bzrlib/tests/test_i18n.py	2011-09-07 11:50:38 +0000
@@ -16,7 +16,11 @@
 
 """Tests for bzrlib.i18n"""
 
-from bzrlib import i18n, tests
+from bzrlib import (i18n, 
+                    tests, 
+                    errors, 
+                    workingtree,
+                    )
 
 
 class ZzzTranslations(object):
@@ -28,7 +32,7 @@
     _null_translation = i18n._gettext.NullTranslations()
 
     def zzz(self, s):
-        return u'zz{{%s}}' % s
+        return u'zz\xe5{{%s}}' % s
 
     def ugettext(self, s):
         return self.zzz(self._null_translation.ugettext(s))
@@ -47,18 +51,18 @@
         trans = ZzzTranslations()
 
         t = trans.zzz('msg')
-        self._check_exact(u'zz{{msg}}', t)
+        self._check_exact(u'zz\xe5{{msg}}', t)
 
         t = trans.ugettext('msg')
-        self._check_exact(u'zz{{msg}}', t)
+        self._check_exact(u'zz\xe5{{msg}}', t)
 
         t = trans.ungettext('msg1', 'msg2', 0)
-        self._check_exact(u'zz{{msg2}}', t)
+        self._check_exact(u'zz\xe5{{msg2}}', t)
         t = trans.ungettext('msg1', 'msg2', 2)
-        self._check_exact(u'zz{{msg2}}', t)
+        self._check_exact(u'zz\xe5{{msg2}}', t)
 
         t = trans.ungettext('msg1', 'msg2', 1)
-        self._check_exact(u'zz{{msg1}}', t)
+        self._check_exact(u'zz\xe5{{msg1}}', t)
 
 
 class TestGetText(tests.TestCase):
@@ -68,11 +72,11 @@
         self.overrideAttr(i18n, '_translations', ZzzTranslations())
 
     def test_oneline(self):
-        self.assertEqual(u"zz{{spam ham eggs}}",
+        self.assertEqual(u"zz\xe5{{spam ham eggs}}",
                          i18n.gettext("spam ham eggs"))
 
     def test_multiline(self):
-        self.assertEqual(u"zz{{spam\nham\n\neggs\n}}",
+        self.assertEqual(u"zz\xe5{{spam\nham\n\neggs\n}}",
                          i18n.gettext("spam\nham\n\neggs\n"))
 
 
@@ -83,11 +87,11 @@
         self.overrideAttr(i18n, '_translations', ZzzTranslations())
 
     def test_oneline(self):
-        self.assertEqual(u"zz{{spam ham eggs}}",
+        self.assertEqual(u"zz\xe5{{spam ham eggs}}",
                          i18n.gettext_per_paragraph("spam ham eggs"))
 
     def test_multiline(self):
-        self.assertEqual(u"zz{{spam\nham}}\n\nzz{{eggs\n}}",
+        self.assertEqual(u"zz\xe5{{spam\nham}}\n\nzz\xe5{{eggs\n}}",
                          i18n.gettext_per_paragraph("spam\nham\n\neggs\n"))
 
 
@@ -104,3 +108,20 @@
         self.overrideEnv('LC_MESSAGES', None)
         self.overrideEnv('LANG', None)
         i18n.install()
+
+class TestTranslate(tests.TestCaseWithTransport):
+
+    def setUp(self):
+        super(TestTranslate, self).setUp()
+        self.overrideAttr(i18n, '_translations', ZzzTranslations())
+
+    def test_error_message_translation(self):
+        """do errors get translated?"""
+        err = None
+        tree = self.make_branch_and_tree('.')
+        self.overrideAttr(i18n, '_translations', ZzzTranslations())
+        try:
+            workingtree.WorkingTree.open('./foo')
+        except errors.NotBranchError,e:
+            err = str(e)
+        self.assertContainsRe(err, u"zz\xe5{{Not a branch: .*}}".encode("utf-8"))
 
=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2011-09-06 13:35:07 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2011-09-07 11:50:38 +0000
@@ -92,6 +92,9 @@
 * bzr now ships with translations for command help.  (Jonathan
   Riddell, INADA Naoki, #83941)
 
+* bzr now ships with translations for command errors. (Jonathan
+  Riddell, INADA Naoki)
+
 Improvements
 ************