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

[qt5] QMetaType fix for Qt5 #6570

Merged
merged 1 commit into from
Dec 25, 2024

Conversation

dyfer
Copy link
Member

@dyfer dyfer commented Dec 12, 2024

Purpose and Motivation

In the process of reviewing #6521 it turned out that the issue in #6508 was already present with Qt 5.15 after adding f70364c. This PR brings back old behavior at the cost of four extra ifdefs in the codebase, reverting that change for Qt5. See discussion in #6521 (comment).

Thanks @xunil-cloud for the tip.

There might be a better way to address this issue, but I have no expertise to pursue that. I think the ifdef is a quick and easy fix, and will be removed when we remove Qt5 support altogether.

Fixes #6508 for Qt 5.15 (macOS legacy build).

Code that was previously failing for reference:

(
t = TreeView().front;
t.addItem(["hi"]);
)

Types of changes

  • Bug fix

To-do list

  • Code is tested
  • All tests are passing
  • n/a Updated documentation
  • This PR is ready for review

@dyfer dyfer added the qt5 label Dec 12, 2024
@capital-G capital-G added the comp: Qt GUI sclang Qt components -- for IDE tickets, use "env: SCIDE" instead label Dec 17, 2024
Copy link
Contributor

@capital-G capital-G 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 this is fine.

If this would be more permanent we could declare a new type alias via using for this a global fix in a compat.cpp.
But as all introduced changes are retrievable and revertable by searching for if (QT_VERSION < QT_VERSION_CHECK(6, 0, 0)) it seems fine as is.

Thanks for taking care of this! Maybe wait a week for an additional review and if no one else gives this a review/remark we can merge this?

@capital-G capital-G merged commit 02c7e68 into supercollider:develop Dec 25, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: Qt GUI sclang Qt components -- for IDE tickets, use "env: SCIDE" instead qt5
Projects
Development

Successfully merging this pull request may close these issues.

can't add items to TreeView
2 participants