Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DirectEntry._autoCapitalize() on Python 3.x #628

Closed
wants to merge 8 commits into from

Conversation

@jwcotejr
Copy link
Contributor

commented Apr 18, 2019

This pull request aims to resolve the following crash with DirectEntry._autoCapitalize() under Python 3.x (Specifically Python 3.7.2 included with the current release of Panda3D 1.10.2, 64-bit, under Windows 10):

Traceback (most recent call last):
  File "_autoCapitalizeTest.py", line 8, in <module>
    base.run()
  File "C:\Panda3D-1.10.2-x64\direct\showbase\ShowBase.py", line 3109, in run
    self.taskMgr.run()
  File "C:\Panda3D-1.10.2-x64\direct\task\Task.py", line 531, in run
    self.step()
  File "C:\Panda3D-1.10.2-x64\direct\task\Task.py", line 485, in step
    self.mgr.poll()
  File "C:\Panda3D-1.10.2-x64\direct\showbase\EventManager.py", line 49, in eventLoopTask
    self.doEvents()
  File "C:\Panda3D-1.10.2-x64\direct\showbase\EventManager.py", line 43, in doEvents
    processFunc(dequeueFunc())
  File "C:\Panda3D-1.10.2-x64\direct\showbase\EventManager.py", line 99, in processEvent
    messenger.send(eventName, paramList)
  File "C:\Panda3D-1.10.2-x64\direct\showbase\Messenger.py", line 333, in send
    self.__dispatch(acceptorDict, event, sentArgs, foundWatch)
  File "C:\Panda3D-1.10.2-x64\direct\showbase\Messenger.py", line 418, in __dispatch
    result = method (*(extraArgs + sentArgs))
  File "C:\Panda3D-1.10.2-x64\direct\gui\DirectEntry.py", line 213, in _handleTyping
    self._autoCapitalize()
  File "C:\Panda3D-1.10.2-x64\direct\gui\DirectEntry.py", line 218, in _autoCapitalize
    name = self.get().decode('utf-8')
AttributeError: 'str' object has no attribute 'decode'

Two simple changes in the _autoCapitalize function were all that was needed to resolve this crash under Python 3.x, while also preserving compatibility with Python 2.7.

A simple unit test was also included to test this, and a simple test program can be found here: https://pastebin.com/P5sjhYQt

@jwcotejr jwcotejr marked this pull request as ready for review Apr 18, 2019

@rdb

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

I presume the original call was there for a reason; I guess if the case that there was a str object in Python 2 that had multi-byte UTF-8 sequences in it. Maybe you should include a UTF-8 string (without u prefix) in your test.

@jwcotejr

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

As in, use UTF-8 characters in a string, or just use any string that just so happens to be UTF-8 encoded?

@rdb

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

As in, use a string that has non-ASCII characters, in UTF-8 encoding. (If a string doesn't have non-ASCII characters, its UTF-8 encoded form is identical to the ASCII form, because of how UTF-8 is designed.)

jwcotejr added 3 commits May 20, 2019
tests: Update DirectEntry._autoCapitalize() unit test
It now tests DirectEntry._autoCapitalize() against a UTF-8 string.
@jwcotejr

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

@rdb Hi there, I have made the necessary changes you have requested. Sorry about the extended delay.

I am not entirely sure if this is 100% ideal, as I am setting the UTF-8 encoding within the unit test itself, nevertheless it does function as intended on both Python 2.x & Python 3.x.

Take care!

jwcotejr added 2 commits May 23, 2019
@NathanX-S

This comment has been minimized.

Copy link

commented Aug 6, 2019

Here I go again! Hopefully reviving @rdb on this one. A definite good push towards full Python 3 support. 👍

@Moguri Moguri added the directgui label Aug 6, 2019

@rdb rdb added this to the 1.10.4 milestone Aug 7, 2019

@rdb

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

Yeah, let's get this into the next release. I'd only additionally like to see tests with Unicode objects in Python 2, but I can make that change myself if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.