Initial port to C++17 and Qt6 using Core5Compat#41
Open
ghutchis wants to merge 1 commit intoOpenChemistry:masterfrom
Open
Initial port to C++17 and Qt6 using Core5Compat#41ghutchis wants to merge 1 commit intoOpenChemistry:masterfrom
ghutchis wants to merge 1 commit intoOpenChemistry:masterfrom
Conversation
Signed-off-by: Geoff Hutchison <geoff.hutchison@gmail.com>
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR comprehensively migrates the MoleQueue project from Qt5 to Qt6, updates CMake to require version 3.24 and C++17, replaces deprecated Qt APIs (QRegExp, qsrand/qrand, qSort), modernizes include paths, and refactors platform-specific code. ChangesQt5 to Qt6 Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
molequeue/app/openwithpatternmodel.cpp (1)
122-143:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBug:
CheckStateRolecan fall through into the column switch and the danglingelseis misleading.A few problems in this block:
value.canConvert<int>()andvalue.canConvert<bool>()will both succeed for any numeric/Qt::CheckState value in Qt6, so the secondifis effectively dead code — both branches do the same thing forCaseSensitivityCol.- The
else { return false; }dangles off the secondif; readers (and the original Qt5 code) likely intended it to cover the “not convertible at all” case for CheckStateRole.- Critically, when
role == Qt::CheckStateRolebutind.column() != CaseSensitivityCol, neither inner branch returns and theelsedoes not fire — control falls through to theswitchbelow and can incorrectly treat a check-state value as aPatternCol/PatternTypeColedit (e.g.,value.canConvert<QString>()is true for an int).A
CheckStateRolesetData should always return from this block. Suggest collapsing to a single conversion path and returning unconditionally:🔧 Proposed fix
if (role == Qt::CheckStateRole) { - if (value.canConvert<int>()) { - Qt::CheckState state = static_cast<Qt::CheckState>(value.toInt()); - if (ind.column() == CaseSensitivityCol) { - regexp.setCaseSensitivity(state == Qt::Checked ? Qt::CaseSensitive - : Qt::CaseInsensitive); - emit dataChanged(ind, ind); - return true; - } - } - if (value.canConvert<bool>()) { - if (ind.column() == CaseSensitivityCol) { - regexp.setCaseSensitivity(value.toBool() ? Qt::CaseSensitive - : Qt::CaseInsensitive); - emit dataChanged(ind, ind); - return true; - } - } - else { - return false; - } + if (ind.column() != CaseSensitivityCol || !value.canConvert<int>()) + return false; + const Qt::CheckState state = static_cast<Qt::CheckState>(value.toInt()); + regexp.setCaseSensitivity(state == Qt::Checked ? Qt::CaseSensitive + : Qt::CaseInsensitive); + emit dataChanged(ind, ind); + return true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@molequeue/app/openwithpatternmodel.cpp` around lines 122 - 143, The CheckStateRole handling in setData incorrectly allows fall-through and has misleading dangling else; change the block that checks role == Qt::CheckStateRole to perform a single, explicit conversion (e.g., to int or use value.canConvert<int>() then value.toInt()), then immediately check ind.column() == CaseSensitivityCol, call regexp.setCaseSensitivity(...) and emit dataChanged(ind, ind) and return true; if conversion fails or the column is not CaseSensitivityCol, return false immediately so control cannot fall through into the later switch (which handles PatternCol/PatternTypeCol). Ensure you update the code paths around Qt::CheckStateRole, CaseSensitivityCol, regexp.setCaseSensitivity, and the dataChanged call accordingly.
🧹 Nitpick comments (7)
molequeue/app/testing/queuemanagertest.cpp (1)
106-106: ⚡ Quick winAdd explicit
<algorithm>include forstd::sort.Relying on transitive includes can break portability across toolchains.
Proposed fix
`#include` <QtTest> +#include <algorithm>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@molequeue/app/testing/queuemanagertest.cpp` at line 106, The test uses std::sort (on queueNames) but doesn't explicitly include <algorithm>, relying on transitive includes; add an explicit `#include` <algorithm> to the top of the file's includes so std::sort is guaranteed to be available across toolchains and compilers.molequeue/app/testing/dummyqueueremote.cpp (1)
50-50: ⚡ Quick winUse
Qt::SkipEmptyPartsto avoid fragile token indexing.
parseQueueLine()readssplit.at(0)as the queue ID. If output has leading/trailing whitespace, empty tokens can shift indexes.Proposed fix
- QStringList split = queueListOutput.split(QRegularExpression("\\s+")); + QStringList split = queueListOutput.split(QRegularExpression("\\s+"), + Qt::SkipEmptyParts);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@molequeue/app/testing/dummyqueueremote.cpp` at line 50, The tokenization of queueListOutput in dummyqueueremote.cpp is fragile because empty tokens from leading/trailing whitespace can shift indexes used by parseQueueLine(); change the split call to remove empty parts (use QString::split with Qt::SkipEmptyParts or pass the equivalent flag to QRegularExpression-based split) so that parseQueueLine() can reliably read split.at(0) as the queue ID; update the split assignment in the code that constructs QStringList split and ensure parseQueueLine() continues to use the same indexing logic.molequeue/app/openwithmanagerdialog.h (1)
23-23: ⚡ Quick winPrefer a forward declaration over including
QtCore5Compat/QRegExpin a public header.
QRegExpis only referenced in this header as a pointer return type (QRegExp *selectedRegExp();), so a forward declaration is sufficient. Putting the compat include here forces every translation unit that includesopenwithmanagerdialog.hto depend onQt6::Core5Compatand pull in the QRegExp definitions, which is heavier than necessary and propagates the deprecated header further than required for the migration.♻️ Proposed fix
`#include` <QtCore/QModelIndexList> -#include <QtCore5Compat/QRegExp> class QAbstractButton; class QDataWidgetMapper; class QItemDelegate; +class QRegExp;…and add
#include <QtCore5Compat/QRegExp>at the top ofopenwithmanagerdialog.cppif it isn't already there.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@molequeue/app/openwithmanagerdialog.h` at line 23, Remove the heavy QtCore5Compat include from the public header openwithmanagerdialog.h and replace it with a forward declaration for QRegExp (since only a pointer return type selectedRegExp() is used); then add `#include` <QtCore5Compat/QRegExp> to the implementation file openwithmanagerdialog.cpp (if not already present) so the definition is available where needed. Ensure the declaration for selectedRegExp() remains unchanged and that no other symbols from QtCore5Compat are referenced in the header.molequeue/app/filebrowsewidget.cpp (1)
28-28: ⚡ Quick winDrop the now-unused
QtCore5Compat/QRegExpinclude.After replacing the regex-based PATH splitter with a
QChar, this file no longer referencesQRegExp. Keeping the Core5Compat include adds an unneeded compile/link dependency on a compat module the rest of this file does not need.♻️ Proposed fix
`#include` <QtCore/QFileInfo> `#include` <QtCore/QProcessEnvironment> -#include <QtCore5Compat/QRegExp>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@molequeue/app/filebrowsewidget.cpp` at line 28, Remove the now-unused compatibility header QtCore5Compat/QRegExp from filebrowsewidget.cpp: locate the include line "#include <QtCore5Compat/QRegExp>" and delete it so the file no longer depends on the QtCore5Compat module after replacing the regex-based PATH splitter with a QChar; ensure no other references to QRegExp remain before committing.molequeue/lastinstall/CMakeLists.txt (1)
1-44: 💤 Low valueHeads-up: bundle deploy still Qt5-only and will fail on Qt6-only systems.
The option default is OFF and the label is honest about the limitation, so this is fine as a short-term gate. However if someone flips
MoleQueue_USE_BUNDLE_DEPLOY=ONon a build environment that no longer has Qt5 installed (which is the supported config after this PR), thefind_package(Qt5 ...)calls inside this block will hard-fail. Consider either:
- Leaving a
# TODO: port to DeployQt6 / qt_generate_deploy_app_scriptcomment near the block, or- Emitting an explicit
message(FATAL_ERROR ...)ifQt6_FOUNDand notQt5_FOUNDso the failure mode is obvious to packagers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@molequeue/lastinstall/CMakeLists.txt` around lines 1 - 44, The bundle-deploy block guarded by the option MoleQueue_USE_BUNDLE_DEPLOY currently calls find_package(Qt5...) and DeployQt5 (and install_qt5_executable) which will hard-fail on Qt6-only systems; update the CMake fragment around the MoleQueue_USE_BUNDLE_DEPLOY conditional to either add a clear TODO comment like "# TODO: port to DeployQt6 / qt_generate_deploy_app_script" next to the DeployQt5/install_qt5_executable usage, or (preferred) add an explicit guard that checks Qt6_FOUND and not Qt5_FOUND and calls message(FATAL_ERROR "MoleQueue_USE_BUNDLE_DEPLOY requires Qt5 but only Qt6 was found") so packagers get a clear error instead of an obscure find_package failure (reference symbols: MoleQueue_USE_BUNDLE_DEPLOY, find_package(Qt5 COMPONENTS Core REQUIRED), find_package(Qt5 COMPONENTS Widgets Network REQUIRED), DeployQt5, install_qt5_executable).molequeue/app/openwithpatternmodel.cpp (1)
173-173: ⚡ Quick winReplace deprecated
QVariant::Intwith modern Qt6-style template overload.
QVariant::Intis deprecated since Qt 6.0; the rest of this file usescanConvert<T>()andtypeId(). Line 173 should use the modern template-based API for consistency and to avoid-Wdeprecated-declarationswarnings.♻️ Proposed fix
- else if (value.canConvert(QVariant::Int)) { + else if (value.canConvert<int>()) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@molequeue/app/openwithpatternmodel.cpp` at line 173, Replace the deprecated QVariant::Int usage in the conditional with the Qt6 template-based canConvert overload: find the conditional using QVariant::Int (the else if that calls value.canConvert(QVariant::Int)) and change it to use the template canConvert form on the same QVariant instance (value.canConvert<T>()), keeping the branch logic intact so it detects integer variants the same way; this aligns with other uses of canConvert<T>()/typeId() in openwithpatternmodel.cpp.molequeue/app/CMakeLists.txt (1)
169-201: 💤 Low valueLGTM — Qt6 wrappers, Core5Compat link, and Windows entry point.
qt6_wrap_ui/qt6_add_resources, theQt6::Core5Compatlink onmolequeue_static, and the Windows-specificQt6::EntryPointPrivatelink are all correct for Qt 6's CMake API.One optional note:
Qt6::EntryPointPrivateis a private target, which works fine but technically relies on private API. The cleaner approach is to useqt_add_executable()instead ofadd_executable()— it automatically handles entry point linking without explicit private dependencies. Alternatively, you could rely on Qt's default CMake behavior to auto-link the entry point through Core, and drop the explicit target link.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@molequeue/app/CMakeLists.txt` around lines 169 - 201, Replace the explicit add_executable(molequeue ...) + manual target_link_libraries(molequeue Qt6::EntryPointPrivate) pattern with Qt's wrapper so we don't depend on the private Qt6::EntryPointPrivate target: use qt_add_executable for the molequeue target (keeping the same sources and rcc_srcs) or remove the explicit Qt6::EntryPointPrivate link and rely on the automatic entry-point handling from Qt6::Core; update references around add_executable(molequeue ...) and the target_link_libraries(molequeue Qt6::EntryPointPrivate) call accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@molequeue/app/CMakeLists.txt`:
- Around line 5-8: The build appends XmlPatterns into the Qt6 find_package
unconditionally which fails because Qt6 has no XmlPatterns; update the CMake
logic so when MoleQueue_USE_EZHPC_UIT is enabled you either (a) use Qt5 for that
branch by calling find_package(Qt5 REQUIRED COMPONENTS XmlPatterns) and link
Qt5::XmlPatterns only inside the UIT block (e.g., guard the XmlPatterns add and
linking of Qt6::XmlPatterns), or (b) explicitly disable/gate the EZHPC UIT path
under Qt6 by skipping the XmlPatterns append and the linking of Qt6::XmlPatterns
and emitting a clear message, and ensure wsdl_uitapi.cpp / KDSoap linking only
occurs when the proper XmlPatterns target exists; adjust the conditional around
the XmlPatterns append and the target link to reference MoleQueue_USE_EZHPC_UIT
and Qt major version checks so builds on Qt6 do not call find_package(Qt6
COMPONENTS XmlPatterns) or link Qt6::XmlPatterns.
In `@molequeue/app/jobtableproxymodel.cpp`:
- Around line 213-216: When handling negated tokens in the filtering code, guard
against a token that's just "-" by checking the term after stripping the leading
'-' (the result of QStringView(fullTerm).mid(1)) and skipping it if empty so you
don't call contains(""); update the logic around isNegated/term to
continue/ignore empty terms after removing the '-' (i.e., if term.isEmpty()
after the mid(1) call, do not treat it as a valid filter term).
In `@molequeue/app/queuesettingsdialog.cpp`:
- Line 221: The code calls std::sort(rows.begin(), rows.end()) but does not
include <algorithm>, which can cause toolchain-dependent build failures; fix by
adding the header include <algorithm> near the top of queuesettingsdialog.cpp
(or in the header that defines the code) so std::sort is declared before it is
used, ensuring the call to std::sort (and related algorithms) compiles reliably.
In `@molequeue/app/testing/dummyserver.h`:
- Line 52: Add an explicit include for <limits> because dummyserver.h calls
std::numeric_limits<int>::max() (used when initializing randVal via
gen.bounded(...)); update the header to `#include` <limits> so the use of
std::numeric_limits in the expression int randVal =
gen.bounded(std::numeric_limits<int>::max()); is well-defined and does not rely
on transitive includes.
In `@molequeue/app/testing/queuetest.cpp`:
- Line 117: Add the missing header for std::sort by including <algorithm> at the
top of the file so the call to std::sort(programNames.begin(),
programNames.end()) in queuetest.cpp is properly declared; update the includes
near other standard headers (same style as queuemanagerdialog.cpp) to ensure
portability and avoid relying on transitive Qt headers.
---
Outside diff comments:
In `@molequeue/app/openwithpatternmodel.cpp`:
- Around line 122-143: The CheckStateRole handling in setData incorrectly allows
fall-through and has misleading dangling else; change the block that checks role
== Qt::CheckStateRole to perform a single, explicit conversion (e.g., to int or
use value.canConvert<int>() then value.toInt()), then immediately check
ind.column() == CaseSensitivityCol, call regexp.setCaseSensitivity(...) and emit
dataChanged(ind, ind) and return true; if conversion fails or the column is not
CaseSensitivityCol, return false immediately so control cannot fall through into
the later switch (which handles PatternCol/PatternTypeCol). Ensure you update
the code paths around Qt::CheckStateRole, CaseSensitivityCol,
regexp.setCaseSensitivity, and the dataChanged call accordingly.
---
Nitpick comments:
In `@molequeue/app/CMakeLists.txt`:
- Around line 169-201: Replace the explicit add_executable(molequeue ...) +
manual target_link_libraries(molequeue Qt6::EntryPointPrivate) pattern with Qt's
wrapper so we don't depend on the private Qt6::EntryPointPrivate target: use
qt_add_executable for the molequeue target (keeping the same sources and
rcc_srcs) or remove the explicit Qt6::EntryPointPrivate link and rely on the
automatic entry-point handling from Qt6::Core; update references around
add_executable(molequeue ...) and the target_link_libraries(molequeue
Qt6::EntryPointPrivate) call accordingly.
In `@molequeue/app/filebrowsewidget.cpp`:
- Line 28: Remove the now-unused compatibility header QtCore5Compat/QRegExp from
filebrowsewidget.cpp: locate the include line "#include <QtCore5Compat/QRegExp>"
and delete it so the file no longer depends on the QtCore5Compat module after
replacing the regex-based PATH splitter with a QChar; ensure no other references
to QRegExp remain before committing.
In `@molequeue/app/openwithmanagerdialog.h`:
- Line 23: Remove the heavy QtCore5Compat include from the public header
openwithmanagerdialog.h and replace it with a forward declaration for QRegExp
(since only a pointer return type selectedRegExp() is used); then add `#include`
<QtCore5Compat/QRegExp> to the implementation file openwithmanagerdialog.cpp (if
not already present) so the definition is available where needed. Ensure the
declaration for selectedRegExp() remains unchanged and that no other symbols
from QtCore5Compat are referenced in the header.
In `@molequeue/app/openwithpatternmodel.cpp`:
- Line 173: Replace the deprecated QVariant::Int usage in the conditional with
the Qt6 template-based canConvert overload: find the conditional using
QVariant::Int (the else if that calls value.canConvert(QVariant::Int)) and
change it to use the template canConvert form on the same QVariant instance
(value.canConvert<T>()), keeping the branch logic intact so it detects integer
variants the same way; this aligns with other uses of canConvert<T>()/typeId()
in openwithpatternmodel.cpp.
In `@molequeue/app/testing/dummyqueueremote.cpp`:
- Line 50: The tokenization of queueListOutput in dummyqueueremote.cpp is
fragile because empty tokens from leading/trailing whitespace can shift indexes
used by parseQueueLine(); change the split call to remove empty parts (use
QString::split with Qt::SkipEmptyParts or pass the equivalent flag to
QRegularExpression-based split) so that parseQueueLine() can reliably read
split.at(0) as the queue ID; update the split assignment in the code that
constructs QStringList split and ensure parseQueueLine() continues to use the
same indexing logic.
In `@molequeue/app/testing/queuemanagertest.cpp`:
- Line 106: The test uses std::sort (on queueNames) but doesn't explicitly
include <algorithm>, relying on transitive includes; add an explicit `#include`
<algorithm> to the top of the file's includes so std::sort is guaranteed to be
available across toolchains and compilers.
In `@molequeue/lastinstall/CMakeLists.txt`:
- Around line 1-44: The bundle-deploy block guarded by the option
MoleQueue_USE_BUNDLE_DEPLOY currently calls find_package(Qt5...) and DeployQt5
(and install_qt5_executable) which will hard-fail on Qt6-only systems; update
the CMake fragment around the MoleQueue_USE_BUNDLE_DEPLOY conditional to either
add a clear TODO comment like "# TODO: port to DeployQt6 /
qt_generate_deploy_app_script" next to the DeployQt5/install_qt5_executable
usage, or (preferred) add an explicit guard that checks Qt6_FOUND and not
Qt5_FOUND and calls message(FATAL_ERROR "MoleQueue_USE_BUNDLE_DEPLOY requires
Qt5 but only Qt6 was found") so packagers get a clear error instead of an
obscure find_package failure (reference symbols: MoleQueue_USE_BUNDLE_DEPLOY,
find_package(Qt5 COMPONENTS Core REQUIRED), find_package(Qt5 COMPONENTS Widgets
Network REQUIRED), DeployQt5, install_qt5_executable).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0fa6e2b-34d5-4a4c-b694-6d41e98ba354
📒 Files selected for processing (51)
CMakeLists.txtmolequeue/app/CMakeLists.txtmolequeue/app/addqueuedialog.cppmolequeue/app/filebrowsewidget.cppmolequeue/app/importqueuedialog.cppmolequeue/app/jobactionfactories/killjobactionfactory.cppmolequeue/app/jobactionfactories/opendirectoryactionfactory.cppmolequeue/app/jobactionfactories/openwithactionfactory.cppmolequeue/app/jobactionfactories/openwithactionfactory.hmolequeue/app/jobactionfactories/removejobactionfactory.cppmolequeue/app/jobactionfactories/viewjoblogactionfactory.cppmolequeue/app/jobtableproxymodel.cppmolequeue/app/jobview.cppmolequeue/app/logger.hmolequeue/app/logwindow.cppmolequeue/app/mainwindow.cppmolequeue/app/openwithmanagerdialog.cppmolequeue/app/openwithmanagerdialog.hmolequeue/app/openwithpatternmodel.cppmolequeue/app/openwithpatternmodel.hmolequeue/app/programconfiguredialog.cppmolequeue/app/queue.cppmolequeue/app/queuemanagerdialog.cppmolequeue/app/queues/local.cppmolequeue/app/queues/oar.cppmolequeue/app/queues/pbs.cppmolequeue/app/queues/remote.cppmolequeue/app/queues/remotessh.cppmolequeue/app/queues/sge.cppmolequeue/app/queues/slurm.cppmolequeue/app/queuesettingsdialog.cppmolequeue/app/server.cppmolequeue/app/templatekeyworddialog.cppmolequeue/app/terminalprocess.cppmolequeue/app/terminalprocess.hmolequeue/app/testing/CMakeLists.txtmolequeue/app/testing/clienttestsrc/CMakeLists.txtmolequeue/app/testing/dummyqueueremote.cppmolequeue/app/testing/dummyserver.hmolequeue/app/testing/queuemanagertest.cppmolequeue/app/testing/queueremotetest.cppmolequeue/app/testing/queuetest.cppmolequeue/app/testing/testserver.hmolequeue/app/testing/xmlutils.cppmolequeue/client/CMakeLists.txtmolequeue/client/client.cppmolequeue/client/client.hmolequeue/lastinstall/CMakeLists.txtmolequeue/plugins/CMakeLists.txtmolequeue/plugins/localsocketserver/CMakeLists.txtmolequeue/servercore/CMakeLists.txt
💤 Files with no reviewable changes (1)
- molequeue/app/terminalprocess.h
Comment on lines
5
to
+8
| if(MoleQueue_USE_EZHPC_UIT) | ||
| list(APPEND _qt_packages XmlPatterns) | ||
| endif() | ||
| find_package(Qt5 REQUIRED COMPONENTS ${_qt_packages}) | ||
| find_package(Qt6 REQUIRED COMPONENTS ${_qt_packages}) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is QtXmlPatterns module available in Qt 6 or has it been removed?
💡 Result:
The QtXmlPatterns module has been removed in Qt 6.[1][2] Official Qt 6 documentation lists Qt XML (for DOM) as available,[3][4] but QtXmlPatterns (for XPath/XQuery/XSLT) is absent from all Qt 6 module lists and explicitly stated as removed in Qt 6.0.[1] It was deprecated in Qt 5.13[5] and is only documented for Qt 5.[6][5] Qt forums and mailing lists confirm its removal with no direct replacement in Qt 6.[7][2]
Citations:
- 1: https://doc.qt.io/qt-6.2/whatsnew60.html
- 2: https://lists.qt-project.org/pipermail/interest/2020-December/036087.html
- 3: https://doc.qt.io/qt-6/qtmodules.html
- 4: http://doc.qt.io/qt-6/qtxml-index.html
- 5: https://doc.qt.io/qt-5/qtxmlpatterns-index.html
- 6: https://doc.qt.io/qt-5.6/qtxmlpatterns-index.html
- 7: https://forum.qt.io/topic/154871/qtxmlpatterns-was-removed-from-qt6-how-to-use-xpath-in-qt6
🏁 Script executed:
find . -name "CMakeLists.txt" -path "*/molequeue/app/*" -type fRepository: OpenChemistry/molequeue
Length of output: 190
🏁 Script executed:
cat -n ./molequeue/app/CMakeLists.txt | head -20Repository: OpenChemistry/molequeue
Length of output: 775
🏁 Script executed:
cat -n ./molequeue/app/CMakeLists.txt | tail -100Repository: OpenChemistry/molequeue
Length of output: 3871
🏁 Script executed:
rg -l "wsdl_uitapi\.cpp" .Repository: OpenChemistry/molequeue
Length of output: 98
Qt6 does not ship XmlPatterns — this find_package call will fail when MoleQueue_USE_EZHPC_UIT=ON.
QtXmlPatterns was removed from Qt 6 with no upstream Qt6 port; only Qt5 had the module. Line 6 unconditionally appends XmlPatterns to the find_package(Qt6 ...) COMPONENTS list when the feature is enabled, and line 178 attempts to link Qt6::XmlPatterns, which does not exist as a target.
When MoleQueue_USE_EZHPC_UIT=ON, any build will fail immediately at the find_package step, and wsdl_uitapi.cpp cannot compile.
Options:
- Keep this branch on Qt5 explicitly (e.g.,
find_package(Qt5 REQUIRED COMPONENTS XmlPatterns)and linkQt5::XmlPatternsonly inside the UIT block). This is awkward but likely simplest as a stopgap. - Pull from a community/3rd-party Qt6 port of XmlPatterns (e.g., via a vendored copy) — non-trivial.
- Replace QtXmlPatterns usage in
wsdl_uitapi.cpp/ KDSoap with a still-supported XML stack (QtXml + a third-party XPath/XSLT), or disable the UIT integration on Qt6.
At minimum, the EZHPC UIT path should be marked unsupported under Qt6 and gated so the default build doesn't fail.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@molequeue/app/CMakeLists.txt` around lines 5 - 8, The build appends
XmlPatterns into the Qt6 find_package unconditionally which fails because Qt6
has no XmlPatterns; update the CMake logic so when MoleQueue_USE_EZHPC_UIT is
enabled you either (a) use Qt5 for that branch by calling find_package(Qt5
REQUIRED COMPONENTS XmlPatterns) and link Qt5::XmlPatterns only inside the UIT
block (e.g., guard the XmlPatterns add and linking of Qt6::XmlPatterns), or (b)
explicitly disable/gate the EZHPC UIT path under Qt6 by skipping the XmlPatterns
append and the linking of Qt6::XmlPatterns and emitting a clear message, and
ensure wsdl_uitapi.cpp / KDSoap linking only occurs when the proper XmlPatterns
target exists; adjust the conditional around the XmlPatterns append and the
target link to reference MoleQueue_USE_EZHPC_UIT and Qt major version checks so
builds on Qt6 do not call find_package(Qt6 COMPONENTS XmlPatterns) or link
Qt6::XmlPatterns.
Comment on lines
213
to
216
| if (term.startsWith('-')) { | ||
| isNegated = true; | ||
| term = fullTerm.midRef(1); | ||
| term = QStringView(fullTerm).mid(1); | ||
| } |
There was a problem hiding this comment.
Guard against empty negated terms ("-").
If a token is just "-", Line 215 produces an empty term and contains("") matches everything, so rows are incorrectly filtered out. Skip empty terms after removing -.
Proposed fix
if (term.startsWith('-')) {
isNegated = true;
term = QStringView(fullTerm).mid(1);
+ if (term.isEmpty())
+ continue;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@molequeue/app/jobtableproxymodel.cpp` around lines 213 - 216, When handling
negated tokens in the filtering code, guard against a token that's just "-" by
checking the term after stripping the leading '-' (the result of
QStringView(fullTerm).mid(1)) and skipping it if empty so you don't call
contains(""); update the logic around isNegated/term to continue/ignore empty
terms after removing the '-' (i.e., if term.isEmpty() after the mid(1) call, do
not treat it as a valid filter term).
| } | ||
|
|
||
| qSort(rows); | ||
| std::sort(rows.begin(), rows.end()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check std::sort usages and whether <algorithm> is included in same file
rg -n --type=cpp 'std::sort\s*\('
rg -n --type=cpp '^#include\s*<algorithm>'Repository: OpenChemistry/molequeue
Length of output: 482
🏁 Script executed:
#!/bin/bash
# Read the beginning of queuesettingsdialog.cpp to see all includes
head -50 molequeue/app/queuesettingsdialog.cpp | cat -nRepository: OpenChemistry/molequeue
Length of output: 1972
🏁 Script executed:
#!/bin/bash
# Check line 221 and surrounding context in queuesettingsdialog.cpp
sed -n '215,230p' molequeue/app/queuesettingsdialog.cpp | cat -n
# Also check one of the included headers to see if they transitively include <algorithm>
echo "--- Checking if queue.h includes <algorithm> ---"
head -40 molequeue/app/queue.h | cat -nRepository: OpenChemistry/molequeue
Length of output: 1927
🏁 Script executed:
#!/bin/bash
# Check if queueprogramitemmodel.h includes <algorithm>
rg -n '<algorithm>' molequeue/app/queueprogramitemmodel.h
# Also check other key headers that are included
rg -n '<algorithm>' molequeue/app/*.h | head -20Repository: OpenChemistry/molequeue
Length of output: 49
Add #include <algorithm> for std::sort.
Line 221 uses std::sort without an explicit <algorithm> include. Add it to prevent toolchain-dependent build failures.
Proposed fix
`#include` <QtCore/QDir>
`#include` <QtCore/QFileInfo>
`#include` <QtCore/QSettings>
+#include <algorithm>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@molequeue/app/queuesettingsdialog.cpp` at line 221, The code calls
std::sort(rows.begin(), rows.end()) but does not include <algorithm>, which can
cause toolchain-dependent build failures; fix by adding the header include
<algorithm> near the top of queuesettingsdialog.cpp (or in the header that
defines the code) so std::sort is declared before it is used, ensuring the call
to std::sort (and related algorithms) compiles reliably.
| qsrand(seed); | ||
| int randVal = qrand(); | ||
| QRandomGenerator gen(seed); | ||
| int randVal = gen.bounded(std::numeric_limits<int>::max()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether files using std::numeric_limits include <limits>
rg -n --type=cpp --type=h 'std::numeric_limits<'
rg -n --type=cpp --type=h '^#include\s*<limits>'Repository: OpenChemistry/molequeue
Length of output: 423
🏁 Script executed:
head -60 molequeue/app/testing/dummyserver.h | cat -nRepository: OpenChemistry/molequeue
Length of output: 2297
Add explicit <limits> include for std::numeric_limits.
Line 52 uses std::numeric_limits<int>::max() but this header does not include <limits>. Add it explicitly to avoid transitive-include build breaks.
Proposed fix
`#include` <QtCore/QDateTime>
`#include` <QtCore/QRandomGenerator>
`#include` <QtCore/QThread>
`#include` <QtCore/QTimer>
+#include <limits>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@molequeue/app/testing/dummyserver.h` at line 52, Add an explicit include for
<limits> because dummyserver.h calls std::numeric_limits<int>::max() (used when
initializing randVal via gen.bounded(...)); update the header to `#include`
<limits> so the use of std::numeric_limits in the expression int randVal =
gen.bounded(std::numeric_limits<int>::max()); is well-defined and does not rely
on transitive includes.
| { | ||
| QStringList programNames = m_queue.programNames(); | ||
| qSort(programNames); | ||
| std::sort(programNames.begin(), programNames.end()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether std::sort is used without a local <algorithm> include.
rg -n -C2 'std::sort\s*\(' molequeue/app/testing/queuetest.cpp
rg -n '^\s*#include\s*<algorithm>' molequeue/app/testing/queuetest.cppRepository: OpenChemistry/molequeue
Length of output: 288
🏁 Script executed:
head -30 molequeue/app/testing/queuetest.cppRepository: OpenChemistry/molequeue
Length of output: 980
🏁 Script executed:
# Check if algorithm is included in the directly included headers
rg -l 'algorithm' molequeue/app/testing/queue.h molequeue/app/testing/job.h molequeue/app/testing/jobmanager.h molequeue/app/testing/program.h 2>/dev/null || echo "Checking..."
# Also check in the main source directories for queue/job/jobmanager/program implementations
fd -t f 'queue\.h|job\.h|jobmanager\.h|program\.h' molequeue/app | head -20Repository: OpenChemistry/molequeue
Length of output: 172
🏁 Script executed:
rg '#include.*algorithm' molequeue/app/job.h molequeue/app/jobmanager.h molequeue/app/program.h molequeue/app/queue.hRepository: OpenChemistry/molequeue
Length of output: 49
🏁 Script executed:
# Check for precompiled headers or CMake configuration
find molequeue/app/testing -name "CMakeLists.txt" -o -name "*.h" | xargs grep -l "pch\|precompile" 2>/dev/null | head -5
# Also check if there's any mechanism using algorithm in the codebase
rg -l 'std::sort|std::vector|std::algorithm' molequeue/app --type cpp | head -10Repository: OpenChemistry/molequeue
Length of output: 221
🏁 Script executed:
# Check if other files in the codebase that use std algorithms include <algorithm>
echo "=== queuemanagerdialog.cpp ===" && head -40 molequeue/app/queuemanagerdialog.cpp | grep -E '#include|std::sort'
echo ""
echo "=== queuesettingsdialog.cpp ===" && head -40 molequeue/app/queuesettingsdialog.cpp | grep -E '#include|std::sort'
echo ""
echo "=== queuemanagertest.cpp ===" && head -40 molequeue/app/testing/queuemanagertest.cpp | grep -E '#include|std::sort'Repository: OpenChemistry/molequeue
Length of output: 1313
🏁 Script executed:
# Check if queuemanagertest.cpp uses std::sort or other std algorithms
rg -n 'std::sort|std::find|std::transform' molequeue/app/testing/queuemanagertest.cpp
# Double-check the full includes in queuemanagertest.cpp
head -20 molequeue/app/testing/queuemanagertest.cppRepository: OpenChemistry/molequeue
Length of output: 827
🏁 Script executed:
# Check CMakeLists.txt for precompiled headers or other mechanisms
find molequeue/app -name "CMakeLists.txt" -exec grep -l "PCH\|pch\|target_precompile_headers" {} \;
# Also check the CMakeLists.txt in the testing directory
cat molequeue/app/testing/CMakeLists.txt 2>/dev/null | head -50Repository: OpenChemistry/molequeue
Length of output: 1225
Add #include <algorithm> for std::sort.
Line 117 uses std::sort without an explicit #include <algorithm>. While this may compile due to transitive includes from Qt headers, it's fragile and non-portable. Other files in this codebase (e.g., queuemanagerdialog.cpp) explicitly include <algorithm> when using standard algorithms. Add the include for consistency and portability.
Proposed fix
`#include` <QtTest>
+#include <algorithm>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@molequeue/app/testing/queuetest.cpp` at line 117, Add the missing header for
std::sort by including <algorithm> at the top of the file so the call to
std::sort(programNames.begin(), programNames.end()) in queuetest.cpp is properly
declared; update the includes near other standard headers (same style as
queuemanagerdialog.cpp) to ensure portability and avoid relying on transitive Qt
headers.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit