于 2013/7/3 18:11, Konstantin Kolinko 写道: > 2013/7/3 Dongsheng Song <dongsheng.s...@gmail.com>: >> On 2013/7/3 6:55, Konstantin Kolinko wrote: >>> 2013/7/3 Andreas Stieger <andreas.stie...@gmx.de>: >>>> Hi There, >>>> >>>> On 02/07/13 16:00, Dongsheng Song wrote: >>>>> 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" >>>>> >>>>> As your judgement, this is not "obvious fix", should I revert this >>>>> commit ? >>>> I cannot make sense of this change other than when malformed input files >>>> are concerned. Your example "^#~" requires msgstr == None == msgid as >>>> per the return of parse_translation(). That, then, means that comments >>>> evaluates to true (has entries) for the break in line 153 not to trigger. >>>> >>>> Can you give to input files (URl/revisions) that trigger this? So far >>>> this is my best guess: >>>> >>>> #SOMETHING >>>> #~ msgid "some msgid" >>>> #~ msgstr "some msgstr" >>>> >>>> I agree that msgstr == None should not be iterated, however I don't see >>>> how we get to this case. >>>> >>> Just noting: >>> The documentation string for "parse_translation(f)" function >>> explicitly documents what returned values can be None. The msgstr is >>> not one of them, it says "The msgstr is a list of strings.". >>> >>> But the actual implementation has one return statement that returns >>> None for that value. >>> >>> 39 arfrever 876651 > if line.strip() == '' or line[:2] == '#~': >>> 40 arfrever 874551 > return comments, None, None, None >>> >>> If you are going on with r1498947 then I think it would be better to >>> update the docstring. >>> >>> Alternatively, returning an empty array instead of the last 'None' >>> should be an other way to fix this issue. >>> >>> Best regards, >>> Konstantin Kolinko >> Yes, your noting looks more pretty, here is the patch: >> >> --- po-merge.py (revision 1499219) >> +++ po-merge.py (working copy) >> @@ -28,7 +28,7 @@ >> """Read a single translation entry from the file F and return a >> tuple with the comments, msgid, msgid_plural and msgstr. The comments >> is >> returned as a list of lines which do not end in new-lines. The msgid is >> - string. The msgid_plural is string or None. The msgstr is a list of >> + string or None. The msgid_plural is string or None. The msgstr is a >> list of > You fix the method to never return None here. Thus there is no need to > update the docstring.
No. The docstring which I fix is msgid, which maybe retrun None, but docstring not said. >> strings. The msgid, msgid_plural and msgstr strings can contain >> embedded >> newlines""" >> line = f.readline() >> @@ -37,7 +37,7 @@ >> comments = [] >> while True: >> if line.strip() == '' or line[:2] == '#~': >> - return comments, None, None, None >> + return comments, None, None, [] >> elif line[0] == '#': >> comments.append(line[:-1]) >> else: >> @@ -178,17 +178,16 @@ >> for i in msgstr: >> outfile.write('msgstr[%s] %s\n' % (n, msgstr[n])) >> n += 1 >> - if msgstr is not None: >> - for m in msgstr: >> - if m == '""': >> - untranslated += 1 >> + for m in msgstr: >> + if m == '""': >> + untranslated += 1 >> for c in comments: >> if c.startswith('#,') and 'fuzzy' in c.split(', '): >> fuzzy += 1 >> >> # We're done. Tell the user what we did. >> print(('%d strings updated. ' >> - '%d fuzzy strings. ' >> + '%d fuzzy strings. ' > What is the change in the above line? I do not see any difference from > this diff. Me too. Then I view as hex, I found it's a strip of tailing space. Regards, Dongsheng >> '%d of %d strings are still untranslated (%.0f%%).' % >> (update_count, fuzzy, untranslated, string_count, >> 100.0 * untranslated / string_count))) >> > Best regards, > Konstantin Kolinko
signature.asc
Description: OpenPGP digital signature