Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] tools: lazy translation method #31211

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@mart-e
Copy link
Contributor

mart-e commented Feb 18, 2019

Introduces the method _lt

The translation method is now evaluated lazily.
It allows to declare global variables with translatable content

i.e. this code will now work:

LABEL = _lt("User")

def _compute_label(self):
    context = {'lang': self.partner_id.lang}
    self.user_label = LABEL

Alternative patch for odoo/enterprise#3665

Doing self.assertEqual(str(language), "Klingon", ... is needed as comparing language with "Klingon", returns an error:
AssertionError: Klingon != 'Klingon' : The translation should not be applied yet

@robodoo robodoo added the seen 🙂 label Feb 18, 2019

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch 3 times, most recently Feb 18, 2019

@mart-e mart-e requested a review from xmo-odoo Feb 18, 2019

@xmo-odoo
Copy link
Collaborator

xmo-odoo left a comment

Might be worth overloading __eq__ and raising NotImplementedError just in case someone happens to try equality checks on the object.

@C3POdoo C3POdoo added the RD label Feb 18, 2019

@odony

odony approved these changes Feb 18, 2019

Copy link
Contributor

odony left a comment

Looks very nice, thanks! What do you think of using this instead of odoo/enterprise#3665?

Not r+ing yet so you can have a look at my minor comments below ;-)

Show resolved Hide resolved odoo/tools/translate.py Outdated
Show resolved Hide resolved odoo/tools/translate.py Outdated
Show resolved Hide resolved odoo/tools/translate.py Outdated
Show resolved Hide resolved odoo/tools/translate.py Outdated

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch 2 times, most recently Feb 19, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Feb 19, 2019

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch Feb 19, 2019

@robodoo robodoo added the CI 🤖 label Feb 19, 2019

@mart-e mart-e requested a review from rco-odoo Feb 20, 2019

@xmo-odoo

This comment has been minimized.

Copy link
Collaborator

xmo-odoo commented Feb 28, 2019

@mart-e might also be nice if either an error was raised when _() is called during import / at the toplevel of a module, or if it could automatically fallback to lazy in that case, but I'm not sure how to detect such a case.

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch Feb 28, 2019

@robodoo robodoo removed the CI 🤖 label Feb 28, 2019

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch 2 times, most recently Mar 1, 2019

@mart-e

This comment has been minimized.

Copy link
Contributor Author

mart-e commented Mar 4, 2019

@rco-odoo @odony should be good now. I have added a few corrections of bad usage of _ in another commit.

@robodoo robodoo added the CI 🤖 label Mar 4, 2019

@mart-e mart-e requested a review from odony Mar 5, 2019

Show resolved Hide resolved odoo/tools/translate.py Outdated

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch Mar 5, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 5, 2019

odoo/addons/test_translation_import/tests/test_term_count.py Outdated
self.assertEqual(_("Klingon"), "tlhIngan", "The direct code translation was not applied")
context = None

language = _lt("Klingon")

This comment has been minimized.

@rco-odoo

rco-odoo Mar 5, 2019

Member

I suggest to move language at the module level, which is exactly how it should be used.
This would avoid any side effect in the call to _lt.

This comment has been minimized.

@rco-odoo

rco-odoo Mar 5, 2019

Member

BTW, the name "language" is a bit confusing, as it is a term to translate.
Call it TERM or something like that.

odoo/addons/test_translation_import/tests/test_term_count.py Outdated
self.assertEqual(str(language), "Klingon", "The translation should not be applied yet")

context = {'lang': "tlh"}
self.assertEqual(str(language), "tlhIngan", "The lazy code translation was not applied")

This comment has been minimized.

@rco-odoo

rco-odoo Mar 5, 2019

Member

What happens if you do language == "Klingon" ?
Maybe write a test to document that behavior.

@@ -456,7 +462,22 @@ def __call__(self, source):
cr.close()
return res


class LazyStrTranslator:

This comment has been minimized.

@rco-odoo

rco-odoo Mar 5, 2019

Member

Can't you name the class _lt directly?
Please document it!

odoo/tools/translate.py Outdated
def __init__(self, source):
self._source = source

def __repr__(self):

This comment has been minimized.

@rco-odoo

rco-odoo Mar 5, 2019

Member

Why translating in __repr__ instead of __str__?

return _._get_translation(self._source)

def __eq__(self, other):
raise NotImplementedError()

This comment has been minimized.

@rco-odoo

rco-odoo Mar 5, 2019

Member

Please explain why.

This comment has been minimized.

@rco-odoo

rco-odoo Mar 5, 2019

Member

Maybe add the decorator @functools.total_ordering to override all comparison operators.

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch Mar 5, 2019

@robodoo robodoo removed the CI 🤖 label Mar 5, 2019

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch Mar 5, 2019

@robodoo robodoo added the CI 🤖 label Mar 5, 2019

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch Mar 5, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 5, 2019

odoo/tools/translate.py Outdated
def __str__(self):
return _._get_translation(self._source)

@functools.total_ordering

This comment has been minimized.

@rco-odoo

rco-odoo Mar 11, 2019

Member

This decorator must be on the class.

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch to 58ec066 Mar 11, 2019

@robodoo robodoo removed the CI 🤖 label Mar 11, 2019

mart-e added some commits Feb 18, 2019

[ADD] tools: lazy translation method
Introduces the method _lt

The translation method is now evaluated lazily.
It allows to declare global variables with translatable content

e.g. this code will now work:

LABEL = _lt("User")

def _compute_label(self):
    context = {'lang': self.partner_id.lang}
    self.user_label = LABEL
[FIX] *: bad usage of _ method
No need for selection fields
If in global variable, the _lt should be used instead

@mart-e mart-e force-pushed the odoo-dev:master-lazy-translate-mat branch from 58ec066 to e3ef657 Mar 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.