Nice patch! See comments below.
on 2014/04/08 09:38, shaohef(a)linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
When I do i18n translation, I find some string formattings are wrong.
So I add a check-local command to help the developer to check their
string formatting.
Every developers please run this command before submit your patch.
After you run this command, it may report some invalid string
formattings as follow.
$ make check-local
Checking for invalid i18n string...
bad i18n string formatting:
KCHAPI0005E: Create is not allowed for %(resource).
make: *** [check-local] Error 1
You should check %(resource) is what you want.
Ref:
https://docs.python.org/2/library/stdtypes.html#string-formatting-operations
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com>
---
Makefile.am | 5 +++++
contrib/check_i18n.py | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+)
create mode 100755 contrib/check_i18n.py
diff --git a/Makefile.am b/Makefile.am
index c68f050..401ff37 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -73,7 +73,12 @@ PEP8_WHITELIST = \
SKIP_PYFLAKES_ERR = "\./src/kimchi/websocket\.py"
+I18N_FILES = plugins/*/i18n.py \
+ src/kimchi/i18n.py \
+ $(NULL)
+
check-local:
+ contrib/check_i18n.py $(I18N_FILES)
find . -path './.git' -prune -type f -o \
-name '*.py' -o -name '*.py.in' | xargs $(PYFLAKES) | \
grep -w -v $(SKIP_PYFLAKES_ERR) | \
diff --git a/contrib/check_i18n.py b/contrib/check_i18n.py
new file mode 100755
index 0000000..e3c2b8c
--- /dev/null
+++ b/contrib/check_i18n.py
@@ -0,0 +1,61 @@
+#!/usr/bin/python
+#
+# Project Kimchi
+#
+# Copyright IBM, Corp. 2014
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+import glob
+import imp
+import os
+import re
+import sys
+
+
+PATTERN = re.compile("%\([^\)]*\)[#0\-\+\s]?(\d+|\*)?(\.(\d+|\*)?)?"
+ "[lLh]?[cdeEfFgGioursxX%]")
It seems the PATTERN does not fully follow the Python specification. In
"5.6.2. String Formatting Operations", there is a "Flag" table. It
shows
that "#" "0" "-" " "(a space) and "+"
are valid flags. It implies the
following
'%(a) d' % {'a': 100} produces ' 100'
'%(a) d' % {'a': -100} produces '-100'
So the a space should be added to "[#0\-\+\s]" part of the pattern.
According to the description of the "5. Precision", it seem that
"(\.(\d+|\*)?)?" should be changed to "(\.(\d+|\*))?".
+BAD_PATTERN = re.compile("%\([^\)]*\)")
+
+
+def load_i18n_module(i18nfile):
+ path = os.path.dirname(i18nfile)
+ mname = i18nfile.replace("/", "_").rstrip(".py")
+ mobj = imp.find_module("i18n", [path])
+ return imp.load_module(mname, *mobj)
+
+
+def check_string_formatting(messages):
+ for k, v in messages.iteritems():
+ if BAD_PATTERN.findall(PATTERN.sub(" ", v)):
+ print "bad i18n string formatting:"
+ print " %s: %s" % (k, v)
+ exit(1)
+
+
+def main():
+ files = []
+ for v in sys.argv[1:]:
+ files.extend(glob.glob(v))
+
It seems that you don't have to do path pattern matching here. The shell
shall perform the path expansion automatically. When in Makefile it runs
the following,
contrib/check_i18n.py $(I18N_FILES)
Then make expands I18N_FILES into
plugins/*/i18n.py src/kimchi/i18n.py
Then it runs
contrib/check_i18n.py plugins/*/i18n.py src/kimchi/i18n.py
Then shell expands plugins/*/i18n.py properly. So we don't have to call
gloab ourselves.
+ print "Checking for invalid i18n string..."
+ for f in files:
+ messages = load_i18n_module(f).messages
+ check_string_formatting(messages)
+ print "Checking for invalid i18n string successfully"
+
+
+if __name__ == '__main__':
+ main()
--
Thanks and best regards!
Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou(a)linux.vnet.ibm.com
Telephone: 86-10-82454397