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

[FIX] base: ignore empty translatable attribute #32018

Closed

Conversation

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

mart-e commented Mar 21, 2019

Complete cd40808

  <span class="fa fa-globe" title="Title stuff"/>

is not correctly extracted but

  <span class="fa fa-globe" title=""/>

was also extracted while there is no content

Check the size of the content too

Similar content is present on odoo.com website and should not be translatable
on Transifex

@@ -257,7 +257,7 @@ def process(node):
result.tail = node.tail
has_text = (
todo_has_text or nonspace(result.text) or nonspace(result.tail)
or any(name in TRANSLATED_ATTRS for name in result.attrib)
or any((name in TRANSLATED_ATTRS and len(result.attrib[name])) for name in result.attrib)

This comment has been minimized.

Copy link
@rco-odoo

rco-odoo Mar 21, 2019

Member

Isn't this simpler?

Suggested change
or any((name in TRANSLATED_ATTRS and len(result.attrib[name])) for name in result.attrib)
or any(key in TRANSLATED_ATTRS and val for key, val in result.attrib.items())
[FIX] base: ignore empty translatable attribute
Complete cd40808
  <span class="fa fa-globe" title="Title stuff"/>
is not correctly extracted but
  <span class="fa fa-globe" title=""/>
was also extracted while there is no content

Check the size of the content too

Similar content is present on odoo.com website and should not be translatable
on Transifex

@mart-e mart-e force-pushed the odoo-dev:12.0-not-translate-empty-title-tags-mat branch from f6ae392 to c3ff818 Mar 21, 2019

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

@@ -257,7 +257,7 @@ def process(node):
result.tail = node.tail
has_text = (
todo_has_text or nonspace(result.text) or nonspace(result.tail)
or any(name in TRANSLATED_ATTRS for name in result.attrib)
or any((key in TRANSLATED_ATTRS and val) for key, val in result.attrib.items())

This comment has been minimized.

Copy link
@mreficent

mreficent Mar 22, 2019

Contributor
Suggested change
or any((key in TRANSLATED_ATTRS and val) for key, val in result.attrib.items())
or any((key in TRANSLATED_ATTRS and val) for (key, val) in list(result.attrib.items()))

See https://docs.python.org/3.6/library/stdtypes.html#dict-views

This comment has been minimized.

Copy link
@mart-e

mart-e Mar 22, 2019

Author Contributor

@mreficent why is it a problem to use views? what is the benefit to use a list?

This comment has been minimized.

Copy link
@mreficent

mreficent Mar 22, 2019

Contributor

It's not a problem. It's just that it's more common to use lists everywhere.

This comment has been minimized.

Copy link
@mart-e

mart-e Mar 22, 2019

Author Contributor

if it was for a long for loop and avoid future side effect, I would agree with you. Here it only complexify this (already too long) one liner.

This comment has been minimized.

Copy link
@mreficent

mreficent Mar 22, 2019

Contributor

ok

@mart-e

This comment has been minimized.

Copy link
Contributor Author

mart-e commented Mar 22, 2019

robodoo r+

@robodoo robodoo added the r+ 👌 label Mar 22, 2019

robodoo pushed a commit that referenced this pull request Mar 22, 2019

[FIX] base: ignore empty translatable attribute
Complete cd40808
  <span class="fa fa-globe" title="Title stuff"/>
is not correctly extracted but
  <span class="fa fa-globe" title=""/>
was also extracted while there is no content

Check the size of the content too

Similar content is present on odoo.com website and should not be translatable
on Transifex

closes #32018

Signed-off-by: Martin Trigaux (mat) <mat@odoo.com>
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Mar 22, 2019

Merged, thanks!

@robodoo robodoo closed this Mar 22, 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.