fix early crash on python3 + PyQt5 #589

Merged
merged 2 commits into from Aug 22, 2016

Conversation

Projects
None yet
2 participants
@sthalik
Contributor

sthalik commented Aug 21, 2016

Must pass "bytes" data type to QByteArray in PyQt5.

sthalik added some commits Aug 21, 2016

on PyQt5 QByteArray expects bytes, not str
Check for Python 3.0 shouldn't be necessary since PyQt5 is python3-only.

Signed-off-by: Stanislaw Halik <sthalik@misaki.pl>
with Py3 bytes ctor for str input needs an encoding
Skip the check at runtime if Python3 isn't found. The Python 2 bytes() -
aka str() ctor works fine without encoding.
@@ -14,7 +14,12 @@
try:
# pylint: disable=bytes-builtin
- bstr = bytes
+ bytes()

This comment has been minimized.

@davvid

davvid Aug 22, 2016

Member

this should work sans the ().... could even do b = bytes

@davvid

davvid Aug 22, 2016

Member

this should work sans the ().... could even do b = bytes

+ bytes()
+ if PY3:
+ def bstr(x):
+ return bytes(x, encoding='latin1')

This comment has been minimized.

@davvid

davvid Aug 22, 2016

Member

this is starting to look suspiciously like core.encode(). Shouldn't it be utf8?

@davvid

davvid Aug 22, 2016

Member

this is starting to look suspiciously like core.encode(). Shouldn't it be utf8?

This comment has been minimized.

@sthalik

sthalik Aug 22, 2016

Contributor

Is it for human-readable strings? If so, definitely.

@sthalik

sthalik Aug 22, 2016

Contributor

Is it for human-readable strings? If so, definitely.

@@ -128,9 +129,12 @@ def apply_state(self, state):
if windowstate is None:
result = False
else:
+ windowstate = str(windowstate)
+ if qtpy.PYQT5:
+ windowstate = str.encode(windowstate, encoding='us-ascii')

This comment has been minimized.

@davvid

davvid Aug 22, 2016

Member

this should probably use core.encode(...)

@davvid

davvid Aug 22, 2016

Member

this should probably use core.encode(...)

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Aug 22, 2016

Member

Notes aside, i think I'll take this PR as-is and see what I can tweak on my end. Thanks for the fix!

Member

davvid commented Aug 22, 2016

Notes aside, i think I'll take this PR as-is and see what I can tweak on my end. Thanks for the fix!

@davvid davvid merged commit f8cdf8a into git-cola:master Aug 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

davvid added a commit to davvid/git-cola that referenced this pull request Aug 22, 2016

compat: rework how `bstr()` is defined
Arrange for `bstr()` to be defined as a function for Python 3 only.
Avoid exception by checking the Python version explicitly.

Set `bstr = bytes` for modern Python2 and use `str` for historical
Pythons.

Related-to: #589
Signed-off-by: David Aguilar <davvid@gmail.com>

davvid added a commit to davvid/git-cola that referenced this pull request Aug 22, 2016

widgets: always encode windowstate
QByteArray on PyQt5 only accepts byte strings.  QByteArray on PyQt4
accepts byte strings too, so simplify the logic by encoding to a byte
string unconditionally.

Related-to: #589
Signed-off-by: David Aguilar <davvid@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment