Fix: correct warnings/errors found whilst working on Windows CI #7224
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Whilst working on getting the Windows CI process to run in a
MSYS2+Mingw-w64 environment on AppVeyor in both Qt 5 and 6 and both 32-bits
and 64-bits (Qt6 only supports 64-Bit builds). I ran into a number of
warnings, some of them about things deprecated in Qt 6.0 or later. This PR
should eliminate all of them for our code (though there are a couple in
upstream things).
Make the build process cleaner all around, especially with moving forward
to Qt 6.
The use of `std::as_const(...)` requires C++17 but we have already mandated
that. `qAsConst(...)` is deprecated in Qt 6.
Some of the places where the above was being done also were missing the use
of a `const` reference rather than the making of a constant copy of the
iterated values; these have been fixed as well.
A couple of Mudlet classses that I haven't yet cleaned up to move as much
of the class initialisation to the header as possible were reporting
initialisation ordering issue (`Host` and `TTimer`). I have fixed those
but only in the region of the issues, more work there is desirable to clean
up every remaining class - but I'm not allowed to leave "TODO:" comments
around nowadays! 😀
`(void) zip_error_to_str(char*, size_t, int, int))` has been obsoleted for
a long time now, and I've finally put in something in a couple of places
that will use the recommended replacement
`(zip_error_t*) zip_get_error(zip*)` and dump the error message out to the
OS console - which was not happening in the past.
`(QString) QString::fromUtf16(...)` has been obsoleted and alternatives
are suggested within the Qt documentation. I've used
`QString::fromWCharArray(...)`. Whilst this compiles ***I am not 100% sure
I have this correct and a second opinion on this change in
`./src/mudlet.cpp` is desirable!***
Qt is renaming in Qt6 a few methods that otherwise function as before:
* `(Qt::KeyboardModifiers) QDragEnterEvent::keyboardModifiers()`
==> `QDragEnterEvent::modifiers()`
* `(Qt::KeyboardModifiers) QDragMoveEvent::keyboardModifiers()`
==> `QDragMoveEvent::modifiers()`
* `(bool) QColor::isValidColor(const QString&)`
==> `(bool) QColor::isValidColorName(QAnyStringView)`
* `(void) QColor::setNamedColor(const QString&)`
==> `(QColor) QColor::fromString(QAnyStringView)`
* `(QString) QLocale::countryToString(Country)`
==> `(QString) QLocale::territoryToString(Territory)`
Windows NTFS permissions checking was being done with a really low-level
proceedure which has been deprecated in Qt 6.6 and replaced with a slightly
better (but also low-level) pair of functions:
* `(bool) qEnableNtfsPermissionChecks()`
* `(bool) qEnableNtfsPermissionChecks()`
to do the same thing in almost the same way with a lesser risk of a
"race-condition". There is a higher-level proceedure involving the use of
a new class `QNtfsPermissionCheckGuard` but that is a different way of
doing things that is not a drop-in replacement AFAICT.
There was an unhandled `case` (for `QTextToSpeech::State::Synthesizing`) in
`(void) TLuaInterpreter::ttsStateChanged(QTextToSpeech::State)` - I've put
in something to report that state but it is not clear that this, seemingly,
transient state, needs anything extra tham that. For instance, given that
it looks to be associated with preparing a text to be spoken it might be
reasonable to report the text involved as the `Speaking` state does...
The point at which it was introduced is also unclear as that isn't
documented!
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
…WorkingOnWindowsCI
|
/refresh links |
…WorkingOnWindowsCI
jmckisson
added a commit
to jmckisson/Mudlet
that referenced
this pull request
May 21, 2024
…CI (Mudlet#7224)" This reverts commit d1ac61d.
SlySven
added a commit
to SlySven/Mudlet
that referenced
this pull request
Jun 15, 2024
…et#7224) #### Brief overview of PR changes/additions Whilst working on getting the Windows CI process to run in a MSYS2+Mingw-w64 environment on AppVeyor in both Qt 5 and 6 and both 32-bits and 64-bits (Qt6 only supports 64-Bit builds). I ran into a number of warnings, some of them about things deprecated in Qt 6.0 or later. This PR should eliminate all of them for our code (though there are a couple in upstream things). #### Motivation for adding to Mudlet Make the build process cleaner all around, especially with moving forward to Qt 6. #### Other info (issues closed, discussion etc) The use of `std::as_const(...)` requires C++17 but we have already mandated that. `qAsConst(...)` is deprecated in Qt 6. Some of the places where the above was being done also were missing the use of a `const` reference rather than the making of a constant copy of the iterated values; these have been fixed as well. A couple of Mudlet classes that I haven't yet cleaned up to move as much of the class initialisation to the header as possible were reporting initialisation ordering issue (`Host` and `TTimer`). I have fixed those but only in the region of the issues, more work there is desirable to clean up every remaining class - but I'm not allowed to leave "TODO:" comments around nowadays! 😀 `(void) zip_error_to_str(char*, size_t, int, int))` has been obsoleted for a long time now, and I've finally put in something in a couple of places that will use the recommended replacement `(zip_error_t*) zip_get_error(zip*)` and dump the error message out to the OS console - which was not happening in the past. `(QString) QString::fromUtf16(...)` has been obsoleted and alternatives are suggested within the Qt documentation. I've used `QString::fromWCharArray(...)`. Whilst this compiles ***I am not 100% sure I have this correct and a second opinion on this change in `./src/mudlet.cpp` is desirable!*** Qt is renaming in Qt6 a few methods that otherwise function as before: * `(Qt::KeyboardModifiers) QDragEnterEvent::keyboardModifiers()` ==> `QDragEnterEvent::modifiers()` * `(Qt::KeyboardModifiers) QDragMoveEvent::keyboardModifiers()` ==> `QDragMoveEvent::modifiers()` * `(bool) QColor::isValidColor(const QString&)` ==> `(bool) QColor::isValidColorName(QAnyStringView)` * `(void) QColor::setNamedColor(const QString&)` ==> `(QColor) QColor::fromString(QAnyStringView)` * `(QString) QLocale::countryToString(Country)` ==> `(QString) QLocale::territoryToString(Territory)` Windows NTFS permissions checking was being done with a really low-level procedure which has been deprecated in Qt 6.6 and replaced with a slightly better (but also low-level) pair of functions: * `(bool) qEnableNtfsPermissionChecks()` * `(bool) qEnableNtfsPermissionChecks()` to do the same thing in almost the same way with a lesser risk of a "race-condition". There is a higher-level procedure involving the use of a new class `QNtfsPermissionCheckGuard` but that is a different way of doing things that is not a drop-in replacement AFAICT. There was an unhandled `case` (for `QTextToSpeech::State::Synthesizing`) in `(void) TLuaInterpreter::ttsStateChanged(QTextToSpeech::State)` - I've put in something to report that state but it is not clear that this, seemingly, transient state, needs anything extra than that. For instance, given that it looks to be associated with preparing a text to be spoken it might be reasonable to report the text involved as the `Speaking` state does... The point at which it was introduced is also unclear as that isn't documented! Signed-off-by: Stephen Lyons <slysven@virginmedia.com> Co-authored-by: Vadim Peretokin <vperetokin@hey.com>
SlySven
deleted the
Fix_correctWarnings&ErrorsFoundWhilstWorkingOnWindowsCI
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Brief overview of PR changes/additions
Whilst working on getting the Windows CI process to run in a MSYS2+Mingw-w64 environment on AppVeyor in both Qt 5 and 6 and both 32-bits and 64-bits (Qt6 only supports 64-Bit builds). I ran into a number of warnings, some of them about things deprecated in Qt 6.0 or later. This PR should eliminate all of them for our code (though there are a couple in upstream things).
Motivation for adding to Mudlet
Make the build process cleaner all around, especially with moving forward to Qt 6.
Other info (issues closed, discussion etc)
The use of
std::as_const(...)requires C++17 but we have already mandated that.qAsConst(...)is deprecated in Qt 6.Some of the places where the above was being done also were missing the use of a
constreference rather than the making of a constant copy of the iterated values; these have been fixed as well.A couple of Mudlet classes that I haven't yet cleaned up to move as much of the class initialisation to the header as possible were reporting initialisation ordering issue (
HostandTTimer). I have fixed those but only in the region of the issues, more work there is desirable to clean up every remaining class - but I'm not allowed to leave "TODO:" comments around nowadays! 😀(void) zip_error_to_str(char*, size_t, int, int))has been obsoleted for a long time now, and I've finally put in something in a couple of places that will use the recommended replacement(zip_error_t*) zip_get_error(zip*)and dump the error message out to the OS console - which was not happening in the past.(QString) QString::fromUtf16(...)has been obsoleted and alternatives are suggested within the Qt documentation. I've usedQString::fromWCharArray(...). Whilst this compiles I am not 100% sure I have this correct and a second opinion on this change in./src/mudlet.cppis desirable!Qt is renaming in Qt6 a few methods that otherwise function as before:
(Qt::KeyboardModifiers) QDragEnterEvent::keyboardModifiers()==>QDragEnterEvent::modifiers()(Qt::KeyboardModifiers) QDragMoveEvent::keyboardModifiers()==>QDragMoveEvent::modifiers()(bool) QColor::isValidColor(const QString&)==>(bool) QColor::isValidColorName(QAnyStringView)(void) QColor::setNamedColor(const QString&)==>(QColor) QColor::fromString(QAnyStringView)(QString) QLocale::countryToString(Country)==>(QString) QLocale::territoryToString(Territory)Windows NTFS permissions checking was being done with a really low-level procedure which has been deprecated in Qt 6.6 and replaced with a slightly better (but also low-level) pair of functions:
(bool) qEnableNtfsPermissionChecks()(bool) qEnableNtfsPermissionChecks()to do the same thing in almost the same way with a lesser risk of a "race-condition". There is a higher-level procedure involving the use of a new class
QNtfsPermissionCheckGuardbut that is a different way of doing things that is not a drop-in replacement AFAICT.There was an unhandled
case(forQTextToSpeech::State::Synthesizing) in(void) TLuaInterpreter::ttsStateChanged(QTextToSpeech::State)- I've put in something to report that state but it is not clear that this, seemingly, transient state, needs anything extra than that. For instance, given that it looks to be associated with preparing a text to be spoken it might be reasonable to report the text involved as theSpeakingstate does... The point at which it was introduced is also unclear as that isn't documented!