From 0ccdcd23e3f3889f3e516d48bb03b27d3ddb62cc Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Thu, 14 Dec 2023 13:02:24 +0100 Subject: [PATCH 1/5] fix: open paths directly Signed-off-by: Sefa Eyeoglu --- launcher/DesktopServices.cpp | 68 +++---------------- launcher/DesktopServices.h | 17 +++-- launcher/FileSystem.cpp | 10 ++- launcher/FileSystem.h | 8 ++- launcher/ui/MainWindow.cpp | 18 ++--- launcher/ui/dialogs/IconPickerDialog.cpp | 2 +- .../pages/instance/ExternalResourcesPage.cpp | 4 +- .../ui/pages/instance/ScreenshotsPage.cpp | 4 +- launcher/ui/pages/instance/VersionPage.cpp | 4 +- launcher/ui/pages/instance/WorldListPage.cpp | 4 +- .../ui/widgets/ThemeCustomizationWidget.cpp | 6 +- 11 files changed, 52 insertions(+), 93 deletions(-) diff --git a/launcher/DesktopServices.cpp b/launcher/DesktopServices.cpp index 17eb7c2df..851516399 100644 --- a/launcher/DesktopServices.cpp +++ b/launcher/DesktopServices.cpp @@ -37,6 +37,7 @@ #include #include #include +#include "FileSystem.h" /** * This shouldn't exist, but until QTBUG-9328 and other unreported bugs are fixed, it needs to be a thing. @@ -96,81 +97,30 @@ bool IndirectOpen(T callable, qint64* pid_forked = nullptr) #endif namespace DesktopServices { -bool openDirectory(const QString& path, [[maybe_unused]] bool ensureExists) +bool openPath(const QFileInfo& path, [[maybe_unused]] bool ensureExists) { - qDebug() << "Opening directory" << path; - QDir parentPath; - QDir dir(path); - if (ensureExists && !dir.exists()) { - parentPath.mkpath(dir.absolutePath()); + qDebug() << "Opening path" << path; + if (ensureExists) { + FS::ensureFolderPathExists(path); } - auto f = [&]() { return QDesktopServices::openUrl(QUrl::fromLocalFile(dir.absolutePath())); }; -#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD) - if (!isSandbox()) { - return IndirectOpen(f); - } -#endif - return f(); + return openUrl(QUrl::fromLocalFile(QFileInfo(path).absolutePath())); } -bool openFile(const QString& path) +bool openPath(const QString& path, [[maybe_unused]] bool ensureExists) { - qDebug() << "Opening file" << path; - auto f = [&]() { return QDesktopServices::openUrl(QUrl::fromLocalFile(path)); }; -#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD) - if (!isSandbox()) { - return IndirectOpen(f); - } else { - return f(); - } -#else - return f(); -#endif -} - -bool openFile(const QString& application, const QString& path, const QString& workingDirectory, qint64* pid) -{ - qDebug() << "Opening file" << path << "using" << application; -#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD) - // FIXME: the pid here is fake. So if something depends on it, it will likely misbehave - if (!isSandbox()) { - return IndirectOpen([&]() { return QProcess::startDetached(application, QStringList() << path, workingDirectory); }, pid); - } else { - return QProcess::startDetached(application, QStringList() << path, workingDirectory, pid); - } -#else - return QProcess::startDetached(application, QStringList() << path, workingDirectory, pid); -#endif + return openPath(QFileInfo(path), ensureExists); } bool run(const QString& application, const QStringList& args, const QString& workingDirectory, qint64* pid) { qDebug() << "Running" << application << "with args" << args.join(' '); -#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD) - if (!isSandbox()) { - // FIXME: the pid here is fake. So if something depends on it, it will likely misbehave - return IndirectOpen([&]() { return QProcess::startDetached(application, args, workingDirectory); }, pid); - } else { - return QProcess::startDetached(application, args, workingDirectory, pid); - } -#else return QProcess::startDetached(application, args, workingDirectory, pid); -#endif } bool openUrl(const QUrl& url) { qDebug() << "Opening URL" << url.toString(); - auto f = [&]() { return QDesktopServices::openUrl(url); }; -#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD) - if (!isSandbox()) { - return IndirectOpen(f); - } else { - return f(); - } -#else - return f(); -#endif + return QDesktopServices::openUrl(url); } bool isFlatpak() diff --git a/launcher/DesktopServices.h b/launcher/DesktopServices.h index 151db5557..770eac984 100644 --- a/launcher/DesktopServices.h +++ b/launcher/DesktopServices.h @@ -3,31 +3,30 @@ #include #include +class QFileInfo; + /** * This wraps around QDesktopServices and adds workarounds where needed * Use this instead of QDesktopServices! */ namespace DesktopServices { /** - * Open a file in whatever application is applicable + * Open a path in whatever application is applicable. + * @param ensurePathExists Make sure the path exists */ -bool openFile(const QString& path); +bool openPath(const QFileInfo& path, bool ensurePathExists = false); /** - * Open a file in the specified application + * Open a path in whatever application is applicable. + * @param ensurePathExists Make sure the path exists */ -bool openFile(const QString& application, const QString& path, const QString& workingDirectory = QString(), qint64* pid = 0); +bool openPath(const QString& path, bool ensurePathExists = false); /** * Run an application */ bool run(const QString& application, const QStringList& args, const QString& workingDirectory = QString(), qint64* pid = 0); -/** - * Open a directory - */ -bool openDirectory(const QString& path, bool ensureExists = false); - /** * Open the URL, most likely in a browser. Maybe. */ diff --git a/launcher/FileSystem.cpp b/launcher/FileSystem.cpp index a30d0ae0b..f9be91a2a 100644 --- a/launcher/FileSystem.cpp +++ b/launcher/FileSystem.cpp @@ -272,15 +272,19 @@ bool ensureFilePathExists(QString filenamepath) return success; } -bool ensureFolderPathExists(QString foldernamepath) +bool ensureFolderPathExists(const QFileInfo folderPath) { - QFileInfo a(foldernamepath); QDir dir; - QString ensuredPath = a.filePath(); + QString ensuredPath = folderPath.filePath(); bool success = dir.mkpath(ensuredPath); return success; } +bool ensureFolderPathExists(const QString folderPathName) +{ + return ensureFolderPathExists(QFileInfo(folderPathName)); +} + bool copyFileAttributes(QString src, QString dst) { #ifdef Q_OS_WIN32 diff --git a/launcher/FileSystem.h b/launcher/FileSystem.h index 861cfa267..f13fb9f28 100644 --- a/launcher/FileSystem.h +++ b/launcher/FileSystem.h @@ -91,7 +91,13 @@ bool ensureFilePathExists(QString filenamepath); * Creates all the folders in a path for the specified path * last segment of the path is treated as a folder name and is created! */ -bool ensureFolderPathExists(QString filenamepath); +bool ensureFolderPathExists(const QFileInfo folderPath); + +/** + * Creates all the folders in a path for the specified path + * last segment of the path is treated as a folder name and is created! + */ +bool ensureFolderPathExists(const QString folderPathName); /** * @brief Copies a directory and it's contents from src to dest diff --git a/launcher/ui/MainWindow.cpp b/launcher/ui/MainWindow.cpp index 42de6c2c0..9f2cc434f 100644 --- a/launcher/ui/MainWindow.cpp +++ b/launcher/ui/MainWindow.cpp @@ -1197,43 +1197,43 @@ void MainWindow::undoTrashInstance() void MainWindow::on_actionViewLauncherRootFolder_triggered() { - DesktopServices::openDirectory("."); + DesktopServices::openPath("."); } void MainWindow::on_actionViewInstanceFolder_triggered() { QString str = APPLICATION->settings()->get("InstanceDir").toString(); - DesktopServices::openDirectory(str); + DesktopServices::openPath(str); } void MainWindow::on_actionViewCentralModsFolder_triggered() { - DesktopServices::openDirectory(APPLICATION->settings()->get("CentralModsDir").toString(), true); + DesktopServices::openPath(APPLICATION->settings()->get("CentralModsDir").toString(), true); } void MainWindow::on_actionViewIconThemeFolder_triggered() { - DesktopServices::openDirectory(APPLICATION->themeManager()->getIconThemesFolder().path(), true); + DesktopServices::openPath(APPLICATION->themeManager()->getIconThemesFolder().path(), true); } void MainWindow::on_actionViewWidgetThemeFolder_triggered() { - DesktopServices::openDirectory(APPLICATION->themeManager()->getApplicationThemesFolder().path(), true); + DesktopServices::openPath(APPLICATION->themeManager()->getApplicationThemesFolder().path(), true); } void MainWindow::on_actionViewCatPackFolder_triggered() { - DesktopServices::openDirectory(APPLICATION->themeManager()->getCatPacksFolder().path(), true); + DesktopServices::openPath(APPLICATION->themeManager()->getCatPacksFolder().path(), true); } void MainWindow::on_actionViewIconsFolder_triggered() { - DesktopServices::openDirectory(APPLICATION->icons()->getDirectory(), true); + DesktopServices::openPath(APPLICATION->icons()->getDirectory(), true); } void MainWindow::on_actionViewLogsFolder_triggered() { - DesktopServices::openDirectory("logs", true); + DesktopServices::openPath("logs", true); } void MainWindow::refreshInstances() @@ -1452,7 +1452,7 @@ void MainWindow::on_actionViewSelectedInstFolder_triggered() { if (m_selectedInstance) { QString str = m_selectedInstance->instanceRoot(); - DesktopServices::openDirectory(QDir(str).absolutePath()); + DesktopServices::openPath(QDir(str).absolutePath()); } } diff --git a/launcher/ui/dialogs/IconPickerDialog.cpp b/launcher/ui/dialogs/IconPickerDialog.cpp index faad3ce75..a196fd587 100644 --- a/launcher/ui/dialogs/IconPickerDialog.cpp +++ b/launcher/ui/dialogs/IconPickerDialog.cpp @@ -159,5 +159,5 @@ IconPickerDialog::~IconPickerDialog() void IconPickerDialog::openFolder() { - DesktopServices::openDirectory(APPLICATION->icons()->getDirectory(), true); + DesktopServices::openPath(APPLICATION->icons()->getDirectory(), true); } diff --git a/launcher/ui/pages/instance/ExternalResourcesPage.cpp b/launcher/ui/pages/instance/ExternalResourcesPage.cpp index 48a71b809..2068fa6b1 100644 --- a/launcher/ui/pages/instance/ExternalResourcesPage.cpp +++ b/launcher/ui/pages/instance/ExternalResourcesPage.cpp @@ -290,12 +290,12 @@ void ExternalResourcesPage::disableItem() void ExternalResourcesPage::viewConfigs() { - DesktopServices::openDirectory(m_instance->instanceConfigFolder(), true); + DesktopServices::openPath(m_instance->instanceConfigFolder(), true); } void ExternalResourcesPage::viewFolder() { - DesktopServices::openDirectory(m_model->dir().absolutePath(), true); + DesktopServices::openPath(m_model->dir().absolutePath(), true); } bool ExternalResourcesPage::current(const QModelIndex& current, const QModelIndex& previous) diff --git a/launcher/ui/pages/instance/ScreenshotsPage.cpp b/launcher/ui/pages/instance/ScreenshotsPage.cpp index 83fb0d5ff..01061160b 100644 --- a/launcher/ui/pages/instance/ScreenshotsPage.cpp +++ b/launcher/ui/pages/instance/ScreenshotsPage.cpp @@ -325,7 +325,7 @@ void ScreenshotsPage::onItemActivated(QModelIndex index) return; auto info = m_model->fileInfo(index); QString fileName = info.absoluteFilePath(); - DesktopServices::openFile(info.absoluteFilePath()); + DesktopServices::openPath(info.absoluteFilePath()); } void ScreenshotsPage::onCurrentSelectionChanged(const QItemSelection& selected) @@ -352,7 +352,7 @@ void ScreenshotsPage::onCurrentSelectionChanged(const QItemSelection& selected) void ScreenshotsPage::on_actionView_Folder_triggered() { - DesktopServices::openDirectory(m_folder, true); + DesktopServices::openPath(m_folder, true); } void ScreenshotsPage::on_actionUpload_triggered() diff --git a/launcher/ui/pages/instance/VersionPage.cpp b/launcher/ui/pages/instance/VersionPage.cpp index e425269c8..487433230 100644 --- a/launcher/ui/pages/instance/VersionPage.cpp +++ b/launcher/ui/pages/instance/VersionPage.cpp @@ -447,12 +447,12 @@ void VersionPage::on_actionAdd_Empty_triggered() void VersionPage::on_actionLibrariesFolder_triggered() { - DesktopServices::openDirectory(m_inst->getLocalLibraryPath(), true); + DesktopServices::openPath(m_inst->getLocalLibraryPath(), true); } void VersionPage::on_actionMinecraftFolder_triggered() { - DesktopServices::openDirectory(m_inst->gameRoot(), true); + DesktopServices::openPath(m_inst->gameRoot(), true); } void VersionPage::versionCurrent(const QModelIndex& current, [[maybe_unused]] const QModelIndex& previous) diff --git a/launcher/ui/pages/instance/WorldListPage.cpp b/launcher/ui/pages/instance/WorldListPage.cpp index 587bb6ce6..692db7ad7 100644 --- a/launcher/ui/pages/instance/WorldListPage.cpp +++ b/launcher/ui/pages/instance/WorldListPage.cpp @@ -207,7 +207,7 @@ void WorldListPage::on_actionRemove_triggered() void WorldListPage::on_actionView_Folder_triggered() { - DesktopServices::openDirectory(m_worlds->dir().absolutePath(), true); + DesktopServices::openPath(m_worlds->dir().absolutePath(), true); } void WorldListPage::on_actionDatapacks_triggered() @@ -223,7 +223,7 @@ void WorldListPage::on_actionDatapacks_triggered() auto fullPath = m_worlds->data(index, WorldList::FolderRole).toString(); - DesktopServices::openDirectory(FS::PathCombine(fullPath, "datapacks"), true); + DesktopServices::openPath(FS::PathCombine(fullPath, "datapacks"), true); } void WorldListPage::on_actionReset_Icon_triggered() diff --git a/launcher/ui/widgets/ThemeCustomizationWidget.cpp b/launcher/ui/widgets/ThemeCustomizationWidget.cpp index 0de97441f..25b91857c 100644 --- a/launcher/ui/widgets/ThemeCustomizationWidget.cpp +++ b/launcher/ui/widgets/ThemeCustomizationWidget.cpp @@ -34,11 +34,11 @@ ThemeCustomizationWidget::ThemeCustomizationWidget(QWidget* parent) : QWidget(pa connect(ui->backgroundCatComboBox, QOverload::of(&QComboBox::currentIndexChanged), this, &ThemeCustomizationWidget::applyCatTheme); connect(ui->iconsFolder, &QPushButton::clicked, this, - [] { DesktopServices::openDirectory(APPLICATION->themeManager()->getIconThemesFolder().path()); }); + [] { DesktopServices::openPath(APPLICATION->themeManager()->getIconThemesFolder().path()); }); connect(ui->widgetStyleFolder, &QPushButton::clicked, this, - [] { DesktopServices::openDirectory(APPLICATION->themeManager()->getApplicationThemesFolder().path()); }); + [] { DesktopServices::openPath(APPLICATION->themeManager()->getApplicationThemesFolder().path()); }); connect(ui->catPackFolder, &QPushButton::clicked, this, - [] { DesktopServices::openDirectory(APPLICATION->themeManager()->getCatPacksFolder().path()); }); + [] { DesktopServices::openPath(APPLICATION->themeManager()->getCatPacksFolder().path()); }); } ThemeCustomizationWidget::~ThemeCustomizationWidget() From 213963257cf2b2f7a265893c3112cd71bbce6199 Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Sat, 16 Dec 2023 13:03:51 +0100 Subject: [PATCH 2/5] chore: remove maybe_unused Signed-off-by: Sefa Eyeoglu --- launcher/DesktopServices.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launcher/DesktopServices.cpp b/launcher/DesktopServices.cpp index 851516399..c9d515323 100644 --- a/launcher/DesktopServices.cpp +++ b/launcher/DesktopServices.cpp @@ -97,7 +97,7 @@ bool IndirectOpen(T callable, qint64* pid_forked = nullptr) #endif namespace DesktopServices { -bool openPath(const QFileInfo& path, [[maybe_unused]] bool ensureExists) +bool openPath(const QFileInfo& path, bool ensureExists) { qDebug() << "Opening path" << path; if (ensureExists) { @@ -106,7 +106,7 @@ bool openPath(const QFileInfo& path, [[maybe_unused]] bool ensureExists) return openUrl(QUrl::fromLocalFile(QFileInfo(path).absolutePath())); } -bool openPath(const QString& path, [[maybe_unused]] bool ensureExists) +bool openPath(const QString& path, bool ensureExists) { return openPath(QFileInfo(path), ensureExists); } From a8220cd296a5b9540df07bce7be7e1f6cc28ffe0 Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Sat, 16 Dec 2023 13:12:12 +0100 Subject: [PATCH 3/5] chore: remove unused methods Signed-off-by: Sefa Eyeoglu --- launcher/DesktopServices.cpp | 62 ------------------------------------ launcher/DesktopServices.h | 5 --- 2 files changed, 67 deletions(-) diff --git a/launcher/DesktopServices.cpp b/launcher/DesktopServices.cpp index c9d515323..ecad825a5 100644 --- a/launcher/DesktopServices.cpp +++ b/launcher/DesktopServices.cpp @@ -39,63 +39,6 @@ #include #include "FileSystem.h" -/** - * This shouldn't exist, but until QTBUG-9328 and other unreported bugs are fixed, it needs to be a thing. - */ -#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD) - -#include -#include -#include -#include - -template -bool IndirectOpen(T callable, qint64* pid_forked = nullptr) -{ - auto pid = fork(); - if (pid_forked) { - if (pid > 0) - *pid_forked = pid; - else - *pid_forked = 0; - } - if (pid == -1) { - qWarning() << "IndirectOpen failed to fork: " << errno; - return false; - } - // child - do the stuff - if (pid == 0) { - // unset all this garbage so it doesn't get passed to the child process - qunsetenv("LD_PRELOAD"); - qunsetenv("LD_LIBRARY_PATH"); - qunsetenv("LD_DEBUG"); - qunsetenv("QT_PLUGIN_PATH"); - qunsetenv("QT_FONTPATH"); - - // open the URL - auto status = callable(); - - // detach from the parent process group. - setsid(); - - // die. now. do not clean up anything, it would just hang forever. - _exit(status ? 0 : 1); - } else { - // parent - assume it worked. - int status; - while (waitpid(pid, &status, 0)) { - if (WIFEXITED(status)) { - return WEXITSTATUS(status) == 0; - } - if (WIFSIGNALED(status)) { - return false; - } - } - return true; - } -} -#endif - namespace DesktopServices { bool openPath(const QFileInfo& path, bool ensureExists) { @@ -141,9 +84,4 @@ bool isSnap() #endif } -bool isSandbox() -{ - return isSnap() || isFlatpak(); -} - } // namespace DesktopServices diff --git a/launcher/DesktopServices.h b/launcher/DesktopServices.h index 770eac984..6c4cbdce7 100644 --- a/launcher/DesktopServices.h +++ b/launcher/DesktopServices.h @@ -41,9 +41,4 @@ bool isFlatpak(); * Determine whether the launcher is running in a Snap environment */ bool isSnap(); - -/** - * Determine whether the launcher is running in a sandboxed (Flatpak or Snap) environment - */ -bool isSandbox(); } // namespace DesktopServices From e5b608447a1c4860343aed15ef1e1bd5940ba17d Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Wed, 3 Jan 2024 18:14:47 +0100 Subject: [PATCH 4/5] chore: improve param name Signed-off-by: Sefa Eyeoglu --- launcher/DesktopServices.cpp | 8 ++++---- launcher/DesktopServices.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/launcher/DesktopServices.cpp b/launcher/DesktopServices.cpp index ecad825a5..7c070bd31 100644 --- a/launcher/DesktopServices.cpp +++ b/launcher/DesktopServices.cpp @@ -40,18 +40,18 @@ #include "FileSystem.h" namespace DesktopServices { -bool openPath(const QFileInfo& path, bool ensureExists) +bool openPath(const QFileInfo& path, bool ensureFolderPathExists) { qDebug() << "Opening path" << path; - if (ensureExists) { + if (ensureFolderPathExists) { FS::ensureFolderPathExists(path); } return openUrl(QUrl::fromLocalFile(QFileInfo(path).absolutePath())); } -bool openPath(const QString& path, bool ensureExists) +bool openPath(const QString& path, bool ensureFolderPathExists) { - return openPath(QFileInfo(path), ensureExists); + return openPath(QFileInfo(path), ensureFolderPathExists); } bool run(const QString& application, const QStringList& args, const QString& workingDirectory, qint64* pid) diff --git a/launcher/DesktopServices.h b/launcher/DesktopServices.h index 6c4cbdce7..6c6208e82 100644 --- a/launcher/DesktopServices.h +++ b/launcher/DesktopServices.h @@ -12,15 +12,15 @@ class QFileInfo; namespace DesktopServices { /** * Open a path in whatever application is applicable. - * @param ensurePathExists Make sure the path exists + * @param ensureFolderPathExists Make sure the path exists */ -bool openPath(const QFileInfo& path, bool ensurePathExists = false); +bool openPath(const QFileInfo& path, bool ensureFolderPathExists = false); /** * Open a path in whatever application is applicable. - * @param ensurePathExists Make sure the path exists + * @param ensureFolderPathExists Make sure the path exists */ -bool openPath(const QString& path, bool ensurePathExists = false); +bool openPath(const QString& path, bool ensureFolderPathExists = false); /** * Run an application From 67d088dc53337fc2abeae3ce20c89c9495f5c9be Mon Sep 17 00:00:00 2001 From: Sefa Eyeoglu Date: Sun, 14 Jan 2024 11:36:17 +0100 Subject: [PATCH 5/5] fix: simplify openPath calls Signed-off-by: Sefa Eyeoglu --- launcher/ui/MainWindow.cpp | 2 +- launcher/ui/pages/instance/ScreenshotsPage.cpp | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/launcher/ui/MainWindow.cpp b/launcher/ui/MainWindow.cpp index 9f2cc434f..85573314d 100644 --- a/launcher/ui/MainWindow.cpp +++ b/launcher/ui/MainWindow.cpp @@ -1452,7 +1452,7 @@ void MainWindow::on_actionViewSelectedInstFolder_triggered() { if (m_selectedInstance) { QString str = m_selectedInstance->instanceRoot(); - DesktopServices::openPath(QDir(str).absolutePath()); + DesktopServices::openPath(QFileInfo(str)); } } diff --git a/launcher/ui/pages/instance/ScreenshotsPage.cpp b/launcher/ui/pages/instance/ScreenshotsPage.cpp index 01061160b..c3f955733 100644 --- a/launcher/ui/pages/instance/ScreenshotsPage.cpp +++ b/launcher/ui/pages/instance/ScreenshotsPage.cpp @@ -324,8 +324,7 @@ void ScreenshotsPage::onItemActivated(QModelIndex index) if (!index.isValid()) return; auto info = m_model->fileInfo(index); - QString fileName = info.absoluteFilePath(); - DesktopServices::openPath(info.absoluteFilePath()); + DesktopServices::openPath(info); } void ScreenshotsPage::onCurrentSelectionChanged(const QItemSelection& selected)