[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: Dongsheng Song <dongsheng.song_at_gmail.com>
Date: Wed, 03 Jul 2013 14:26:25 +0800

On 2013/7/3 6:55, Konstantin Kolinko wrote:
> 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
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
     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. '
           '%d of %d strings are still untranslated (%.0f%%).' %
           (update_count, fuzzy, untranslated, string_count,
            100.0 * untranslated / string_count)))

Received on 2013-07-03 08:27:08 CEST

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