Skip to content

Support Qt in namespace#69

Merged
oKcerG merged 1 commit intooKcerG:masterfrom
RFCreations:master
Dec 20, 2019
Merged

Support Qt in namespace#69
oKcerG merged 1 commit intooKcerG:masterfrom
RFCreations:master

Conversation

@patstew
Copy link
Contributor

@patstew patstew commented Dec 20, 2019

SortFilterProxyModel doesn't currently support Qt built with the -qtnamespace configure option

@oKcerG oKcerG merged commit f839399 into oKcerG:master Dec 20, 2019
@oKcerG
Copy link
Owner

oKcerG commented Dec 20, 2019

Thank you for this PR

oKcerG added a commit that referenced this pull request Jan 6, 2020
@oKcerG
Revert "Support Qt in namespace (#69)"
1e98ca7 
The commit broke the build for non namespaced Qt
This reverts commit f839399.

closes #70
oKcerG added a commit that referenced this pull request Jan 6, 2020
@oKcerG
Revert "Support Qt in namespace (#69)"
787d614 
This reverts commit f839399.
@oKcerG oKcerG mentioned this pull request Jan 6, 2020
Merged
oKcerG added a commit that referenced this pull request Jan 6, 2020
@oKcerG
Revert "Support Qt in namespace (#69)" (#71)
35ffc48 
This reverts commit f839399.
@oKcerG
Copy link
Owner

oKcerG commented Jan 6, 2020

Sorry, I've rushed accepting this one.
It seems that the forward declaration is not working when using a non namespaced Qt. You have to use QT_FORWARD_DECLARE_CLASS instead.

But are you sure that there is not anymore thing to do to support a namespaced Qt? Shouldn't there be QT_USE_NAMESPACE or QT_PREPREND_NAMESPACE all over the place when referencing Qt classes?

@patstew
Copy link
Contributor Author

patstew commented Jan 7, 2020 •
edited
Loading

Apologies, my bad on QT_FORWARD_DECLARE_CLASS. I can add a new PR if you like?
I don't think anything else is necessary, it works for me like it is. QT_USE_NAMESPACE is declared in qglobal.h already, so it's declared if any Qt header is included.

patstew added a commit to RFCreations/SortFilterProxyModel that referenced this pull request Jan 7, 2020
@patstew
Support Qt in namespace (oKcerG#69)
1706bcb 
Enable support for Qt built with the -qtnamespace configure option
Do it properly this time with QT_FORWARD_DECLARE_CLASS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@patstew @oKcerG