Differential D8075

Fix build with Qt 5.10
ClosedPublic
Actions

Authored by fvogt on Sep 30 2017, 2:08 PM.

Details

Reviewers
cfeck
Group Reviewers
Dolphin
KDE Applications
Commits
R318:c96efc7ccba6: Fix build with Qt 5.10
Summary

QStaticText's constructor with const QString & as argument is now explicit.

Test Plan

Did not build before, now does.

Note that this might change the behaviour as the other properties of m_text are now
kept, which was not the case before. I'm not sure what the expected behaviour here
is supposed to be.

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
fvogt created this revision.Sep 30 2017, 2:08 PM
cfeck accepted this revision.Sep 30 2017, 2:11 PM
cfeck added a subscriber: cfeck.

I had expected a clear(), but there is none.

This revision is now accepted and ready to land.Sep 30 2017, 2:11 PM
cfeck added a comment.Sep 30 2017, 2:14 PM

... But, the docs (both 5.10 and dev) doesn't say the constructor is explicit ... hm

fvogt added a comment.Sep 30 2017, 2:23 PM
In D8075#150908, @cfeck wrote:

... But, the docs (both 5.10 and dev) doesn't say the constructor is explicit ... hm

https://github.com/qt/qtbase/commit/49c4c061e0cd9cbbae0c911a81376f6820ce1f1e#diff-83cd6e61ac66462c2d5e44d09c78d4ad

Might be worth a bug report.

Closed by commit R318:c96efc7ccba6: Fix build with Qt 5.10 (authored by fvogt). · Explain WhySep 30 2017, 2:29 PM
This revision was automatically updated to reflect the committed changes.
elvisangelaccio added a subscriber: elvisangelaccio.Sep 30 2017, 2:37 PM
elvisangelaccio added inline comments.
src/kitemviews/kstandarditemlistgroupheader.cpp
104

Why not m_text = QStaticText(); ?

fvogt added inline comments.Sep 30 2017, 2:40 PM
src/kitemviews/kstandarditemlistgroupheader.cpp
104

See the diff description:

Note that this might change the behaviour as the other properties of m_text are now
kept, which was not the case before. I'm not sure what the expected behaviour here
is supposed to be.

elvisangelaccio added inline comments.Sep 30 2017, 2:50 PM
src/kitemviews/kstandarditemlistgroupheader.cpp
104

Right, the new behaviour might be better, indeed. (I don't see why we should reset the tex format and performance hint properties, just because we want to clear the text property).

apol added a subscriber: apol.Oct 2 2017, 11:38 AM
In D8075#150909, @fvogt wrote:
In D8075#150908, @cfeck wrote:

... But, the docs (both 5.10 and dev) doesn't say the constructor is explicit ... hm

https://github.com/qt/qtbase/commit/49c4c061e0cd9cbbae0c911a81376f6820ce1f1e#diff-83cd6e61ac66462c2d5e44d09c78d4ad

Might be worth a bug report.

A patch would possibly be faster and easier than a bug report. ;)

fvogt added a comment.Oct 2 2017, 11:52 AM
In D8075#151557, @apol wrote:
In D8075#150909, @fvogt wrote:
In D8075#150908, @cfeck wrote:

... But, the docs (both 5.10 and dev) doesn't say the constructor is explicit ... hm

https://github.com/qt/qtbase/commit/49c4c061e0cd9cbbae0c911a81376f6820ce1f1e#diff-83cd6e61ac66462c2d5e44d09c78d4ad

Might be worth a bug report.

A patch would possibly be faster and easier than a bug report. ;)

Cases such as https://codereview.qt-project.org/#/c/198123/ show the opposite...

Revision Contents
Changeset List

Diff 20159

View Options

src/kitemviews/kstandarditemlistgroupheader.cpp

Loading...