Merge lp:~openerp-dev/openobject-server/6.0-opw-18482-atp into lp:openobject-server/6.0

Proposed by Atul Patel(OpenERP)
Status: Rejected
Rejected by: Olivier Dony (Odoo)
Proposed branch: lp:~openerp-dev/openobject-server/6.0-opw-18482-atp
Merge into: lp:openobject-server/6.0
Diff against target: 131 lines (+33/-14)
1 file modified
bin/tools/translate.py (+33/-14)
To merge this branch: bzr merge lp:~openerp-dev/openobject-server/6.0-opw-18482-atp
Reviewer Review Type Date Requested Status
Olivier Dony (Odoo) Disapprove
Lorenzo Battistini (community) Approve
Olivier Dony (Old Profile) Pending
Review via email: mp+81701@code.launchpad.net

Description of the change

Hello,

[Issue:18482]- Bad format of exported report translation
 I have fixed issue and Improve translations for webkit report sample using export po.

there is also bug in which step is mentioned. here is link:
https://bugs.launchpad.net/openobject-server/+bug/819334

kindly review it.

Thanks
Atul

To post a comment you must log in.
Revision history for this message
Lorenzo Battistini (elbati) wrote :

Hello Atul,

according to my tests, this merge fixes bug 819334 .

Thanks

review: Approve
Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

I might be wrong, but wouldn't a patch in report_webkit itself be much simpler and less riskier for stable?

For example this minimalist patch [1] makes webkit reports translations global to all webkit reports. This is pretty much what happens with terms that come from the code at the moment, and terms from mako templates are very similar to code. I think it would be sufficient and acceptable here.

[1] http://pastebin.com/KsFEFZyQ

review: Needs Information
Revision history for this message
Rifakat Husen (OpenERP) (rha-openerp) wrote :

@odo: I understand your concern here.
I applied your patch and it didn't work for me. Its calling translate_call() of report_webkit,
but while we do export of any po file from 'Export Translation', its calling server's act_getfile() of base.language.export

report_webkit does not come into picture in exporting po procedure.

Correct me if I am missing something.

Regards.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

On 11/22/2011 07:38 AM, Rifakat (OpenERP) wrote:
> @odo: I understand your concern here.
> I applied your patch and it didn't work for me. Its calling translate_call() of report_webkit,
> but while we do export of any po file from 'Export Translation', its calling server's act_getfile() of base.language.export
>
> report_webkit does not come into picture in exporting po procedure.

Yes that seems like the expected behavior.
As far as I can see on the bug report, the problem is that
translate_call() does not consider the translated strings because it
uses a different name than what is exported in the PO files.

The bug report says:

"""
Due to the line
#: report:addons/report_webkit_sample/report/report_webkit_html.mako:35
translation couldn't correctly imported.
"""

This is the same export format as for strings that come from the code
with _(). And as I explained, I think the exported format is fine!
We decided to consider the mako templates of report_webkit just like
normal code, as they are also using something similar to _().
So there is no reason to change that, and it's fine to see
"report:addons/report_webkit_sample/report/report_webkit_html.mako" in
the exported PO!

The only thing to do is to make translate_call() behave like the _()
method we use in the code, and thus make it ignore the name of the
report, which cannot be matched properly, just like for string that cme
from code. This is what my one-line patch is trying to do.

> Correct me if I am missing something.

Have you tested the case of the bug report after applying my patch?
If it does not work, please explain what does not work:
- what entries do you have in ir.translation after importing a PO
containing webkit translations
- are the strings translated when you generate the report in the PO
language?
- if not, can you investigate why translate_call does not find them?

Keep in mind that we want to fix the fact that the translations are
ignored, *not* the format in which the translations are exported.

Thanks!

Revision history for this message
Rifakat Husen (OpenERP) (rha-openerp) wrote :

Hello Olivier,

Thanks for the clarification.

The problem I am facing is,

While calling the method _get_source() we pass name as None from translate_call(),
but always get name(mako path) in _get_source() rather than None.

Yes I have tested the case with our patch,
>- what entries do you have in ir.translation after importing a PO
>containing webkit translations
Lang: Italiano
name: addons/report_webkit_sample/report/report_webkit_html.mako
source: Supplier Invoice
value: Fattura Fornitore

>- are the strings translated when you generate the report in the PO
>language?
Yes, strings are translated.

Thanks for your help.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

On 11/28/2011 01:38 PM, Rifakat (OpenERP) wrote:
> The problem I am facing is,
>
> While calling the method _get_source() we pass name as None from translate_call(),
> but always get name(mako path) in _get_source() rather than None.

Yes, and that's fine. We don't pass name=None because we expect to have
name=None in the ir.translation table, we pass name=None because we
don't know the name and don't care about matching it.
ir.translation._get_source will not try to match the name if we pass
None. This is just the same as what happens with strings that come from
Python code like _('foo').

> Yes I have tested the case with our patch,
>> - what entries do you have in ir.translation after importing a PO
>> containing webkit translations
> Lang: Italiano
> name: addons/report_webkit_sample/report/report_webkit_html.mako
> source: Supplier Invoice
> value: Fattura Fornitore

Looks correct.

>> - are the strings translated when you generate the report in the PO
>> language?
> Yes, strings are translated.

Then that patch is working fine from the beginning.
As I explained, all we care about is to be able to get translated
strings in final webkit reports, the rest of the bug report can be
ignored. So if it works, use it, we're done, no need to waste more time
on this.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Update: the proper one-line patch has landed on trunk addons at revision [1] and was backported to 6.0 at revision [2].
See the commit comment of revision [1] for more details if my comments on the current merge proposal are not clear enough.

[1] trunk addons rev.6365 revid: <email address hidden>
[2] 6.0 addons rev.5009 revid: <email address hidden>

