[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1498947 - /subversion/trunk/tools/dev/po-merge.py

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Tue, 2 Jul 2013 20:16:15 +0300

Dongsheng Song wrote on Tue, Jul 02, 2013 at 23:00:59 +0800:
> 于 2013/7/2 22:30, Daniel Shahaf 写道:
> >dongsheng_at_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
> >>
> >>
>
Received on 2013-07-02 19:16:53 CEST

This is an archived mail posted to the Subversion Dev mailing list.