launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00509
[Merge] lp:~jtv/launchpad/recife-tighten-setCurrentTranslation into lp:~launchpad/launchpad/recife
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/recife-tighten-setCurrentTranslation into lp:~launchpad/launchpad/recife.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
= Tighten POTMsgSet.setCurrentTranslation API =
This removes two warts in my earlier Recife work, as well as an older one:
== IPOFile.pluralforms ==
A POFile used to keep track of its own number of plural forms and its own plurals formula. This was messy and horribly broken. So, once upon a time, Danilo cleaned it up so that we stored this information uniformly (dare I say "canonically") for each language.
But POFile still had its own pluralforms attribute, which really only cared about self.language and based everything else on that. So I'm moving it into Language as guessed_pluralforms.
This will bring us one step closer to not having to pass POFiles around in cases where we really only want a POTemplate and a Language.
== IPOTemplate.translation_side_traits ==
In my last Recife branch I gave IPOTemplate a property that returned the traits object describing its "translation side." Henning and I discussed this on IRC: it struck us as a bit ugly to expose those.
In this branch, I replace this property with one that merely returns the translation side. It's up to the caller to look up the traits (if needed) using the ITranslationSideTraitsSet utility.
This worked out surprisingly well in the end.
== IPOTMsgSet.setCurrentTranslation ==
This method takes as argument, among others, a POFile and a translation side. But a POFile belongs to a POTemplate, which is on a fixed translation side. So I dropped the parameter and made the method figure out the translation side on its own.
Someday we'll be able to drop the POFile argument as well, in favour of a POTemplate and a Language. (Danilo's cleanup and re-do of the old, badly-designed "language variants" just saved us from having to pass an optional variant name as well). For now, alas, I'm not sure we don't still use TranslationMessage.pofile in one or two places—even though it was supposed to become obsolete with message sharing.
To test:
{{{
./bin/test -vvc lp.translations.tests -m potmsgset -m setcurrent -m clearcurrent
./bin/test -vvc lp.services.worlddata.tests.test_language
}}}
There's still a bit of lint left, but I can't see any now that I added or that I really should clean up here. I do try though!
Jeroen
--
https://code.launchpad.net/~jtv/launchpad/recife-tighten-setCurrentTranslation/+merge/32172
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/recife-tighten-setCurrentTranslation into lp:~launchpad/launchpad/recife.
=== modified file 'lib/lp/services/worlddata/interfaces/language.py'
--- lib/lp/services/worlddata/interfaces/language.py 2010-04-16 10:15:09 +0000
+++ lib/lp/services/worlddata/interfaces/language.py 2010-08-10 07:56:48 +0000
@@ -68,6 +68,10 @@
required=False),
exported_as='plural_forms')
+ guessed_pluralforms = Int(
+ title=u"Number of plural forms, or a reasonable guess",
+ required=False, readonly=True)
+
pluralexpression = exported(
TextLine(
title=u'Plural form expression',
=== modified file 'lib/lp/services/worlddata/model/language.py'
--- lib/lp/services/worlddata/model/language.py 2010-08-02 02:13:52 +0000
+++ lib/lp/services/worlddata/model/language.py 2010-08-10 07:56:48 +0000
@@ -73,6 +73,16 @@
self.__class__.__name__, self.englishname, self.code)
@property
+ def guessed_pluralforms(self):
+ """See `ILanguage`."""
+ forms = self.pluralforms
+ if forms is None:
+ # Just take a plausible guess. The caller needs a number.
+ return 2
+ else:
+ return forms
+
+ @property
def alt_suggestion_language(self):
"""See `ILanguage`.
=== modified file 'lib/lp/services/worlddata/tests/test_language.py'
--- lib/lp/services/worlddata/tests/test_language.py 2010-04-15 11:53:11 +0000
+++ lib/lp/services/worlddata/tests/test_language.py 2010-08-10 07:56:48 +0000
@@ -3,7 +3,9 @@
__metaclass__ = type
-from canonical.testing import FunctionalLayer
+from zope.security.proxy import removeSecurityProxy
+
+from canonical.testing import DatabaseFunctionalLayer
from lazr.lifecycle.interfaces import IDoNotSnapshot
from lp.services.worlddata.interfaces.language import ILanguage
from lp.testing import TestCaseWithFactory
@@ -12,10 +14,20 @@
class TestLanguageWebservice(TestCaseWithFactory):
"""Test Language web service API."""
- layer = FunctionalLayer
+ layer = DatabaseFunctionalLayer
def test_translators(self):
self.failUnless(
IDoNotSnapshot.providedBy(ILanguage['translators']),
"ILanguage.translators should not be included in snapshots, "
"see bug 553093.")
+
+ def test_guessed_pluralforms_guesses(self):
+ language = self.factory.makeLanguage()
+ self.assertIs(None, language.pluralforms)
+ self.assertEqual(2, language.guessed_pluralforms)
+
+ def test_guessed_pluralforms_knows(self):
+ language = self.factory.makeLanguage()
+ removeSecurityProxy(language).pluralforms = 3
+ self.assertEqual(language.pluralforms, language.guessed_pluralforms)
=== modified file 'lib/lp/translations/interfaces/potemplate.py'
--- lib/lp/translations/interfaces/potemplate.py 2010-08-09 08:27:28 +0000
+++ lib/lp/translations/interfaces/potemplate.py 2010-08-10 07:56:48 +0000
@@ -17,7 +17,6 @@
from lp.app.errors import NotFoundError
from lp.registry.interfaces.distribution import IDistribution
from lp.translations.interfaces.rosettastats import IRosettaStats
-from lp.translations.interfaces.side import ITranslationSideTraits
from lp.registry.interfaces.sourcepackagename import (
ISourcePackageName)
from lp.translations.interfaces.translationfileformat import (
@@ -290,10 +289,8 @@
gettext uses the original English strings to identify messages.
"""))
- translation_side_traits = Object(
- schema=ITranslationSideTraits,
- title=_("TranslationSideTraits for this template."), required=True,
- readonly=True)
+ translation_side = Int(
+ title=_("Translation side"), required=True, readonly=True)
def __iter__():
"""Return an iterator over current `IPOTMsgSet` in this template."""
=== modified file 'lib/lp/translations/interfaces/potmsgset.py'
--- lib/lp/translations/interfaces/potmsgset.py 2010-08-09 08:27:28 +0000
+++ lib/lp/translations/interfaces/potmsgset.py 2010-08-10 07:56:48 +0000
@@ -267,7 +267,7 @@
"""
def setCurrentTranslation(pofile, submitter, translations, origin,
- translation_side, share_with_other_side=False):
+ share_with_other_side=False):
"""Set the message's translation in Ubuntu, or upstream, or both.
:param pofile: `POFile` you're setting translations in. Other
@@ -277,8 +277,6 @@
:param translations: a dict mapping plural-form numbers to the
translated string for that form.
:param origin: A `RosettaTranslationOrigin`.
- :param translation_side: The `TranslationSide` that this
- translation is for (Ubuntu or upstream).
:param share_with_other_side: When sharing this translation,
share it with the other `TranslationSide` as well.
"""
=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py 2010-08-04 11:00:51 +0000
+++ lib/lp/translations/model/pofile.py 2010-08-10 07:56:48 +0000
@@ -43,8 +43,6 @@
ITranslationFileData)
from lp.translations.interfaces.translationexporter import (
ITranslationExporter)
-from lp.translations.interfaces.translationfileformat import (
- TranslationFileFormat)
from lp.translations.interfaces.translationgroup import (
TranslationPermission)
from lp.translations.interfaces.translationimporter import (
@@ -239,13 +237,7 @@
@property
def plural_forms(self):
"""See `IPOFile`."""
- if self.language.pluralforms is not None:
- forms = self.language.pluralforms
- else:
- # Don't know anything about plural forms for this
- # language, fallback to the most common case, 2.
- forms = 2
- return forms
+ return self.language.guessed_pluralforms
def canEditTranslations(self, person):
"""See `IPOFile`."""
=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py 2010-08-09 16:50:09 +0000
+++ lib/lp/translations/model/potemplate.py 2010-08-10 07:56:48 +0000
@@ -64,9 +64,7 @@
IPOTemplateSharingSubset,
IPOTemplateSubset,
LanguageNotFound)
-from lp.translations.interfaces.side import (
- ITranslationSideTraitsSet,
- TranslationSide)
+from lp.translations.interfaces.side import TranslationSide
from lp.translations.interfaces.translationcommonformat import (
ITranslationFileData)
from lp.translations.interfaces.translationexporter import (
@@ -981,13 +979,12 @@
yield VPOTExport(self, *row)
@property
- def translation_side_traits(self):
+ def translation_side(self):
"""See `IPOTemplate`."""
if self.productseries is not None:
- side = TranslationSide.UPSTREAM
+ return TranslationSide.UPSTREAM
else:
- side = TranslationSide.UBUNTU
- return getUtility(ITranslationSideTraitsSet).getTraits(side)
+ return TranslationSide.UBUNTU
class POTemplateSubset:
=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py 2010-08-09 08:09:26 +0000
+++ lib/lp/translations/model/potmsgset.py 2010-08-10 07:56:48 +0000
@@ -1085,8 +1085,9 @@
**translation_args)
def setCurrentTranslation(self, pofile, submitter, translations, origin,
- translation_side, share_with_other_side=False):
+ share_with_other_side=False):
"""See `IPOTMsgSet`."""
+ translation_side = pofile.potemplate.translation_side
helper = make_message_side_helpers(
translation_side, self, pofile.potemplate, pofile.language)
@@ -1241,7 +1242,8 @@
share_with_other_side=False):
"""See `IPOTMsgSet`."""
template = pofile.potemplate
- traits = template.translation_side_traits
+ traits = getUtility(ITranslationSideTraitsSet).getTraits(
+ template.translation_side)
current = traits.getCurrentMessage(self, template, pofile.language)
if current is None:
=== modified file 'lib/lp/translations/tests/test_clearCurrentTranslation.py'
--- lib/lp/translations/tests/test_clearCurrentTranslation.py 2010-08-09 16:50:09 +0000
+++ lib/lp/translations/tests/test_clearCurrentTranslation.py 2010-08-10 07:56:48 +0000
@@ -5,16 +5,24 @@
__metaclass__ = type
+from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
from canonical.testing import DatabaseFunctionalLayer
from lp.testing import TestCaseWithFactory
+from lp.translations.interfaces.side import ITranslationSideTraitsSet
from lp.translations.interfaces.translationmessage import (
RosettaTranslationOrigin)
ORIGIN = RosettaTranslationOrigin.SCM
+def get_traits(potemplate):
+ """Obtain the translation side traits for template."""
+ return getUtility(ITranslationSideTraitsSet).getTraits(
+ potemplate.translation_side)
+
+
class ScenarioMixin:
layer = DatabaseFunctionalLayer
@@ -67,14 +75,14 @@
potmsgset.clearCurrentTranslation(pofile, template.owner, ORIGIN)
- current = template.translation_side_traits.getCurrentMessage(
+ current = get_traits(template).getCurrentMessage(
potmsgset, template, pofile.language)
self.assertIs(None, current)
def test_deactivates_shared_message(self):
pofile = self._makePOFile()
template = pofile.potemplate
- traits = template.translation_side_traits
+ traits = get_traits(template)
potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
tm = self._makeTranslationMessage(potmsgset, pofile)
traits.setFlag(tm, True)
@@ -87,7 +95,7 @@
def test_deactivates_diverged_message(self):
pofile = self._makePOFile()
template = pofile.potemplate
- traits = template.translation_side_traits
+ traits = get_traits(template)
potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
tm = self._makeTranslationMessage(potmsgset, pofile, diverged=True)
traits.setFlag(tm, True)
@@ -102,7 +110,7 @@
# diverged message to mask the shared message.
pofile = self._makePOFile()
template = pofile.potemplate
- traits = template.translation_side_traits
+ traits = get_traits(template)
potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
shared_tm = self._makeTranslationMessage(potmsgset, pofile)
traits.setFlag(shared_tm, True)
@@ -125,7 +133,7 @@
def test_ignores_other_message(self):
pofile = self._makePOFile()
template = pofile.potemplate
- traits = template.translation_side_traits
+ traits = get_traits(template)
potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
tm = self._makeTranslationMessage(potmsgset, pofile)
traits.setFlag(tm, True)
@@ -142,7 +150,7 @@
def test_deactivates_one_side(self):
pofile = self._makePOFile()
template = pofile.potemplate
- traits = template.translation_side_traits
+ traits = get_traits(template)
potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
tm = self._makeTranslationMessage(potmsgset, pofile)
traits.setFlag(tm, True)
@@ -156,7 +164,7 @@
def test_deactivates_both_sides(self):
pofile = self._makePOFile()
template = pofile.potemplate
- traits = template.translation_side_traits
+ traits = get_traits(template)
potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
tm = self._makeTranslationMessage(potmsgset, pofile)
traits.setFlag(tm, True)
@@ -174,7 +182,7 @@
template = pofile.potemplate
potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
tm = self._makeTranslationMessage(potmsgset, pofile, translations)
- template.translation_side_traits.setFlag(tm, True)
+ get_traits(template).setFlag(tm, True)
suggestion = self._makeTranslationMessage(
potmsgset, pofile, translations)
@@ -187,7 +195,7 @@
def test_converges_with_empty_shared_message(self):
pofile = self._makePOFile()
template = pofile.potemplate
- traits = template.translation_side_traits
+ traits = get_traits(template)
potmsgset = self.factory.makePOTMsgSet(template, sequence=1)
diverged_tm = self._makeTranslationMessage(
potmsgset, pofile, diverged=True)
=== modified file 'lib/lp/translations/tests/test_potmsgset.py'
--- lib/lp/translations/tests/test_potmsgset.py 2010-08-06 07:18:14 +0000
+++ lib/lp/translations/tests/test_potmsgset.py 2010-08-10 07:56:48 +0000
@@ -1605,8 +1605,7 @@
origin = RosettaTranslationOrigin.SCM
message = potmsgset.setCurrentTranslation(
- pofile, pofile.potemplate.owner, translations, origin,
- TranslationSide.UPSTREAM)
+ pofile, pofile.potemplate.owner, translations, origin)
self.assertEqual(message, potmsgset.getImportedTranslationMessage(
pofile.potemplate, pofile.language))
@@ -1676,10 +1675,10 @@
first_message = potmsgset.setCurrentTranslation(
pofile, pofile.potemplate.owner, translations,
- RosettaTranslationOrigin.ROSETTAWEB, TranslationSide.UPSTREAM)
+ RosettaTranslationOrigin.ROSETTAWEB)
second_message = potmsgset.setCurrentTranslation(
pofile, self.factory.makePerson(), translations,
- RosettaTranslationOrigin.SCM, TranslationSide.UPSTREAM)
+ RosettaTranslationOrigin.SCM)
self.assertEqual(first_message, second_message)
message = first_message
@@ -1694,8 +1693,7 @@
origin = RosettaTranslationOrigin.ROSETTAWEB
message = potmsgset.setCurrentTranslation(
- pofile, pofile.potemplate.owner, translations, origin,
- TranslationSide.UPSTREAM)
+ pofile, pofile.potemplate.owner, translations, origin)
self.assertEqual(message, potmsgset.getImportedTranslationMessage(
pofile.potemplate, pofile.language))
=== modified file 'lib/lp/translations/tests/test_setcurrenttranslation.py'
--- lib/lp/translations/tests/test_setcurrenttranslation.py 2010-08-04 09:33:10 +0000
+++ lib/lp/translations/tests/test_setcurrenttranslation.py 2010-08-10 07:56:48 +0000
@@ -13,7 +13,6 @@
from lp.testing import TestCaseWithFactory
from lp.translations.interfaces.translationmessage import (
RosettaTranslationOrigin)
-from lp.translations.interfaces.side import TranslationSide
from lp.translations.model.translationmessage import (
TranslationMessage)
@@ -75,7 +74,6 @@
return self.potmsgset.setCurrentTranslation(
self.pofile, self.pofile.owner, translations,
origin=RosettaTranslationOrigin.ROSETTAWEB,
- translation_side=TranslationSide.UBUNTU,
share_with_other_side=share_with_other_side)
def assert_Current_Diverged_Other_DivergencesElsewhere_are(
