-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Optimized tooltip generation in WBaseWidget #13952
base: main
Are you sure you want to change the base?
Conversation
Declared toQWidget const
} | ||
m_pWidget->setToolTip(debug.join("\n")); | ||
m_pWidget->setToolTip(debug.join(QStringLiteral("\n"))); |
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.
m_pWidget->setToolTip(debug.join(QStringLiteral("\n"))); | |
m_pWidget->setToolTip(debug.join(QChar('\n'))); |
} | ||
for (const auto& pControlConnection : std::as_const(m_connections)) { | ||
*debug << QString("Connection: %1").arg(pControlConnection->toDebugString()); | ||
*debug << QStringLiteral("ClassName: %1") |
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.
since this is supposed to be an optimization, you may want to consider reserving space as appropriate. This adds quite a lot of elements, so it needs to reallocate at least once, avoiding the second allocation by reserving enough upfront may make sense.
*debug << QStringLiteral("ClassName: %1") | |
debug->reserve(debug.size() + 7 + m_leftConnections.size() + m_rightConnections.size() + m_connections.size() + 1); | |
*debug << QStringLiteral("ClassName: %1") |
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.
The QStringBuilder does it implicit for us, if we replace the %1 placeholder by normal string concat using +.
However is it worth the efforts in this PR?
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.
debug
is a QList<QString>
. I don't see how QStringBuilder is relevant here.
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.
I mean
QStringLiteral("ObjectName: %1").arg(m_pWidget->objectName())
->
QStringLiteral("ObjectName: ") + m_pWidget->objectName()
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.
I don't think that makes a meaningful difference for a single argument
Code cleanup. Main change is that empty lines at the end of the tooltips are no longer printed.