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

PR: Add missing QtGui utility function to QtCore.Qt for PySide bindings #313

Merged
merged 6 commits into from
Jan 18, 2022

Conversation

dalthviz
Copy link
Member

@dalthviz dalthviz commented Jan 13, 2022

Fixes #311

Edit: Also fixes an issue with PyQt6 and sip to use mightBeRichText:

imagen

@dalthviz dalthviz added this to the v2.0.1 milestone Jan 13, 2022
@dalthviz dalthviz self-assigned this Jan 13, 2022
@dalthviz dalthviz changed the title PR: Add missing QtGui utility function to QtCore.Qt for PySide bindings PR [WIP]: Add missing QtGui utility function to QtCore.Qt for PySide bindings Jan 13, 2022
@dalthviz dalthviz changed the title PR [WIP]: Add missing QtGui utility function to QtCore.Qt for PySide bindings [WIP] PR: Add missing QtGui utility function to QtCore.Qt for PySide bindings Jan 13, 2022
@dalthviz dalthviz changed the title [WIP] PR: Add missing QtGui utility function to QtCore.Qt for PySide bindings PR: Add missing QtGui utility function to QtCore.Qt for PySide bindings Jan 13, 2022
@dalthviz dalthviz force-pushed the fixes_issue_311 branch 3 times, most recently from 9542944 to 9de93c6 Compare January 13, 2022 17:11
Copy link
Member

@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.

Thanks @dalthviz for this! I left two small comments, otherwise looks good to me.

qtpy/QtCore.py Show resolved Hide resolved
qtpy/QtCore.py Show resolved Hide resolved
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Thanks @dalthviz ! Looks like @ccordoba12 's suggestions were implemented, so this is ready to go with a couple trivial comments

.github/workflows/ci.yml Outdated Show resolved Hide resolved
qtpy/QtCore.py Outdated Show resolved Hide resolved
dalthviz and others added 2 commits January 17, 2022 09:33
Improve comment punctuation/caps

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
Copy link
Member

@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 now, thanks @dalthviz!

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks @dalthviz !

@CAM-Gerlach
Copy link
Member

Merging so we can fix the tests in the other PRs.

@CAM-Gerlach CAM-Gerlach merged commit 3d17033 into spyder-ide:master Jan 18, 2022
# trying to import `PyQt6.QtGui.Qt`, some functions like
# `PyQt6.QtCore.Qt.mightBeRichText` are missing.
try:
from PyQt6.QtGui import Qt
Copy link
Contributor

@StSav012 StSav012 Nov 1, 2023

Choose a reason for hiding this comment

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

Just out of curiosity, shouldn't this overwrite the Qt imported from PyQt6.QtCore a bit earlier?

Copy link
Contributor

@StSav012 StSav012 left a comment

Choose a reason for hiding this comment

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

I've stumbled upon the issue #311 while looking through the comments in the qtpy.QtCore code. Although it's merged ages ago, I'd like to discuss the fix a little.

What versions of PyQt6, PySide2, and PySide6 require the fix? I can't tell whether the code is long obsolete or still needed. Please clarify this somewhere.

@dalthviz
Copy link
Member Author

dalthviz commented Nov 1, 2023

What versions of PyQt6, PySide2, and PySide6 require the fix? I can't tell whether the code is long obsolete or still needed. Please clarify this somewhere.

Due the dates the issue was raised and fixed I would guess that is needed for Qt 6.1 or 6.2 bindings (although not completely sure 🤔). Also, did a quick check using PySide2 5.15.2.1 and I got this:

image

So the fix is at least required by PySide 5.15.2.1. Could you give it a try to the Qt 6.2 bindings to check if the fix is still necessary there?

@StSav012
Copy link
Contributor

StSav012 commented Nov 2, 2023

So, the tests I've conducted gave the following results:

