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

Removing the dependency of the Core5Compat module with Qt6 #198

Closed
wants to merge 15 commits into from

Conversation

geustache
Copy link

Remove dependency on QTextCodec for qt6 with QStringConvert and no longer need to include the QT5CoreCompat module
Pass qztest on qt5 and qt6
One warning for qt6 for the Russian language :
Actual (zip.getComment()) : "\u0412\u043E\u043F\u0440\u043E\u0441"
Expected (QString::fromUtf8("бНОПНЯ")): "\u0431\u041D\u041E\u041F\u041D\u042F"

…nger need to include the QT5CoreCompat module

Pass qztest on qt5 and qt6
One warning for qt6 for the Russian language :
   Actual   (zip.getComment())           : "\u0412\u043E\u043F\u0440\u043E\u0441"
   Expected (QString::fromUtf8("бНОПНЯ")): "\u0431\u041D\u041E\u041F\u041D\u042F"
@geustache geustache changed the title Remove dependency on QTextCodec for qt6 with QStringConvert and no lo… Remove dependency with QT5CoreCompat Jun 22, 2024
@geustache geustache changed the title Remove dependency with QT5CoreCompat Remove dependency with Core5Compat Jun 22, 2024
@geustache
Copy link
Author

It seems that QStringConvert in qt6 only handles the following encodings
Utf8, Utf16, Utf16BE, Utf16LE, Utf32, ..., System

This undoubtedly explains the message during testing for the Russian language

@geustache
Copy link
Author

Added compatibility with Core5Compat
To summarize, we have 3 cases:

Qt5 -> Uses QTextCodec (Exactly the same behavior as before)
Qt6 with Core5Compat -> Uses QTextCodec (Exactly the same behavior as before)
Qt6 without Core5Compat -> Uses QStringConverter (fewer encodings available)

@geustache geustache closed this Jun 24, 2024
@geustache geustache reopened this Jun 24, 2024
@geustache geustache changed the title Remove dependency with Core5Compat Remove Qt6 dependency with Core5Compat Jun 24, 2024
@geustache geustache changed the title Remove Qt6 dependency with Core5Compat Remove Core5Compat module dependency with Qt6 Jun 24, 2024
@geustache geustache changed the title Remove Core5Compat module dependency with Qt6 Removing the dependency of the Core5Compat module with Qt6 Jun 24, 2024
@cen1
Copy link
Collaborator

cen1 commented Jun 28, 2024

Thank you for the pull. I will do my best to review this soon.

@oetken
Copy link

oetken commented Jul 30, 2024

@geustache I guess you should rebase and squash the commits (esp. due to some accidental commits). And put license header to new file.

@geustache geustache closed this Jul 31, 2024
@geustache
Copy link
Author

Thanks @oetken

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

Successfully merging this pull request may close these issues.

None yet

3 participants