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

Update for Qt6 #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update for Qt6 #3

wants to merge 1 commit into from

Conversation

jmlich
Copy link

@jmlich jmlich commented Jul 7, 2023

Even if it is not finished, I would like to open a discussion about what changes are needed for inclusion in the master. I may open separate pull requests for every part.

@jmlich jmlich mentioned this pull request Jul 7, 2023
52 tasks
@jmlich jmlich changed the title WIP: update for Qt6 Update for Qt6 Aug 25, 2023
@jmlich
Copy link
Author

jmlich commented Aug 25, 2023

Seems to be working as expected now

@Thaodan
Copy link
Contributor

Thaodan commented Aug 28, 2023

Don't include merge commits.
Some of the commits could be smashed as for example those that address Qt6 support.
Also remove commits that drop changes from earlier commits.

src/micuconversions.cpp Outdated Show resolved Hide resolved
@jmlich jmlich force-pushed the master branch 2 times, most recently from 8d56385 to 030d78e Compare August 28, 2023 15:18
Copy link
Contributor

@Thaodan Thaodan left a comment

Choose a reason for hiding this comment

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

Please change the commit message style similar to:
sailfishos/qt-mobility-haptics-ffmemless#1 (review)

Rest looks good to me.

qt5 in .pro replaced by qt$${QT_MAJOR_VERSION}
QTextCodec is deprecated in Qt6
QIODevice::WriteOnly was moved to QIODeviceBase::WriteOnly
QRegExp was replaced by QRegularExpression
QString::SkipEmptyParts was moved to Qt::SkipEmptyParts

Co-authored-by: Chupligin Sergey <neochapay@gmail.com>
@@ -4770,8 +4803,9 @@ QString MLocalePrivate::formatPhoneNumber( const QString& phoneNumber,
MLocale::PhoneNumberGrouping grouping ) const
{
// first do sanity check of the input string
QRegExp rx( "\\+?\\d*" );
if ( ! rx.exactMatch( phoneNumber ) )
QRegularExpression rx( QRegularExpression::anchoredPattern("\\+?\\d*") );
Copy link
Contributor

Choose a reason for hiding this comment

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

anchoredPattern() introduced in qt 5.15. We still need support for older versions too.

@@ -28,7 +28,9 @@
#include <QMap>

class QString;
class QStringList;
#if QT_VERSION < 0x051500
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this? All versions have QStringList in the API and even if those didn't, the class declaration shouldn't have too much side-effects?

@@ -65,7 +65,11 @@ class MDebug
QtMsgType convertMsgType(int type);

struct Stream {
#if QT_VERSION < 0x051500
Copy link
Contributor

Choose a reason for hiding this comment

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

QIODeviceBase is a Qt6 thing?

@@ -500,6 +502,12 @@ QList<MCharsetMatch> MCharsetDetector::detectAll()
QString MCharsetDetector::text(const MCharsetMatch &charsetMatch)
{
Q_D(MCharsetDetector);
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
Copy link
Contributor

@pvuorela pvuorela Nov 28, 2023

Choose a reason for hiding this comment

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

Elsewhere the #ifs not indented.

@@ -500,6 +502,12 @@ QList<MCharsetMatch> MCharsetDetector::detectAll()
QString MCharsetDetector::text(const MCharsetMatch &charsetMatch)
{
Q_D(MCharsetDetector);
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
if(charsetMatch.name().toLatin1().isEmpty() || charsetMatch.name().toLatin1() != "UTF-8") {
qWarning() << "Don`t use non utf8 charset";
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it make the class detecting encodings a bit pointless if it can be used only with utf8? Could use core5compat and use the QTextCodec?

@@ -155,12 +155,15 @@ void Ut_Translations::testOriginalQtTr()
// Everything should be untranslated now:
QCOMPARE(tr(qPrintable(string_id)), string_id);

#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why these variations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants