Skip to content

Conversation

@fedelibre
Copy link
Member

If Qt version is below the required version (currently 6.6), the following error window will appear:

image

and the user will be forced to quit Frescobaldi.

@fedelibre fedelibre requested a review from jeanas May 4, 2025 22:02
Copy link
Collaborator

@bmjcode bmjcode left a comment

Choose a reason for hiding this comment

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

I think the idea here is good, but comparing the string values is an unreliable way to implement it. For example, the check would fail for a hypothetical Qt 6.10 because a string starting with "6.1" sorts before one starting with "6.6".

I would instead use a tuple in appinfo.py:

required_qt_version = (6, 6)

And write the check using QVersionInfo like so:

from PyQt6.QtCore import QLibraryInfo, QVersionNumber
v = QLibraryInfo.version()
r = QVersionNumber(*appinfo.required_qt_version)
if v < r:
    error("Qt version is too old.",
        f"Frescobaldi is started with Qt {v.toString()} \
but requires at least version {r.toString()}.")

This ensures the version is processed numerically as intended.

@fedelibre
Copy link
Member Author

I think the idea here is good, but comparing the string values is an unreliable way to implement it. For example, the check would fail for a hypothetical Qt 6.10 because a string starting with "6.1" sorts before one starting with "6.6".

Ooops, you are right:

>>> v="6.10"
>>> v>"6.6"
False

I'll follow your advice, thanks.

Copy link
Collaborator

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

@bmjcode bmjcode merged commit 0c6b8a3 into master May 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants