In Qt 5.11 was introduced horizontalAdvance() to replace width(). In version 5.13, the old method is marked as obsolete.
In a far future, when the minimal Qt version will be updated (now 5.9), the old methods will be removed safely.
In Qt 5.11 was introduced horizontalAdvance() to replace width(). In version 5.13, the old method is marked as obsolete.
In a far future, when the minimal Qt version will be updated (now 5.9), the old methods will be removed safely.
Code must compile on Qt < 5.11 and Qt >= 5.11. No visual changes should be noticed.
Lint Skipped |
Unit Tests Skipped |
Isn't it be easier to use boundingRect(text).width() as a replacement of horizontalAdvance(text) which does not require Qt 5.11 and is not deprecated?
Thanks in advance for your answer.
@yurchor : I follwed the guidelines from the documentation (https://doc.qt.io/qt-5/qfontmetrics-obsolete.html#width); I haven't tried to see where the difference really is, I think there could be some pixel overlapped or some extra space.
I agree we can substitute the width method with the horizontalAdvance, however I propose we use compat.h to eliminate code duplication here.
Changed with macro, as suggested by @nmel; the code now is cleaner (and QFONTMETRICS_HORIZONTAL_ADVANCE seems a bit long)
I checked the macro expansion with gcc -E
Thanks Davide! It's cleaner and macro name is good. No issues with fonts were discovered when I tested.
krusader/compat.h | ||
---|---|---|
40 ↗ | (On Diff #62219) | (optional) I'd use QFontMetrics::horizontalAdvance(A) and QFontMetrics::width(A) for additional type check. |
(optional) I'd use QFontMetrics::horizontalAdvance(A) and QFontMetrics::width(A) for additional type check.
Even if those changes are not made, the new code works using Kubuntu 18.04, and improves the current situation. So thanks, Davide! (and Nikita and Yuri :-) )
An article about it has been published recently:
About deprecation of QFontMetrics::width() https://kdepepo.wordpress.com/2019/08/05/about-deprecation-of-qfontmetricswidth/
by @cfeck (Christoph Feck, who has commited some changes to the repository of Krusader).
I also read https://tsdgeos.blogspot.com/2019/07/beware-of-some-of-qt-513-deprecation.html by @aacid. That's why I wanted to wait to test more this patch. In D22504, for example, the changes were closer to a method rename.
While on my tests both methods work fine, I haven't found yet a font which should make the difference.
When I see the usage of the method, like
const float fontWidth = (fm.QFONTMETRICS_WIDTH("WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW") - fm.QFONTMETRICS_WIDTH("W")) / 99.0;
or
headerView->resizeSection(KrViewProperties::Ext, QFontMetrics(_viewFont).QFONTMETRICS_WIDTH("tar.bz2 ")); headerView->resizeSection(KrViewProperties::KrPermissions, QFontMetrics(_viewFont).QFONTMETRICS_WIDTH("rwx ")); headerView->resizeSection(KrViewProperties::Size, QFontMetrics(_viewFont).QFONTMETRICS_WIDTH("9") * 10);
which are all dirty hacks, I doubt we need to care too much about the differences mentioned. This code needs to be refactored properly but it's not a goal of this change. QFONTMETRICS_WIDTH will actually become a good marker of the places that need review if someone will find time and courage to make it right one day...