[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: Konstantin Kolinko <knst.kolinko_at_gmail.com>
Date: Wed, 3 Jul 2013 02:55:21 +0400

2013/7/3 Andreas Stieger <andreas.stieger_at_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
Received on 2013-07-03 00:55:53 CEST

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