Fix Qt6 events #139
Fix Qt6 events #139
Conversation
WalkthroughThe recent updates across various widget files in the project primarily focus on improving type checking mechanisms. Instead of creating new instances or directly converting types, the changes advocate for the use of Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (4)
- magicclass/widgets/codeedit.py (1 hunks)
- magicclass/widgets/color.py (1 hunks)
- magicclass/widgets/colormap.py (1 hunks)
- magicclass/widgets/eval.py (1 hunks)
Additional comments: 3
magicclass/widgets/eval.py (1)
- 212-212: The addition of
assert isinstance(event, QtGui.QKeyEvent)ensures that the event being processed is indeed a key event. This is a good practice for type safety and clarity in the code. However, it's important to note that assertions can be disabled in Python when optimization is turned on (using the-Oflag). If the type check is critical for the program's correctness or security, consider using explicit runtime checks that cannot be disabled.magicclass/widgets/color.py (1)
- 80-80: The change to use
assert isinstance(event, QtGui.QHelpEvent)for type checking in theeventmethod is consistent with the approach taken in other files of this PR. It enhances readability and ensures type safety. However, as mentioned earlier, consider the implications of assertions being disabled in optimized runs of Python and whether a more robust type checking mechanism is needed based on the criticality of this check.magicclass/widgets/colormap.py (1)
- 328-328: The addition of
assert isinstance(a0, QtGui.QHelpEvent)in theeventmethod is a good practice for ensuring that the event being processed is of the expected type. This change aligns with the type checking improvements seen in other parts of the PR. Remember to consider the potential for assertions to be disabled in optimized Python runs and evaluate if this type check is critical to the application's functionality or security.
| @@ -140,7 +140,7 @@ def event(self, ev: QtCore.QEvent): | |||
| if ev.type() == QtCore.QEvent.Type.ToolTip: | |||
| return self._show_tooltip(ev) | |||
| elif ev.type() == QtCore.QEvent.Type.KeyPress: | |||
| ev = QtGui.QKeyEvent(ev) | |||
| assert isinstance(ev, QtGui.QKeyEvent) | |||
There was a problem hiding this comment.
The change from creating a new instance of QtGui.QKeyEvent for type checking to using an assert statement with isinstance is a positive change in terms of performance and directness. However, it's important to note that assert statements can be disabled in optimized runs of Python (using the -O flag), which might lead to this type check being skipped. Consider if this behavior is acceptable or if an alternative approach that remains active in optimized runs is preferable.
|
|
||
| def event(self, event: QtCore.QEvent): | ||
| if event.type() == QtCore.QEvent.Type.KeyPress: | ||
| event = QtGui.QKeyEvent(event) | ||
| assert isinstance(event, QtGui.QKeyEvent) | ||
| if event.key() == Qt.Key.Key_Tab: | ||
| self.show_completion() | ||
| return True |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [167-167]
The use of eval is detected in this file. While eval can be powerful, it also poses a significant security risk if it's used to evaluate dynamic content that could be influenced by external sources. Ensure that the content being evaluated cannot be manipulated by external inputs to prevent code injection vulnerabilities. Consider alternative approaches if the evaluated content can come from untrusted sources.
Also applies to: 301-301
This PR fixes bugs in Qt6, as Qt6 cannot use events to construct events.
Summary by CodeRabbit