Skip to content

Update to python 3.7 and latest nuklear#1

Open
creikey wants to merge 8 commits into
nathanrw:masterfrom
creikey:master
Open

Update to python 3.7 and latest nuklear#1
creikey wants to merge 8 commits into
nathanrw:masterfrom
creikey:master

Conversation

@creikey

@creikey creikey commented Jan 13, 2019

Copy link
Copy Markdown

Everything except the overview translated to python seems to work
perfectly. The biggest change between versions was that now with
every string on cffi you have to do "test".encode('utf-8') instead
of just using a string in a cffi function like "test". I have not
found a way to mitigate this issue so far. This change in encoding
practice with cffi is the source of almost all breaking in
overview.py. I used a vim regex command to add encode('utf-8') to
all strings that aren't doc strings, but there are several issues
still present and undiscovered, and can only be discovered by
running overview and pressing on random buttons.

Everything except the overview translated to python seems to work
perfectly. The biggest change between versions was that now with
every string on cffi you have to do "test".encode('utf-8') instead
of just using a string in a cffi function like "test". I have not
found a way to mitigate this issue so far. This change in encoding
practice with cffi is the source of almost all braking in
overview.py. I used a vim regex command to add encode('utf-8') to
all strings that aren't doc strings, but there are several issues
still present and undiscovered, and can only be discovered by
running overview and pressing on random buttons.
@creikey

creikey commented Jan 13, 2019

Copy link
Copy Markdown
Author

Seems not to be building on travis...

@creikey

creikey commented Jan 14, 2019

Copy link
Copy Markdown
Author

Ok everything seems to be building now, had to remove some stuff with sed in a makefile

@creikey

creikey commented Jan 17, 2019

Copy link
Copy Markdown
Author

So it turns out that you can use b"my string" instead of "my string".encode('utf-8'). This should make development a little easier with the new changes

@nathanrw

Copy link
Copy Markdown
Owner

Hi,

Thanks for contributing this! Python 3 support would be fantastic.

I've not got a vast amount of free time at the moment so it might be a little while before i can look at merging your changes.

A few things stick out at me that probably mean i cant merge it in its current form

  • upgrading nuklear and python 3 support are 2 different things, so we should do 1 at a time. Python 3 support is less likely to cause downstream breakage if done correctly, whereas a major library upgrade could do who knows what.
  • we should endeavour to retain python 2 support as well as support python 3.
  • would like to avoid dependency on unix commandline tools (the fact that the preprocessed header is committed to source control is really a bodge and id prefer to avoid relying on it)
  • its best to avoid making unnecessary changes e.g. reformatting code without otherwise changing it

I'll try to find the time to take a closer look as soon as i can.

Thanks

Nathan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants