On 03/22/2012 06:01 PM, John Dennis wrote:
This addresses tickets 2044 and 1958. Ticket 2044 was on my short list
but the code had already been written for ticket 1958 (validation of
substitution variables). It had been sitting in my home directory
waiting for integration so since the original 2044 ticket required me to
update test_i18n.py it seemed like a perfect opportunity to get ticket
1958 done as well, it only added 2 hours of additional work.
This is great!
Summary:
In the install/po directory you can now do this:
% make test
% make validate
Details (aside from what is in the commit message):
Also attached (to prevent line wrapping) is an example of validating the
Spanish translation. It gives a good example of the type of errors it's
capable of finding. This is not abstract, mistakes like these caused us
to throw errors in different locales and since we normally test only
with English it's important to assure the other languages do not cause
Python errors.
We should consider adding the i18n validation to our top level
validation logic so don't allow problems to creep in (our original
non-translated strings are checked too and are just as capable of having
errors as the translated strings).
Errors in English strings should be caught by regular tests. But yes, if
we ship translations in the repo, the main test suite should check them.
(If it doesn't take minutes to run).
When I ran the validation on our complete set of files a lot of errors
were reported for
ipalib/plugins/hbactest.py:27
msgid has whitespace between $ and variable in '$ ipa '
This is due to a doc string with shell command examples, the $
is the shell prompt. The validation code looks for $ followed by a word
token (these are template substitutions). We have a lot of variable and
template substitutions in our strings, any of which could be malformed
and cause serious run-time problems. I don't know of any way to have the
validation logic ignore the use of $ as a shell prompt without adding a
special case exception. Since shell prompts are rarity in strings my
suggestion would be to pick a different shell prompt character,
something other than $ and be done with the issue and not try to
over-engineer the validation logic.
No need to test this case -- if there's whitespace, it's not a shell
substitution.
If it's wrong in English, we should notice. If it's wrong in another
language, it should be caught by checking if the translation contains
all necessary substitutions.
(In fact, all these syntax tests are redundant. But they do make nice
error messages.)
Some of the option parsing code is reinventing the wheel a bit. The rest
of the project uses optparse.OptionParser, it'd be nice to switch to that.
Particularly the global config dict is a bad idea. Don't assume all of
this will only ever run as a script. Give the constants their own
variables (and advertise that they're constants, as per
http://www.python.org/dev/peps/pep-0008/#constants).
In validate_substitutions_match, you may be over-validating. Why does a
given substitution need to appear the same number of times in both
strings? Making sure translators include all needed info is nice, but
outside of that, give them some freedom.
A nice sanity test I've seen in Qt is checking if the original and
translation strings end in the same punctuation ('.', '?', '!', ':').
Would it make sense to implement that?
I saw some style issues, e.g.:
- in places such as this:
> raise ValueError("Translated string \"%s\" minus the first & last
character is not equal to msgid \"%s\"" % \
> (translated.encode('utf-8'), msgid))
line continuation backslashes are unnecessary inside parentheses.
Also you can mix single and double quotes to avoid backslashes in strings.
- {'':'', '(':')', '{':'}'} should have spaces after the colons
- in entry_seperator = '-' * page_width there's too much whitespace
All of this is quite a lot of nontrivial code; some tests for the
validation itself would be nice.
All in all, nice work! I hope I don't sound too negative, I really like
where this is going.
--
Petr³
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
