On (02/02/15 13:32), Petr Viktorin wrote:
On 02/02/2015 09:08 AM, Lukas Slebodnik wrote:
[...]
>>>Patch 1/5: Note that even under Python 2, `except ValueError, KeyError:`
>>>would not work (it catches ValueError, and stores the exception instance
>>>under the name KeyError); please test these changes well.
>>>
>>It means that in python2 KeyError was not handled at all.
>>
>>python2 documentation says:
>>"If an exception occurs which does not match the exception named in the
except
>>clause, it is passed on to outer try statements; if no handler is found, it is
>>an unhandled exception and execution stops with a message as shown above."
>>
>>In my patch, you can see that different exception was thrown (ParsingError) in
>>handler. I'm not sure hot to test it therefore you are in CC ;-)
>>Shall I remove KeyError?
Yes, remove KeyError. Looking at the history, since 2009 the code worked
without catching KeyError (even if that wasn't the intention). I see no point
in adding it now.
Removed
Otherwise LGTM
>>
>>>Patch 4/5: you can use io.StringIO in both 2.6+ and 3.x+
>>>
>>Yes, but...
>>Traceback (most recent call last):
>> File "../src/sbus/sbus_codegen", line 931, in <module>
>> main()
>> File "../src/sbus/sbus_codegen", line 919, in main
>> generate_source(parser.parsed_interfaces, filename, options.include)
>> File "../src/sbus/sbus_codegen", line 600, in generate_source
>> out("/* The following definitions are auto-generated from %s */",
basename)
>> File "../src/sbus/sbus_codegen", line 217, in out
>> sys.stdout.write(str)
>>TypeError: unicode argument expected, got 'str'
>>
>>So I'm find with such solution for StringIO. The sbus_codegen is python
script
>>which generate C source file. I do not expect any unicode characters.
>>But I'm not opposed better version.
I see.
People could expect "io" to be the io module, so you could write
if sys.version_info[0] > 2:
from io import StringIO
else:
from StringIO import StringIO
...
sys.stdout = buf = StringIO()
Fixed
... but at this point that's just nitpicking.
>>
>>Thank you very much for review.
>>I squashed some small patches.
>>
>>LS
>
>I realized there were hardcoded python2 on some places. The 3rd patch should
>fix it. I used "env pyhton" for internal python scripts and for tests which
are
>not installed.
>
>I had a discussion with Pavel B. and we agreed
>that patch "sbus_codegen: Port to python3" needn't be push to upstream
yet.
>It would break his other patches. So current version would be used mostly by
>downstream.
Thank you very much for review.
Updated version is attached.
LS