9
.. i18n: OpenERP Specific Guidelines
10
.. i18n: +++++++++++++++++++++++++++
13
OpenERP Specific Guidelines
14
+++++++++++++++++++++++++++
16
.. i18n: Bazaar is your historian
17
.. i18n: ------------------------
18
.. i18n: Do not comment out code if you want to disable it. The code is versioned in
19
.. i18n: Bazaar, and regardless of your opinion about Bazaar, it does not lose
20
.. i18n: the file history.
23
Bazaar is your historian
24
------------------------
25
Do not comment out code if you want to disable it. The code is versioned in
26
Bazaar, and regardless of your opinion about Bazaar, it does not lose
29
.. i18n: Leaving comments just makes the code messier and harder to read. And don't
30
.. i18n: worry about making your changes obvious, that's the purpose of ``diff``::
32
.. i18n: # no example for this one, code was really removed ;-)
35
Leaving comments just makes the code messier and harder to read. And don't
36
worry about making your changes obvious, that's the purpose of ``diff``::
38
# no example for this one, code was really removed ;-)
40
.. i18n: Call your fish a fish
41
.. i18n: ---------------------
42
.. i18n: Give your variables a meaningful name all the time.
43
.. i18n: You may know what it is referring to now, but you won't in 2 months, and
44
.. i18n: others don't either. One-letter variables are acceptable only in lambda
45
.. i18n: expressions and loop indices, or perhaps in pure maths expressions
46
.. i18n: (and even there it doesn't hurt to use a real name)::
48
.. i18n: # unclear and misleading
54
.. i18n: selected_fields = {}
59
Give your variables a meaningful name all the time.
60
You may know what it is referring to now, but you won't in 2 months, and
61
others don't either. One-letter variables are acceptable only in lambda
62
expressions and loop indices, or perhaps in pure maths expressions
63
(and even there it doesn't hurt to use a real name)::
65
# unclear and misleading
73
.. i18n: Do not bypass the ORM
74
.. i18n: ---------------------
75
.. i18n: You should never use the database cursor directly when the ORM can do the
76
.. i18n: same thing! By doing so you are bypassing all the ORM features,
77
.. i18n: possibly the transactions, access rights and so on.
82
You should never use the database cursor directly when the ORM can do the
83
same thing! By doing so you are bypassing all the ORM features,
84
possibly the transactions, access rights and so on.
86
.. i18n: And chances are that you are also making the code harder to read and
87
.. i18n: probably less secure (see also next guideline)::
89
.. i18n: # very very wrong
90
.. i18n: cr.execute('select id from auction_lots where auction_id in (' +
91
.. i18n: ','.join(map(str,ids))+') and state=%s and obj_price>0',
93
.. i18n: auction_lots_ids = [x[0] for x in cr.fetchall()]
95
.. i18n: # no injection, but still wrong
96
.. i18n: cr.execute('select id from auction_lots where auction_id in %s '\
97
.. i18n: 'and state=%s and obj_price>0',
98
.. i18n: (tuple(ids),'draft',))
99
.. i18n: auction_lots_ids = [x[0] for x in cr.fetchall()]
102
.. i18n: auction_lots_ids = self.search(cr,uid,
103
.. i18n: [('auction_id','in',ids),
104
.. i18n: ('state','=','draft'),
105
.. i18n: ('obj_price','>',0)])
108
And chances are that you are also making the code harder to read and
109
probably less secure (see also next guideline)::
112
cr.execute('select id from auction_lots where auction_id in (' +
113
','.join(map(str,ids))+') and state=%s and obj_price>0',
115
auction_lots_ids = [x[0] for x in cr.fetchall()]
117
# no injection, but still wrong
118
cr.execute('select id from auction_lots where auction_id in %s '\
119
'and state=%s and obj_price>0',
120
(tuple(ids),'draft',))
121
auction_lots_ids = [x[0] for x in cr.fetchall()]
124
auction_lots_ids = self.search(cr,uid,
125
[('auction_id','in',ids),
126
('state','=','draft'),
127
('obj_price','>',0)])
129
.. i18n: No SQL injections, please!
130
.. i18n: --------------------------
131
.. i18n: Care must be taken not to introduce SQL injections vulnerabilities when using
132
.. i18n: manual SQL queries. The vulnerability is present when user input is either
133
.. i18n: incorrectly filtered or badly quoted which allow an attacker to introduce
134
.. i18n: undesirable clauses to a SQL query (such as circumventing filters or executing
135
.. i18n: **UPDATE** or **DELETE** commands).
138
No SQL injections, please!
139
--------------------------
140
Care must be taken not to introduce SQL injections vulnerabilities when using
141
manual SQL queries. The vulnerability is present when user input is either
142
incorrectly filtered or badly quoted which allow an attacker to introduce
143
undesirable clauses to a SQL query (such as circumventing filters or executing
144
**UPDATE** or **DELETE** commands).
146
.. i18n: The best way to be safe is to never, NEVER use Python string concatenation (+)
147
.. i18n: or string parameters interpolation (%) to pass variables to a SQL query string.
150
The best way to be safe is to never, NEVER use Python string concatenation (+)
151
or string parameters interpolation (%) to pass variables to a SQL query string.
153
.. i18n: The second reason, which is almost as important, is that it is the job of the
154
.. i18n: database abstraction layer (psycopg2) to decide how to format query parameters,
155
.. i18n: not your job!
156
.. i18n: For example psycopg2 knows that when you pass a list of values it needs to
157
.. i18n: format them as a comma-separated list, enclosed in parentheses!
160
The second reason, which is almost as important, is that it is the job of the
161
database abstraction layer (psycopg2) to decide how to format query parameters,
163
For example psycopg2 knows that when you pass a list of values it needs to
164
format them as a comma-separated list, enclosed in parentheses!
166
.. i18n: .. code-block:: python
168
.. i18n: # the following is very bad:
169
.. i18n: # - it's a SQL injection vulnerability
170
.. i18n: # - it's unreadable
171
.. i18n: # - it's not your job to format the list of ids
172
.. i18n: cr.execute('select distinct child_id from account_account_consol_rel ' +
173
.. i18n: 'where parent_id in ('+','.join(map(str, ids))+')')
176
.. i18n: cr.execute('SELECT DISTINCT child_id '\
177
.. i18n: 'FROM account_account_consol_rel '\
178
.. i18n: 'WHERE parent_id IN %s',
179
.. i18n: (tuple(ids),))
182
.. code-block:: python
184
# the following is very bad:
185
# - it's a SQL injection vulnerability
187
# - it's not your job to format the list of ids
188
cr.execute('select distinct child_id from account_account_consol_rel ' +
189
'where parent_id in ('+','.join(map(str, ids))+')')
192
cr.execute('SELECT DISTINCT child_id '\
193
'FROM account_account_consol_rel '\
194
'WHERE parent_id IN %s',
197
.. i18n: This is very important, so please be careful also when refactoring, and most
198
.. i18n: importantly do not copy these patterns!
201
This is very important, so please be careful also when refactoring, and most
202
importantly do not copy these patterns!
204
.. i18n: Here is a `memorable example <http://www.bobby-tables.com>`_ to help
205
.. i18n: you remember what the issue is about (but do not copy the code there).
208
Here is a `memorable example <http://www.bobby-tables.com>`_ to help
209
you remember what the issue is about (but do not copy the code there).
211
.. i18n: Before continuing, please be sure to read the online documentation of pyscopg2
212
.. i18n: to learn of to use it properly:
215
Before continuing, please be sure to read the online documentation of pyscopg2
216
to learn of to use it properly:
218
.. i18n: * `The problem with query parameters <http://initd.org/psycopg/docs/usage.html#the-problem-with-the-query-parameters>`_
219
.. i18n: * `How to pass parameters with psycopg2 <http://initd.org/psycopg/docs/usage.html#passing-parameters-to-sql-queries>`_
220
.. i18n: * `Advanced parameter types <http://initd.org/psycopg/docs/usage.html#adaptation-of-python-values-to-sql-types>`_
223
* `The problem with query parameters <http://initd.org/psycopg/docs/usage.html#the-problem-with-the-query-parameters>`_
224
* `How to pass parameters with psycopg2 <http://initd.org/psycopg/docs/usage.html#passing-parameters-to-sql-queries>`_
225
* `Advanced parameter types <http://initd.org/psycopg/docs/usage.html#adaptation-of-python-values-to-sql-types>`_
227
.. i18n: Factor out the code
228
.. i18n: -------------------
229
.. i18n: If you are writing the same code over and over and it is more than one line,
230
.. i18n: then you must factor it out into a reusable function or method::
232
.. i18n: # after writing this multiple times:
233
.. i18n: terp = get_module_resource(module, '__openerp__.py')
234
.. i18n: if not os.path.isfile(terp):
235
.. i18n: terp = addons.get_module_resource(module, '__terp__.py')
236
.. i18n: info = eval(tools.file_open(terp).read())
238
.. i18n: # make a function out of it
239
.. i18n: def _load_information_from_description_file(module):
240
.. i18n: for filename in ['__openerp__.py', '__terp__.py']:
241
.. i18n: description_file = addons.get_module_resource(module, filename)
242
.. i18n: if os.path.isfile(description_file):
243
.. i18n: return eval(tools.file_open(description_file).read())
244
.. i18n: raise Exception('The module %s does not contain a description file!')
249
If you are writing the same code over and over and it is more than one line,
250
then you must factor it out into a reusable function or method::
252
# after writing this multiple times:
253
terp = get_module_resource(module, '__openerp__.py')
254
if not os.path.isfile(terp):
255
terp = addons.get_module_resource(module, '__terp__.py')
256
info = eval(tools.file_open(terp).read())
258
# make a function out of it
259
def _load_information_from_description_file(module):
260
for filename in ['__openerp__.py', '__terp__.py']:
261
description_file = addons.get_module_resource(module, filename)
262
if os.path.isfile(description_file):
263
return eval(tools.file_open(description_file).read())
264
raise Exception('The module %s does not contain a description file!')
266
.. i18n: The infamous context
267
.. i18n: --------------------
268
.. i18n: Do not use mutable objects as default values for functions, because they are
269
.. i18n: created as constants (evaluated only once), so you will have possible
270
.. i18n: side-effects if you modify them.
271
.. i18n: The usual example of this is the ``context`` argument to all ORM methods::
273
.. i18n: # bad, this could have side-effects
274
.. i18n: def spam(eggs, context={}):
275
.. i18n: setting = context.get('foo')
278
.. i18n: # this is better if your need to use the context
279
.. i18n: def spam(eggs, context=None):
280
.. i18n: if context is None:
281
.. i18n: context = {}
282
.. i18n: setting = context.get('foo')
288
Do not use mutable objects as default values for functions, because they are
289
created as constants (evaluated only once), so you will have possible
290
side-effects if you modify them.
291
The usual example of this is the ``context`` argument to all ORM methods::
293
# bad, this could have side-effects
294
def spam(eggs, context={}):
295
setting = context.get('foo')
298
# this is better if your need to use the context
299
def spam(eggs, context=None):
302
setting = context.get('foo')
305
.. i18n: Also be careful with boolean tests on lists and maps, because an empty
306
.. i18n: dict, list or tuple will evaluate as ``False``::
308
.. i18n: # bad, you shadow the original context if it's empty
309
.. i18n: def spam(eggs, context=None):
310
.. i18n: if not context:
311
.. i18n: context = {}
312
.. i18n: setting = context.get('foo')
316
Also be careful with boolean tests on lists and maps, because an empty
317
dict, list or tuple will evaluate as ``False``::
319
# bad, you shadow the original context if it's empty
320
def spam(eggs, context=None):
323
setting = context.get('foo')
326
.. i18n: And it's okay if you only need to forward it, you can pass ``None`` and
327
.. i18n: let the downstream code handle it::
330
.. i18n: def spam(eggs, context=None):
331
.. i18n: setting = get_setting(True, context=context)
334
And it's okay if you only need to forward it, you can pass ``None`` and
335
let the downstream code handle it::
338
def spam(eggs, context=None):
339
setting = get_setting(True, context=context)
341
.. i18n: See also `launchpad bug 525808 <https://bugs.launchpad.net/openobject-server/+bug/525808>`_.
344
See also `launchpad bug 525808 <https://bugs.launchpad.net/openobject-server/+bug/525808>`_.
346
.. i18n: There is better than lambda, sometimes
347
.. i18n: --------------------------------------
348
.. i18n: Instead of writing trivial lambda expression to extract items or attributes
349
.. i18n: from a list of data structures, learn to use list comprehension
350
.. i18n: or ``operator.itemgetter`` and ``operator.attrgetter`` instead, which are
351
.. i18n: often more readable and faster::
353
.. i18n: # not very readable
354
.. i18n: partner_tuples = map(lambda x: (x['id'], x['name']), partners)
356
.. i18n: # better with list comprehension for just one item/attribute
357
.. i18n: partner_ids = [partner['id'] for partner in partners]
359
.. i18n: # better with operator for many items/attributes
360
.. i18n: from operator import itemgetter
362
.. i18n: partner_tuples = map(itemgetter('id', 'name'), partners)
365
There is better than lambda, sometimes
366
--------------------------------------
367
Instead of writing trivial lambda expression to extract items or attributes
368
from a list of data structures, learn to use list comprehension
369
or ``operator.itemgetter`` and ``operator.attrgetter`` instead, which are
370
often more readable and faster::
373
partner_tuples = map(lambda x: (x['id'], x['name']), partners)
375
# better with list comprehension for just one item/attribute
376
partner_ids = [partner['id'] for partner in partners]
378
# better with operator for many items/attributes
379
from operator import itemgetter
381
partner_tuples = map(itemgetter('id', 'name'), partners)
383
.. i18n: See also http://docs.python.org/library/operator.html#operator.attrgetter
386
See also http://docs.python.org/library/operator.html#operator.attrgetter
388
.. i18n: As of version 6.0 you can also use literal values as defaults for
389
.. i18n: your ORM columns, which means that you can stop writing these::
391
.. i18n: # lots of trivial one-liners in 5.0
392
.. i18n: _defaults = {
393
.. i18n: 'active': lambda *x: True,
394
.. i18n: 'state': lambda *x: 'draft',
397
.. i18n: # much simpler as of 6.0
398
.. i18n: _defaults = {
399
.. i18n: 'active': True,
400
.. i18n: 'state': 'draft',
404
As of version 6.0 you can also use literal values as defaults for
405
your ORM columns, which means that you can stop writing these::
407
# lots of trivial one-liners in 5.0
409
'active': lambda *x: True,
410
'state': lambda *x: 'draft',
413
# much simpler as of 6.0
419
.. i18n: .. warning::
421
.. i18n: Be careful with this, because non-callable defaults are only evaluated
422
.. i18n: once! If you want to generate new default values for each
423
.. i18n: record you really need to keep the ``lambda`` or make it a callable.
428
Be careful with this, because non-callable defaults are only evaluated
429
once! If you want to generate new default values for each
430
record you really need to keep the ``lambda`` or make it a callable.
432
.. i18n: The most frequent error is with timestamps, as in the following example::
434
.. i18n: # This will always give the server start time!
435
.. i18n: _defaults = {
436
.. i18n: 'timestamp': time.strftime('%Y-%m-%d %H:%M:%S'),
439
.. i18n: # You need to keep it callable, e.g:
440
.. i18n: _defaults = {
441
.. i18n: 'timestamp': lambda *x: time.strftime('%Y-%m-%d %H:%M:%S'),
445
The most frequent error is with timestamps, as in the following example::
447
# This will always give the server start time!
449
'timestamp': time.strftime('%Y-%m-%d %H:%M:%S'),
452
# You need to keep it callable, e.g:
454
'timestamp': lambda *x: time.strftime('%Y-%m-%d %H:%M:%S'),
457
.. i18n: Keep your methods short/simple when possible
458
.. i18n: --------------------------------------------
459
.. i18n: Functions and methods should not contain too much logic: having a lot of small and simple methods is more advisable than having few
460
.. i18n: large and complex methods. A good rule of thumb is to split a method as soon as:
463
Keep your methods short/simple when possible
464
--------------------------------------------
465
Functions and methods should not contain too much logic: having a lot of small and simple methods is more advisable than having few
466
large and complex methods. A good rule of thumb is to split a method as soon as:
468
.. i18n: * it has more than one responsibility (see http://en.wikipedia.org/wiki/Single_responsibility_principle)
469
.. i18n: * it is too big to fit on one screen.
472
* it has more than one responsibility (see http://en.wikipedia.org/wiki/Single_responsibility_principle)
473
* it is too big to fit on one screen.
475
.. i18n: Also, name your functions accordingly: small and properly named functions are the starting point of readable/maintainable code and tighter documentation.
478
Also, name your functions accordingly: small and properly named functions are the starting point of readable/maintainable code and tighter documentation.
480
.. i18n: This recommendation is also relevant for classes, files, modules and packages. (See also http://en.wikipedia.org/wiki/Cyclomatic_complexity )
483
This recommendation is also relevant for classes, files, modules and packages. (See also http://en.wikipedia.org/wiki/Cyclomatic_complexity )
485
.. i18n: Never commit the transaction
486
.. i18n: ----------------------------
487
.. i18n: The OpenERP/OpenObject framework is in charge of providing the transactional context for all RPC calls.
488
.. i18n: The principle is that a new database cursor is opened at the begining of each RPC call, and commited
489
.. i18n: when the call has returned, just before transmitting the answer to the RPC client, approximately like this::
491
.. i18n: def execute(self, db_name, uid, obj, method, *args, **kw):
492
.. i18n: db, pool = pooler.get_db_and_pool(db_name)
493
.. i18n: # create transaction cursor
494
.. i18n: cr = db.cursor()
496
.. i18n: res = pool.execute_cr(cr, uid, obj, method, *args, **kw)
497
.. i18n: cr.commit() # all good, we commit
498
.. i18n: except Exception:
499
.. i18n: cr.rollback() # error, rollback everything atomically
502
.. i18n: cr.close() # always close cursor opened manually
506
Never commit the transaction
507
----------------------------
508
The OpenERP/OpenObject framework is in charge of providing the transactional context for all RPC calls.
509
The principle is that a new database cursor is opened at the begining of each RPC call, and commited
510
when the call has returned, just before transmitting the answer to the RPC client, approximately like this::
512
def execute(self, db_name, uid, obj, method, *args, **kw):
513
db, pool = pooler.get_db_and_pool(db_name)
514
# create transaction cursor
517
res = pool.execute_cr(cr, uid, obj, method, *args, **kw)
518
cr.commit() # all good, we commit
520
cr.rollback() # error, rollback everything atomically
523
cr.close() # always close cursor opened manually
526
.. i18n: If any error occurs during the execution of the RPC call, the transaction is rolled back atomically,
527
.. i18n: preserving the state of the system.
530
If any error occurs during the execution of the RPC call, the transaction is rolled back atomically,
531
preserving the state of the system.
533
.. i18n: Similarly, the system also provides a dedicated transaction during the execution of tests suites,
534
.. i18n: so it can be rolled back or not depending on the server startup options.
537
Similarly, the system also provides a dedicated transaction during the execution of tests suites,
538
so it can be rolled back or not depending on the server startup options.
540
.. i18n: The consequence is that if you manually call ``cr.commit()`` anywhere there is a very high chance
541
.. i18n: that you **will** break the system in various ways, because you will cause partial commits, and thus
542
.. i18n: partial and unclean rollbacks, causing among others:
545
The consequence is that if you manually call ``cr.commit()`` anywhere there is a very high chance
546
that you **will** break the system in various ways, because you will cause partial commits, and thus
547
partial and unclean rollbacks, causing among others:
549
.. i18n: - inconsistent business data, usually data loss ;
550
.. i18n: - workflow desynchronization, documents stuck permanently ;
551
.. i18n: - tests that can't be rolled back cleanly, and will start polluting the database,
552
.. i18n: and triggering error (this is true even if no error occurs during the transaction) ;
555
- inconsistent business data, usually data loss ;
556
- workflow desynchronization, documents stuck permanently ;
557
- tests that can't be rolled back cleanly, and will start polluting the database,
558
and triggering error (this is true even if no error occurs during the transaction) ;
560
.. i18n: Here is the very simple rule:
563
Here is the very simple rule:
565
.. i18n: .. warning:: **You should NEVER call cr.commit() yourself, UNLESS you have created your own
566
.. i18n: database cursor explicitly! And the situations where you need to do that are exceptional!**
569
.. warning:: **You should NEVER call cr.commit() yourself, UNLESS you have created your own
570
database cursor explicitly! And the situations where you need to do that are exceptional!**
572
.. i18n: And by the way if you did create your own cursor, then you need to handle error cases and proper
573
.. i18n: rollback, as well as properly close the cursor when you're done with it.
576
And by the way if you did create your own cursor, then you need to handle error cases and proper
577
rollback, as well as properly close the cursor when you're done with it.
579
.. i18n: And contrary to popular belief, you do *not* even need to call ``cr.commit()`` in the following
583
And contrary to popular belief, you do *not* even need to call ``cr.commit()`` in the following
586
.. i18n: - in the ``_auto_init()`` method of an ``osv.osv`` object: this is taken care of by the addons
587
.. i18n: initialization method, or by the ORM transaction when creating custom models ;
588
.. i18n: - in reports: the ``commit()`` is handled by the framework too, so you can update the database
589
.. i18n: even from within a report ;
590
.. i18n: - within ``osv.osv_memory`` methods: these methods are called exactly like regular ``osv.osv``
591
.. i18n: ones, within a transaction and with the corresponding ``cr.commit()``/``rollback()`` at the end ;
592
.. i18n: - etc. (see general rule above if you have in doubt!)
595
- in the ``_auto_init()`` method of an ``osv.osv`` object: this is taken care of by the addons
596
initialization method, or by the ORM transaction when creating custom models ;
597
- in reports: the ``commit()`` is handled by the framework too, so you can update the database
598
even from within a report ;
599
- within ``osv.osv_memory`` methods: these methods are called exactly like regular ``osv.osv``
600
ones, within a transaction and with the corresponding ``cr.commit()``/``rollback()`` at the end ;
601
- etc. (see general rule above if you have in doubt!)
603
.. i18n: And another very simple rule:
606
And another very simple rule:
608
.. i18n: .. warning:: **All cr.commit() calls outside of the server framework from now on must have an explicit
609
.. i18n: comment explaining why they are absolutely necessary, why they are indeed correct, and why
610
.. i18n: they do not break the transactions. Otherwise they can and will be removed!**
613
.. warning:: **All cr.commit() calls outside of the server framework from now on must have an explicit
614
comment explaining why they are absolutely necessary, why they are indeed correct, and why
615
they do not break the transactions. Otherwise they can and will be removed!**
617
.. i18n: Use the gettext method correctly
618
.. i18n: --------------------------------
621
Use the gettext method correctly
622
--------------------------------
624
.. i18n: OpenERP uses a GetText-like method named "underscore" ``_( )`` to indicate that a static
625
.. i18n: string used in the code needs to be translated at runtime using the language of the context.
626
.. i18n: This pseudo-method is accessed within your code by importing as follows::
628
.. i18n: from tools.translate import _
631
OpenERP uses a GetText-like method named "underscore" ``_( )`` to indicate that a static
632
string used in the code needs to be translated at runtime using the language of the context.
633
This pseudo-method is accessed within your code by importing as follows::
635
from tools.translate import _
637
.. i18n: A few very important rules must be followed when using it, in order for it to work and to
638
.. i18n: avoid filling the translations with useless junk.
641
A few very important rules must be followed when using it, in order for it to work and to
642
avoid filling the translations with useless junk.
644
.. i18n: Basically, this method should only be used for static strings written manually in the code,
645
.. i18n: it will not work to translate field *values*, such as Product names, etc. This must be
646
.. i18n: done instead using the ``translate`` flag on the corresponding field.
649
Basically, this method should only be used for static strings written manually in the code,
650
it will not work to translate field *values*, such as Product names, etc. This must be
651
done instead using the ``translate`` flag on the corresponding field.
653
.. i18n: The rule is very simple: calls to the underscore method should *always* be in the form ``_('literal string')``
654
.. i18n: and nothing else::
656
.. i18n: # Good: plain strings
657
.. i18n: error = _('This record is locked!')
659
.. i18n: # Good: strings with formatting patterns included
660
.. i18n: error = _('Record %s cannot be modified!') % record
662
.. i18n: # OK too: multi-line literal strings
663
.. i18n: error = _("""This is a bad multiline example
664
.. i18n: about record %s!""") % record
665
.. i18n: error = _('Record %s cannot be modified' \
666
.. i18n: 'after being validated!') % record
668
.. i18n: # BAD: tries to translate after string formatting
669
.. i18n: # (pay attention to brackets!)
670
.. i18n: # This does NOT work and messes up the translations!
671
.. i18n: error = _('Record %s cannot be modified!' % record)
673
.. i18n: # BAD: dynamic string, string concatenation, etc are forbidden!
674
.. i18n: # This does NOT work and messes up the translations!
675
.. i18n: error = _("'" + que_rec['question'] + "' \n")
677
.. i18n: # BAD: field values are automatically translated by the framework
678
.. i18n: # This is useless and will not work the way you think:
679
.. i18n: error = _("Product %s is out of stock!") % _(product.name)
680
.. i18n: # and the following will of course not work as already explained:
681
.. i18n: error = _("Product %s is out of stock!" % product.name)
683
.. i18n: # BAD: field values are automatically translated by the framework
684
.. i18n: # This is useless and will not work the way you think:
685
.. i18n: error = _("Product %s is not available!") % _(product.name)
686
.. i18n: # and the following will of course not work as already explained:
687
.. i18n: error = _("Product %s is not available!" % product.name)
689
.. i18n: # Instead you can do the following and everything will be translated,
690
.. i18n: # including the product name if its field definition has the
691
.. i18n: # translate flag properly set:
692
.. i18n: error = _("Product %s is not available!") % product.name
695
The rule is very simple: calls to the underscore method should *always* be in the form ``_('literal string')``
698
# Good: plain strings
699
error = _('This record is locked!')
701
# Good: strings with formatting patterns included
702
error = _('Record %s cannot be modified!') % record
704
# OK too: multi-line literal strings
705
error = _("""This is a bad multiline example
706
about record %s!""") % record
707
error = _('Record %s cannot be modified' \
708
'after being validated!') % record
710
# BAD: tries to translate after string formatting
711
# (pay attention to brackets!)
712
# This does NOT work and messes up the translations!
713
error = _('Record %s cannot be modified!' % record)
715
# BAD: dynamic string, string concatenation, etc are forbidden!
716
# This does NOT work and messes up the translations!
717
error = _("'" + que_rec['question'] + "' \n")
719
# BAD: field values are automatically translated by the framework
720
# This is useless and will not work the way you think:
721
error = _("Product %s is out of stock!") % _(product.name)
722
# and the following will of course not work as already explained:
723
error = _("Product %s is out of stock!" % product.name)
725
# BAD: field values are automatically translated by the framework
726
# This is useless and will not work the way you think:
727
error = _("Product %s is not available!") % _(product.name)
728
# and the following will of course not work as already explained:
729
error = _("Product %s is not available!" % product.name)
731
# Instead you can do the following and everything will be translated,
732
# including the product name if its field definition has the
733
# translate flag properly set:
734
error = _("Product %s is not available!") % product.name
736
.. i18n: Also, keep in mind that translators will have to work with the literal values that are passed
737
.. i18n: to the underscore function, so please try to make them easy to understand and keep spurious
738
.. i18n: characters and formatting to a minimum. Translators must be aware that formatting patterns such
739
.. i18n: as ``%s`` or ``%d``, newlines, etc. need to be preserved, but it's important to use these
740
.. i18n: in a sensible and obvious manner::
742
.. i18n: # Bad: makes the translations hard to work with
743
.. i18n: error = "'" + question + _("' \nPlease enter an integer value ")
745
.. i18n: # Better (pay attention to position of the brackets too!)
746
.. i18n: error = _("Answer to question %s is not valid.\n" \
747
.. i18n: "Please enter an integer value.") % question
750
Also, keep in mind that translators will have to work with the literal values that are passed
751
to the underscore function, so please try to make them easy to understand and keep spurious
752
characters and formatting to a minimum. Translators must be aware that formatting patterns such
753
as ``%s`` or ``%d``, newlines, etc. need to be preserved, but it's important to use these
754
in a sensible and obvious manner::
756
# Bad: makes the translations hard to work with
757
error = "'" + question + _("' \nPlease enter an integer value ")
759
# Better (pay attention to position of the brackets too!)
760
error = _("Answer to question %s is not valid.\n" \
761
"Please enter an integer value.") % question