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

Update DirectScrolledList string type checking #752

Closed
wants to merge 1 commit into from

Conversation

@Hawkheart
Copy link
Contributor

Hawkheart commented Oct 4, 2019

The previous implementation (which used type checking via __class__.__name__) was causing issues while porting my project from Python 2 to be Python 2/3 compatible.

I've replaced these calls; additionally, I've replaced the more acceptable type(item) == type('') checking to match other string-type-checks done in the direct.gui module.

@rdb

This comment has been minimized.

Copy link
Member

rdb commented Oct 4, 2019

Doesn't this break passing in a unicode object in Python 2? What kind of issues were you running into with the existing approach of checking against basestring?

@Hawkheart

This comment has been minimized.

Copy link
Contributor Author

Hawkheart commented Oct 4, 2019

The existing approach used in this file was not checking against basestring (it was checking type or class name for exact equality to str), so Unicode objects were already causing crashes due to accidental reparentTo calls.

I don't believe the current patch will work with bytes on Python 3, however, if that is required -- though the changes here are a change to match previous compatibility changes made to other DirectGUI modules (such as DirectFrame).

@rdb
rdb approved these changes Oct 5, 2019
Copy link
Member

rdb left a comment

Sorry, I read the diff backwards. This LGTM.

@rdb

This comment has been minimized.

Copy link
Member

rdb commented Oct 5, 2019

Cherry-picked as 984d70e. Thanks!

@rdb rdb closed this Oct 5, 2019
@rdb rdb added this to the 1.10.5 milestone Dec 31, 2019
@rdb rdb added bug directgui labels Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.