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

Use string representation for QKeySequence construction #601

Merged
merged 1 commit into from
May 5, 2024

Conversation

mborgerson
Copy link
Contributor

This fixes an issue in PySide6 v6.6.2 where key sequence construction with the | operator is broken, and improves readability.
Ideally this would be fixed in PySide6 or qtpy to retain original behavior, but since it also improves readability I think it makes sense to fix it here too.

Broken with PySide6 v6.6.2:

from qtpy import QtCore
QtCore.Qt.AltModifier | QtCore.Qt.Key.Key_C
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[2], line 1
----> 1 QtCore.Qt.AltModifier | QtCore.Qt.Key.Key_C

TypeError: unsupported operand type(s) for |: 'KeyboardModifier' and 'Key'

This fixes an issue in PySide6 v6.6.2 where key sequence construction
with the | operator is broken, and improves readability.
@mborgerson
Copy link
Contributor Author

Hi @ccordoba12, would you mind taking a look at this patch? The failing CI appears to be unrelated to the patch.

@ccordoba12 ccordoba12 added this to the 5.5.2 milestone May 5, 2024
@ccordoba12 ccordoba12 added the bug label May 5, 2024
Copy link
Collaborator

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks @mborgerson!

(Sorry for the delay in reviewing your work. This one just missed my radar).

@ccordoba12 ccordoba12 merged commit b7febdf into jupyter:main May 5, 2024
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants