-
Notifications
You must be signed in to change notification settings - Fork 51
Python3 unicode - Fixing issue #24 -NameError: name 'unicode' is not defined #47
Conversation
The Python3 way, using unicode strings instead of bytes.
@RockyRoad29 Thanks for this. I wish I was able to provide useful feedback. Your changes look reasonable to me anyways! Hopefully someone else will step in and review your changes. We (the open source team) are looking for someone to take ownership of this plugin. Let me know (here or in the Gitter room for our main repo) if you're interested. Without an active maintainer, and not knowing enough Python to be able to judge, there's no one to merge this (or any other) pull request. |
Hi the LightTable team, Thank you very much for your comment Kenny. Sorry for being to long to reply, I've not been much on the computer It's a pity that nobody else tried to review the changes since. I'm pretty confident that my patch will fix things maybe beyond this Regards, On 19/01/16 23:23, kenny-evitt wrote:
|
@RockyRoad29 You can always run your own version of the Python plugin and you could also create your own plugin, that others could install in LT too, by forking this repo. That's probably a lot less work than committing to taking over this plugin. |
@kenny-evitt This PR already links to my fork ... here it is, available for all: |
@RockyRoad29 Thanks! Any other open PRs on this repo that you merged into your fork? |
I applied this in my install and it seems fine (I use pylint with other editors and enforce PEP8 in my code). |
@stemd It's your call now! If you think it's good, merge away. By-the-way, ping me, in an issue or in Gitter, when you're ready to release a new version of the plugin (so everyone can upgrade in LT – tho the in-LT upgrade is still broken! [LightTable/LightTable#2138]). |
Has this been merged? If not, what's stopping it? |
It has not been merged yet as this plugin is in need of a maintainer, @hellerbarde. Based on feedback here and comments on issue #54 I am comfortable with merging this PR and will do so. However, I am not particularly familiar with Python or use it on a daily basis so there is a potential cause for concern with cutting a release (and merging other Python plugin PRs). Until someone comes along to maintain the plugin (and make a release), you can pull down the repo and use it instead of the last proper plugin release. |
I have just tried this fix with Python 3.3.5 on my Win10 64-bit PC and it fixes the problem for me, |
Hi,
I read the few suggestions and fix proposals for bug #24, but here's my own approach,
feel free to comment.
So the problem is in
ltmain.py::ensureUtf()
which is used when callingcompile().
So if
compile()
expects unicode (let's ignore old-time latin),it is
bytes.decode()
that we'd need... and the source encoding mightdepend on the user's platform ?
Here's my proposal :
I tested it with python2.7.11 and python3.5.1 on my archlinux platform.
Then, if accepted it makes sense to rename the function to reflect its actual role,
which I did in a later commit.
... edit: Ooops, sorry, the rename was not complete, please check the last commit(s) from my branch.