Skip to content
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

use QString::mid instead of QString::midRef #4372

Merged
merged 1 commit into from
Oct 12, 2021

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Oct 10, 2021

QString::midRef was removed in Qt 6.

@github-actions github-actions bot added the ui label Oct 10, 2021
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
idView = idView.sliced(2, idView.size() - 2);
#else
idView = idView.mid(2);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this method is deprecated in Qt6. It is much more concise than the new sliced method in Qt6.

@@ -155,7 +155,14 @@ void DlgKeywheel::updateSvg() {

if (text.isText()) {
QDomText textNode = text.toText();
ChromaticKey key = static_cast<ChromaticKey>(id.midRef(2).toInt());
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
QStringView idView(id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the simplified expression I proposed? Didn't that work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I did not see the variant of QStringView::sliced that omits the second parameter.

Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! LGTM

@uklotzde uklotzde merged commit 2a0b951 into mixxxdj:main Oct 12, 2021
@Be-ing Be-ing deleted the qstring_midref branch October 12, 2021 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants