Skip to content

Migrate to CMake/Qt6 and modernize code (regex, encoding, GUI centering, build)#3

Open
mskpluk wants to merge 2 commits intomasterfrom
codex/przenies-qnapi-na-qt6
Open

Migrate to CMake/Qt6 and modernize code (regex, encoding, GUI centering, build)#3
mskpluk wants to merge 2 commits intomasterfrom
codex/przenies-qnapi-na-qt6

Conversation

@mskpluk
Copy link
Copy Markdown
Owner

@mskpluk mskpluk commented Apr 19, 2026

Motivation

  • Move the project to CMake and Qt6 to modernize the build and enable use of newer Qt APIs.
  • Replace deprecated APIs and platform-dependent hacks (e.g. QRegExp, QDesktopWidget, qrand, QTextCodec) with modern equivalents for maintainability and correctness.
  • Improve subtitle encoding detection/handling and parsing reliability across platforms by centralizing encoding logic.

Description

  • Add CMake buildsystem with CMakeLists.txt and subdirectory CMake targets for libqnapi, gui, and cli, and create an alias target qnapi::libqnapi.
  • Update Debian packaging metadata in debian/control and debian/changelog to version 0.4.0 and switch runtime deps to Qt6 (libqt6*).
  • Replace window centering using QDesktopWidget with QGuiApplication::primaryScreen()->availableGeometry() and remove obsolete headers accordingly.
  • Replace QRegExp/QRegExp::exactMatch usages with QRegularExpression and switch other parsing to the newer API, e.g. subtitle format parsers, p7zip output parsing and argument splitting/joining.
  • Introduce EncodingUtils to centralize encoding detection and conversions using QStringConverter and migrate file I/O and subtitle processing to use EncodingUtils instead of QTextCodec/QTextStream.
  • Replace qrand/qsrand usage with QRandomGenerator::global()->generate() for tmp filename generation and use qEnvironmentVariable("PATH") instead of QProcess::systemEnvironment() to discover executables.
  • Miscellaneous modernizations and cleanups across many files (includes, small API changes, and platform-specific source selection in CMake).

Testing

  • No automated tests were executed as part of this change.

Codex Task

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd45591e8b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CMakeLists.txt Outdated

project(qnapi VERSION 0.0.0 LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 11)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Raise CMake C++ standard to 17 for Qt6 targets

This migration enables Qt6 but still forces CMAKE_CXX_STANDARD to 11, which breaks CMake-based builds on Qt6 toolchains that require C++17-capable consumers. In practice this causes compile-time failure for users trying the new CMake path, so the build configuration introduced here is not usable without bumping the language standard.

Useful? React with 👍 / 👎.

Comment thread CMakeLists.txt Outdated
@@ -0,0 +1,26 @@
cmake_minimum_required(VERSION 3.21)

project(qnapi VERSION 0.0.0 LANGUAGES CXX)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use real project version instead of 0.0.0

The top-level CMake project version is hardcoded to 0.0.0 while this same change advertises a 0.4.0 release in Debian metadata. Any packaging or artifact naming that reads PROJECT_VERSION will emit incorrect versioned outputs, which can break release automation and produce misleading package filenames.

Useful? React with 👍 / 👎.

Comment on lines +321 to +323
EncodingUtils encodingUtils;
QStringList codecs = encodingUtils.availableEncodings();
std::sort(codecs.begin(), codecs.end());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Restore full encoding list for “Show all encodings”

Switching this path from QTextCodec::availableCodecs() to EncodingUtils::availableEncodings() silently narrows “show all” to the same short internal codec list used for heuristics, so users lose access to many previously selectable encodings. This also makes existing configs with values outside that shortlist fail findText(...), increasing the chance those settings are unintentionally replaced when options are saved.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant