Dongsheng Song wrote on Tue, Jul 02, 2013 at 23:00:59 +0800: > 于 2013/7/2 22:30, Daniel Shahaf 写道: > >dongsh...@apache.org wrote on Tue, Jul 02, 2013 at 14:12:44 -0000: > >>Author: dongsheng > >>Date: Tue Jul 2 14:12:44 2013 > >>New Revision: 1498947 > >> > >>URL: http://svn.apache.org/r1498947 > >>Log: > >>Make sure msgstr is not None before do iteration. > >> > >>Obvious fix. > >> > >No, it's not an obvious fix. It's a code change, and one which is not > >obvious, so it requires review. > > > >> Since parse_translation maybe return (comments, None, None, None), > >When does that happen? Why is that not a bug? Why no translator run > >into it until today? > Today, when I merge zh_CN.po from trunk to 1.8.x, I had encounter > the following error: > > $ python ../../../../trunk/tools/dev/po-merge.py < > ../../../../trunk/subversion/po/zh_CN.po zh_CN.po > Traceback (most recent call last): > File "../../../../trunk/tools/dev/po-merge.py", line 196, in <module> > main(sys.argv) > File "../../../../trunk/tools/dev/po-merge.py", line 181, in main > for m in msgstr: > TypeError: 'NoneType' object is not iterable > > Then I found in the line 39-40 of po-merge.py return None as msgstr: > > if line.strip() == '' or line[:2] == '#~': > return comments, None, None, None > > So we should not do iteration on msgstr without make sure msgstr is > not None. > > This happened because zh_CN.po have msgmerged po comments like this: > > #~ msgid "Uncommitted local addition, copy or move%s" > #~ msgstr "未提交的本地增加，复制或移动 %s" >
Thanks. That explains why msgstr was None. But I still don't understand why ignoring the for loop is the correct fix. > As your judgement, this is not "obvious fix", should I revert this commit ? It'd be simpler if someone looked at the commit and gave it his +1. Daniel > > >Thanks, > > > >Daniel > > > >>* tools/dev/po-merge.py > >> we should not do iteration without check msgstr is not None. Otherwise, > >> we may encounter the following TypeError: > >> > >> Traceback (most recent call last): > >> File "../../../../trunk/tools/dev/po-merge.py", line 196, in <module> > >> main(sys.argv) > >> File "../../../../trunk/tools/dev/po-merge.py", line 181, in main > >> for m in msgstr: > >> TypeError: 'NoneType' object is not iterable > >> > >>Modified: > >> subversion/trunk/tools/dev/po-merge.py > >> > >>Modified: subversion/trunk/tools/dev/po-merge.py > >>URL: > >>http://svn.apache.org/viewvc/subversion/trunk/tools/dev/po-merge.py?rev=1498947&r1=1498946&r2=1498947&view=diff > >>============================================================================== > >>--- subversion/trunk/tools/dev/po-merge.py (original) > >>+++ subversion/trunk/tools/dev/po-merge.py Tue Jul 2 14:12:44 2013 > >>@@ -178,9 +178,10 @@ def main(argv): > >> for i in msgstr: > >> outfile.write('msgstr[%s] %s\n' % (n, msgstr[n])) > >> n += 1 > >>- for m in msgstr: > >>- if m == '""': > >>- untranslated += 1 > >>+ if msgstr is not None: > >>+ for m in msgstr: > >>+ if m == '""': > >>+ untranslated += 1 > >> for c in comments: > >> if c.startswith('#,') and 'fuzzy' in c.split(', '): > >> fuzzy += 1 > >> > >> >