Flavor QtCore.qVersion() hasattr(QtCore.Qt, "mightBeRichText") hasattr(QtGui.Qt, "mightBeRichText"))
PySide2 5.13.2, 5.15.2 False True
PySide6 6.2.4, 6.5.3, 6.6.0 False True
PyQt5 5.13.2, 5.15.2 True 'PyQt5.QtGui' has no attribute 'Qt'
PyQt6 6.2.4, 6.5.3, 6.6.0 True 'PyQt6.QtGui' has no attribute 'Qt'

So, the fix is required for all current PySide versions, but shouldn't be here for PyQt6.

But that's not all.

Flavor set(dir(QtGui.Qt)) - set(dir(QtCore.Qt))
PySide2 {'convertFromPlainText', 'codecForHtml', 'mightBeRichText'}
PySide6 {'convertFromPlainText', 'mightBeRichText'}

Shouldn't convertFromPlainText be defined for PyQt5 and PyQt6 then, too?

def convertFromPlainText(plain: str, mode: Qt.WhiteSpaceMode = Qt.WhiteSpaceMode.WhiteSpacePre) -> str:
    import html

    plain = html.escape(plain)
    if mode == Qt.WhiteSpaceMode.WhiteSpacePre:
        plain = plain.replace(" ", "\xa0").replace("\t", "\xa0" * 4)
    return "<p>" + plain + "</p>"

@dalthviz
Copy link
Member Author

dalthviz commented Nov 3, 2023

Thanks for the new info @StSav012 ! Could you open a new issue with your findings? Seems like we need to work in a couple of things indeed

@StSav012
Copy link
Contributor

StSav012 commented Nov 7, 2023

I'm unsure whether it's worth it. I've run [a for a in dir(QtCore.Qt) if a[0].islower()] (to filter the constants and classes out) for various flavors.

Flavor [a for a in dir(QtCore.Qt) if a[0].islower()]
PyQt5-Qt5==5.15.2 ['black', 'blue', 'color0', 'color1', 'cyan', 'darkBlue', 'darkCyan', 'darkGray', 'darkGreen', 'darkMagenta', 'darkRed', 'darkYellow', 'gray', 'green', 'lightGray', 'magenta', 'red', 'transparent', 'white', 'yellow']
PySide2==5.13.2 ['black', 'blue', 'color0', 'color1', 'cyan', 'darkBlue', 'darkCyan', 'darkGray', 'darkGreen', 'darkMagenta', 'darkRed', 'darkYellow', 'gray', 'green', 'lightGray', 'magenta', 'red', 'transparent', 'white', 'yellow']
PyQt6-Qt6==6.6.0 ['bin', 'bom', 'center', 'dec', 'endl', 'fixed', 'flush', 'forcepoint', 'forcesign', 'hex', 'left', 'lowercasebase', 'lowercasedigits', 'noforcepoint', 'noforcesign', 'noshowbase', 'oct', 'reset', 'right', 'scientific', 'showbase', 'uppercasebase', 'uppercasedigits', 'ws']
PySide6-Essentials==6.6.0 ['beginPropertyUpdateGroup', 'bin', 'bom', 'center', 'dec', 'endPropertyUpdateGroup', 'endl', 'fixed', 'flush', 'forcepoint', 'forcesign', 'hex', 'left', 'lowercasebase', 'lowercasedigits', 'noforcepoint', 'noforcesign', 'noshowbase', 'oct', 'reset', 'right', 'scientific', 'showbase', 'uppercasebase', 'uppercasedigits', 'ws']

All but the two first ones differ. Does anyone need the functions to spend time porting them?

As for convertFromPlainText, it's nowhere to be found. Weirdly. Not even in PyQt*.QtCore. So, the flavors are compatible in this sense.

@dalthviz
Copy link
Member Author

All but the two first ones differ. Does anyone need the functions to spend time porting them?

I guess for the moment no since no issue has been opened regarding that but it's good to know the variations that we could potentially handle

And thanks for opening the issue!

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.

QtCore.Qt.mightBeRichText undefined in PySide
4 participants