review: Disapprove

Unmerged revisions

3536. By Atul Patel(OpenERP)

[FIX]translations: support webkit report translation.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/tools/translate.py'
2--- bin/tools/translate.py 2011-06-08 12:56:28 +0000
3+++ bin/tools/translate.py 2011-11-09 10:14:13 +0000
4@@ -42,6 +42,11 @@
5 from tools.misc import UpdateableStr
6 from tools.misc import SKIPPED_ELEMENT_TYPES
7
8+join_dquotes = re.compile(r'([^\\])"[\s\\]*"', re.DOTALL)
9+join_quotes = re.compile(r'([^\\])\'[\s\\]*\'', re.DOTALL)
10+re_dquotes = re.compile(r'[^a-zA-Z0-9_]_\([\s]*"(.+?)"[\s]*?\)', re.DOTALL)
11+re_quotes = re.compile(r'[^a-zA-Z0-9_]_\([\s]*\'(.+?)\'[\s]*?\)', re.DOTALL)
12+
13 _LOCALE2WIN32 = {
14 'af_ZA': 'Afrikaans_South Africa',
15 'sq_AL': 'Albanian_Albania',
16@@ -316,6 +321,7 @@
17 if not line.startswith('msgid'):
18 raise Exception("malformed file: bad line: %s" % line)
19 source = unquote(line[6:])
20+
21 line = self.lines.pop(0).strip()
22 if not source and self.first:
23 # if the source is "" and it's the first msgid, it's the special
24@@ -482,6 +488,16 @@
25 res.extend(trans_parse_xsl(n))
26 return res
27
28+def trans_parse_webkit(de):
29+ res = []
30+ ite = re_dquotes.finditer(de)
31+ for i in ite:
32+ src = i.group(1)
33+ src = join_dquotes.sub(r'\1', src)
34+ src = src.decode('string_escape')
35+ res.append(src)
36+ return res
37+
38 def trans_parse_rml(de):
39 res = []
40 for n in de:
41@@ -534,7 +550,6 @@
42 uid = 1
43 l = pool.obj_pool.items()
44 l.sort()
45-
46 query = 'SELECT name, model, res_id, module' \
47 ' FROM ir_model_data'
48
49@@ -554,7 +569,6 @@
50 query_models += ' ORDER BY module, model'
51
52 cr.execute(query, query_param)
53-
54 _to_translate = []
55 def push_translation(module, type, name, id, source):
56 tuple = (module, source, name, id, type)
57@@ -669,12 +683,27 @@
58 fname = ""
59 if obj.report_rml:
60 fname = obj.report_rml
61- parse_func = trans_parse_rml
62+ file, fileextension = os.path.splitext(fname)
63+ if fileextension == '.mako':
64+ parse_func = trans_parse_webkit
65+ report_type = "report"
66+ else:
67+ parse_func = trans_parse_rml
68 report_type = "report"
69+
70 elif obj.report_xsl:
71 fname = obj.report_xsl
72 parse_func = trans_parse_xsl
73 report_type = "xsl"
74+
75+ if obj.report_type == 'webkit':
76+ report_file = tools.file_open(fname)
77+ dt = report_file.read()
78+ res = parse_func(dt)
79+ name = 'report.%s' % name
80+ for src in res:
81+ push_translation(module, report_type, name, 0, src)
82+
83 if fname and obj.report_type in ('pdf', 'xsl'):
84 try:
85 report_file = tools.file_open(fname)
86@@ -697,15 +726,12 @@
87 push_translation(module, 'model', name, xml_name, encode(trad))
88
89 # End of data for ir.model.data query results
90-
91 cr.execute(query_models, query_param)
92-
93 def push_constraint_msg(module, term_type, model, msg):
94 # Check presence of __call__ directly instead of using
95 # callable() because it will be deprecated as of Python 3.0
96 if not hasattr(msg, '__call__'):
97 push_translation(module, term_type, model, 0, encode(msg))
98-
99 for (model_id, model, module) in cr.fetchall():
100 module = encode(module)
101 model = encode(model)
102@@ -760,11 +786,7 @@
103 logger.debug("Scanning modules at paths: ", path_list)
104
105 mod_paths = []
106- join_dquotes = re.compile(r'([^\\])"[\s\\]*"', re.DOTALL)
107- join_quotes = re.compile(r'([^\\])\'[\s\\]*\'', re.DOTALL)
108- re_dquotes = re.compile(r'[^a-zA-Z0-9_]_\([\s]*"(.+?)"[\s]*?\)', re.DOTALL)
109- re_quotes = re.compile(r'[^a-zA-Z0-9_]_\([\s]*\'(.+?)\'[\s]*?\)', re.DOTALL)
110-
111+
112 def export_code_terms_from_file(fname, path, root, terms_type):
113 fabsolutepath = join(root, fname)
114 frelativepath = fabsolutepath[len(path):]
115@@ -819,8 +841,6 @@
116 for root, dummy, files in tools.osutil.walksymlinks(path):
117 for fname in itertools.chain(fnmatch.filter(files, '*.py')):
118 export_code_terms_from_file(fname, path, root, 'code')
119- for fname in itertools.chain(fnmatch.filter(files, '*.mako')):
120- export_code_terms_from_file(fname, path, root, 'report')
121
122
123 out = [["module","type","name","res_id","src","value"]] # header
124@@ -829,7 +849,6 @@
125 for module, source, name, id, type in _to_translate:
126 trans = trans_obj._get_source(cr, uid, name, type, lang, source)
127 out.append([module, type, name, id, source, encode(trans) or ''])
128-
129 return out
130
131 def trans_load(cr, filename, lang, verbose=True, context=None):