于 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


Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to