-
Notifications
You must be signed in to change notification settings - Fork 36.2k
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
qt: Replace functions deprecated in Qt 5.13 #16701
Conversation
In Qt 5.12 and before the QFontMetrics::width() is used and it is deprecated since Qt 13.0. In Qt 5.11 the QFontMetrics::horizontalAdvance() was introduced.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Concept ACK, saw these warnings just now while building with Qt 5.13.0 on macOS. |
@@ -956,4 +956,13 @@ void PolishProgressDialog(QProgressDialog* dialog) | |||
#endif | |||
} | |||
|
|||
int TextWidth(const QFontMetrics& fm, const QString& text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it better to use a deprecated function instead of implementing workarounds? IMHO deprecated stuff can be removed only when it's 1-to-1. So in this case this change sounds premature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO deprecated stuff can be removed only when it's 1-to-1.
Unfortunately, Qt's "support version window" (for the mentioned feature), i.e., 5.11 to 5.13, is narrower than ours, i.e., 5.5.1 to 5.13.
68e2d76
to
c6dd32d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK c6dd32d
Abstracting it into our helper layer seems a good approach.
I think qt is over-zealous here with renaming, but that's not our fault, utACK c6dd32d |
c6dd32d qt: Replace obsolete functions of QDesktopWidget (Hennadii Stepanov) 1260ecd qt: Add TextWidth() wrapper (Hennadii Stepanov) Pull request description: The following functions are obsolete in Qt 5.13: - [`QFontMetrics::width()`](https://doc.qt.io/qt-5/qfontmetrics-obsolete.html#width) - [`QDesktopWidget::availableGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#availableGeometry) - [`QDesktopWidget::screenGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#screenGeometry) This PR replaces them and does not change behavior. Here are some excerpts from the master build log: ``` qt/bitcoingui.cpp: In constructor ‘BitcoinGUI::BitcoinGUI(interfaces::Node&, const PlatformStyle*, const NetworkStyle*, QWidget*)’: qt/bitcoingui.cpp:84:57: warning: ‘const QRect QDesktopWidget::availableGeometry(int) const’ is deprecated: Use QGuiApplication::screens() [-Wdeprecated-declarations] move(QApplication::desktop()->availableGeometry().center() - frameGeometry().center()); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QDesktopWidget:1:0, from qt/bitcoingui.cpp:43: /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qdesktopwidget.h:88:67: note: declared here QT_DEPRECATED_X("Use QGuiApplication::screens()") const QRect availableGeometry(int screen = -1) const; ^~~~~~~~~~~~~~~~~ ``` ``` qt/bitcoingui.cpp:1410:74: warning: ‘int QFontMetrics::width(const QString&, int) const’ is deprecated: Use QFontMetrics::horizontalAdvance [-Wdeprecated-declarations] max_width = qMax(max_width, fm.width(BitcoinUnits::longName(unit))); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qwidget.h:50:0, from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qdialog.h:44, from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QDialog:1, from ./qt/optionsdialog.h:8, from ./qt/bitcoingui.h:12, from qt/bitcoingui.cpp:5: /home/hebasto/Qt/5.13.0/gcc_64/include/QtGui/qfontmetrics.h:108:9: note: declared here int width(const QString &, int len = -1) const; ^~~~~ ``` ``` qt/splashscreen.cpp: In constructor ‘SplashScreen::SplashScreen(interfaces::Node&, Qt::WindowFlags, const NetworkStyle*)’: qt/splashscreen.cpp:127:50: warning: ‘const QRect QDesktopWidget::screenGeometry(int) const’ is deprecated: Use QGuiApplication::screens() [-Wdeprecated-declarations] move(QApplication::desktop()->screenGeometry().center() - r.center()); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QDesktopWidget:1:0, from qt/splashscreen.cpp:24: /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qdesktopwidget.h:79:67: note: declared here QT_DEPRECATED_X("Use QGuiApplication::screens()") const QRect screenGeometry(int screen = -1) const; ^~~~~~~~~~~~~~ ``` ACKs for top commit: jonasschnelli: utACK c6dd32d Tree-SHA512: deb7bcbf86e1dcc6508bd91288772c2fe8811db79fa2011de37d0469cdd094fbf7fd8c4512c607bed0bd08dc2968e893c0bbc190732c43c69ed1085259df766c
c6dd32d qt: Replace obsolete functions of QDesktopWidget (Hennadii Stepanov) 1260ecd qt: Add TextWidth() wrapper (Hennadii Stepanov) Pull request description: The following functions are obsolete in Qt 5.13: - [`QFontMetrics::width()`](https://doc.qt.io/qt-5/qfontmetrics-obsolete.html#width) - [`QDesktopWidget::availableGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#availableGeometry) - [`QDesktopWidget::screenGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#screenGeometry) This PR replaces them and does not change behavior. Here are some excerpts from the master build log: ``` qt/bitcoingui.cpp: In constructor ‘BitcoinGUI::BitcoinGUI(interfaces::Node&, const PlatformStyle*, const NetworkStyle*, QWidget*)’: qt/bitcoingui.cpp:84:57: warning: ‘const QRect QDesktopWidget::availableGeometry(int) const’ is deprecated: Use QGuiApplication::screens() [-Wdeprecated-declarations] move(QApplication::desktop()->availableGeometry().center() - frameGeometry().center()); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QDesktopWidget:1:0, from qt/bitcoingui.cpp:43: /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qdesktopwidget.h:88:67: note: declared here QT_DEPRECATED_X("Use QGuiApplication::screens()") const QRect availableGeometry(int screen = -1) const; ^~~~~~~~~~~~~~~~~ ``` ``` qt/bitcoingui.cpp:1410:74: warning: ‘int QFontMetrics::width(const QString&, int) const’ is deprecated: Use QFontMetrics::horizontalAdvance [-Wdeprecated-declarations] max_width = qMax(max_width, fm.width(BitcoinUnits::longName(unit))); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qwidget.h:50:0, from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qdialog.h:44, from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QDialog:1, from ./qt/optionsdialog.h:8, from ./qt/bitcoingui.h:12, from qt/bitcoingui.cpp:5: /home/hebasto/Qt/5.13.0/gcc_64/include/QtGui/qfontmetrics.h:108:9: note: declared here int width(const QString &, int len = -1) const; ^~~~~ ``` ``` qt/splashscreen.cpp: In constructor ‘SplashScreen::SplashScreen(interfaces::Node&, Qt::WindowFlags, const NetworkStyle*)’: qt/splashscreen.cpp:127:50: warning: ‘const QRect QDesktopWidget::screenGeometry(int) const’ is deprecated: Use QGuiApplication::screens() [-Wdeprecated-declarations] move(QApplication::desktop()->screenGeometry().center() - r.center()); ^ In file included from /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/QDesktopWidget:1:0, from qt/splashscreen.cpp:24: /home/hebasto/Qt/5.13.0/gcc_64/include/QtWidgets/qdesktopwidget.h:79:67: note: declared here QT_DEPRECATED_X("Use QGuiApplication::screens()") const QRect screenGeometry(int screen = -1) const; ^~~~~~~~~~~~~~ ``` ACKs for top commit: jonasschnelli: utACK c6dd32d Tree-SHA512: deb7bcbf86e1dcc6508bd91288772c2fe8811db79fa2011de37d0469cdd094fbf7fd8c4512c607bed0bd08dc2968e893c0bbc190732c43c69ed1085259df766c
8b6f5aa qt: Replace QFontMetrics::width() with TextWidth() (Hennadii Stepanov) Pull request description: Compiling master (d8fc997) on macOS Catalina (with a patch from #16720) reveals one more instance of `QFontMetrics::width()` which is supposed to be replaced with `TextWidth()` in the merged #16701. Sorry for incomplete solution provided in #16701. It’s especially sad that the line I missed lies in only 7 lines from the code touched in #16701. ACKs for top commit: fanquake: ACK 8b6f5aa Tree-SHA512: 65cd8bea550150e5ee47c1e906d8c2393547cf4feba3701a933a4f24fad5ecdb552ac2de4e1200ed14efaa0df0480150dd58fccbddc3b902f6c2141603874902
Summary: c6dd32da697e5a8052cbabe8c7605d27c43a8dfb qt: Replace obsolete functions of QDesktopWidget (Hennadii Stepanov) 1260ecd812e35185898fd555ad3e01d019072bcf qt: Add TextWidth() wrapper (Hennadii Stepanov) Pull request description: The following functions are obsolete in Qt 5.13: - [`QFontMetrics::width()`](https://doc.qt.io/qt-5/qfontmetrics-obsolete.html#width) - [`QDesktopWidget::availableGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#availableGeometry) - [`QDesktopWidget::screenGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#screenGeometry) This PR replaces them and does not change behavior. --- This is a backport from Core [[bitcoin/bitcoin#16701 | PR16701]] Test Plan: ninja check run bitcoin-qt and click around to see everything is sane Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5369
Backport from ABC commit: Bitcoin-ABC@93934d3 --- Summary: c6dd32da697e5a8052cbabe8c7605d27c43a8dfb qt: Replace obsolete functions of QDesktopWidget (Hennadii Stepanov) 1260ecd812e35185898fd555ad3e01d019072bcf qt: Add TextWidth() wrapper (Hennadii Stepanov) Pull request description: The following functions are obsolete in Qt 5.13: - [`QFontMetrics::width()`](https://doc.qt.io/qt-5/qfontmetrics-obsolete.html#width) - [`QDesktopWidget::availableGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#availableGeometry) - [`QDesktopWidget::screenGeometry()`](https://doc.qt.io/qt-5/qdesktopwidget-obsolete.html#screenGeometry) This PR replaces them and does not change behavior. --- This is a backport from Core [[bitcoin/bitcoin#16701 | PR16701]] Test Plan: ninja check run bitcoin-qt and click around to see everything is sane Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5369
8b6f5aa qt: Replace QFontMetrics::width() with TextWidth() (Hennadii Stepanov) Pull request description: Compiling master (d8fc997) on macOS Catalina (with a patch from bitcoin#16720) reveals one more instance of `QFontMetrics::width()` which is supposed to be replaced with `TextWidth()` in the merged bitcoin#16701. Sorry for incomplete solution provided in bitcoin#16701. It’s especially sad that the line I missed lies in only 7 lines from the code touched in bitcoin#16701. ACKs for top commit: fanquake: ACK 8b6f5aa Tree-SHA512: 65cd8bea550150e5ee47c1e906d8c2393547cf4feba3701a933a4f24fad5ecdb552ac2de4e1200ed14efaa0df0480150dd58fccbddc3b902f6c2141603874902
8b6f5aa qt: Replace QFontMetrics::width() with TextWidth() (Hennadii Stepanov) Pull request description: Compiling master (d8fc997) on macOS Catalina (with a patch from bitcoin#16720) reveals one more instance of `QFontMetrics::width()` which is supposed to be replaced with `TextWidth()` in the merged bitcoin#16701. Sorry for incomplete solution provided in bitcoin#16701. It’s especially sad that the line I missed lies in only 7 lines from the code touched in bitcoin#16701. ACKs for top commit: fanquake: ACK 8b6f5aa Tree-SHA512: 65cd8bea550150e5ee47c1e906d8c2393547cf4feba3701a933a4f24fad5ecdb552ac2de4e1200ed14efaa0df0480150dd58fccbddc3b902f6c2141603874902
8b6f5aa qt: Replace QFontMetrics::width() with TextWidth() (Hennadii Stepanov) Pull request description: Compiling master (d8fc997) on macOS Catalina (with a patch from bitcoin#16720) reveals one more instance of `QFontMetrics::width()` which is supposed to be replaced with `TextWidth()` in the merged bitcoin#16701. Sorry for incomplete solution provided in bitcoin#16701. It’s especially sad that the line I missed lies in only 7 lines from the code touched in bitcoin#16701. ACKs for top commit: fanquake: ACK 8b6f5aa Tree-SHA512: 65cd8bea550150e5ee47c1e906d8c2393547cf4feba3701a933a4f24fad5ecdb552ac2de4e1200ed14efaa0df0480150dd58fccbddc3b902f6c2141603874902
8b6f5aa qt: Replace QFontMetrics::width() with TextWidth() (Hennadii Stepanov) Pull request description: Compiling master (d8fc997) on macOS Catalina (with a patch from bitcoin#16720) reveals one more instance of `QFontMetrics::width()` which is supposed to be replaced with `TextWidth()` in the merged bitcoin#16701. Sorry for incomplete solution provided in bitcoin#16701. It’s especially sad that the line I missed lies in only 7 lines from the code touched in bitcoin#16701. ACKs for top commit: fanquake: ACK 8b6f5aa Tree-SHA512: 65cd8bea550150e5ee47c1e906d8c2393547cf4feba3701a933a4f24fad5ecdb552ac2de4e1200ed14efaa0df0480150dd58fccbddc3b902f6c2141603874902
8b6f5aa qt: Replace QFontMetrics::width() with TextWidth() (Hennadii Stepanov) Pull request description: Compiling master (d8fc997) on macOS Catalina (with a patch from bitcoin#16720) reveals one more instance of `QFontMetrics::width()` which is supposed to be replaced with `TextWidth()` in the merged bitcoin#16701. Sorry for incomplete solution provided in bitcoin#16701. It’s especially sad that the line I missed lies in only 7 lines from the code touched in bitcoin#16701. ACKs for top commit: fanquake: ACK 8b6f5aa Tree-SHA512: 65cd8bea550150e5ee47c1e906d8c2393547cf4feba3701a933a4f24fad5ecdb552ac2de4e1200ed14efaa0df0480150dd58fccbddc3b902f6c2141603874902
8b6f5aa qt: Replace QFontMetrics::width() with TextWidth() (Hennadii Stepanov) Pull request description: Compiling master (d8fc997) on macOS Catalina (with a patch from bitcoin#16720) reveals one more instance of `QFontMetrics::width()` which is supposed to be replaced with `TextWidth()` in the merged bitcoin#16701. Sorry for incomplete solution provided in bitcoin#16701. It’s especially sad that the line I missed lies in only 7 lines from the code touched in bitcoin#16701. ACKs for top commit: fanquake: ACK 8b6f5aa Tree-SHA512: 65cd8bea550150e5ee47c1e906d8c2393547cf4feba3701a933a4f24fad5ecdb552ac2de4e1200ed14efaa0df0480150dd58fccbddc3b902f6c2141603874902
The following functions are obsolete in Qt 5.13:
QFontMetrics::width()
QDesktopWidget::availableGeometry()
QDesktopWidget::screenGeometry()
This PR replaces them and does not change behavior.
Here are some excerpts from the master build log: