NOISSUE Simplify opening files, directories and URLs

On linux, we now use 'xdg-open' directly with an URL instead of the complicates
indirect opening mechanism.
Everywhere, the logging of issues in opening thing should be better.

launcher/Application.cpp

@@ -1225,7 +1225,7 @@ bool Application::openJsonEditor(const QString &filename)
     const QString file = QDir::current().absoluteFilePath(filename);
     if (m_settings->get("JsonEditor").toString().isEmpty())
     {
-        return DesktopServices::openUrl(QUrl::fromLocalFile(file));
+        return DesktopServices::openFile(file);
     }
     else
     {

launcher/DesktopServices.cpp

@@ -3,146 +3,62 @@
 #include <QDesktopServices>
 #include <QProcess>
 #include <QDebug>
-
-/**
- * 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 <unistd.h>
-#include <errno.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-
-template <typename T>
-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
+#include "FileSystem.h"
 
 namespace DesktopServices {
-bool openDirectory(const QString &path, bool ensureExists)
+bool openDirectory(const QString &path)
 {
-    qDebug() << "Opening directory" << path;
-    QDir parentPath;
-    QDir dir(path);
-    if (!dir.exists())
+    QUrl url = QUrl::fromLocalFile(path);
+    QString urlString = url.toString(QUrl::FullyEncoded);
+
+    qDebug() << "Opening directory" << path << "url" << urlString;
+
+    if(!FS::ensureFolderPathExists(path))
     {
-        parentPath.mkpath(dir.absolutePath());
+        qDebug() << "Failed to create directory for opening:" << path << "url" << urlString;
+        return false;
     }
-    auto f = [&]()
-    {
-        return QDesktopServices::openUrl(QUrl::fromLocalFile(dir.absolutePath()));
-    };
+
 #if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
-    return IndirectOpen(f);
+    return QProcess::startDetached("xdg-open", QStringList() << urlString);
 #else
-    return f();
+    return QDesktopServices::openUrl(url);
 #endif
 }
 
 bool openFile(const QString &path)
 {
-    qDebug() << "Opening file" << path;
-    auto f = [&]()
-    {
-        return QDesktopServices::openUrl(QUrl::fromLocalFile(path));
-    };
+    QUrl url = QUrl::fromLocalFile(path);
+    QString urlString = url.toString(QUrl::FullyEncoded);
+    qDebug() << "Opening file" << path << " url " << urlString;
+
 #if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
-    return IndirectOpen(f);
+    return QProcess::startDetached("xdg-open", QStringList() << urlString);
 #else
-    return f();
+    return QDesktopServices::openUrl(url);
 #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
-    return IndirectOpen([&]()
-    {
-        return QProcess::startDetached(application, QStringList() << path, workingDirectory);
-    }, pid);
-#else
     return QProcess::startDetached(application, QStringList() << path, workingDirectory, pid);
-#endif
 }
 
 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)
-    // 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);
-#endif
 }
 
 bool openUrl(const QUrl &url)
 {
-    qDebug() << "Opening URL" << url.toString();
-    auto f = [&]()
-    {
-        return QDesktopServices::openUrl(url);
-    };
+    QString urlString = url.toString(QUrl::FullyEncoded);
+    qDebug() << "Opening URL" << urlString;
 #if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
-    return IndirectOpen(f);
+    return QProcess::startDetached("xdg-open", QStringList() << urlString);
 #else
-    return f();
+    return QDesktopServices::openUrl(url);
 #endif
 }
 

launcher/DesktopServices.h

@@ -27,7 +27,7 @@ namespace DesktopServices
     /**
      * Open a directory
      */
-    bool openDirectory(const QString &path, bool ensureExists = false);
+    bool openDirectory(const QString &path);
 
     /**
      * Open the URL, most likely in a browser. Maybe.

launcher/ui/MainWindow.cpp

@@ -1640,8 +1640,7 @@ void MainWindow::deleteGroup()
 
 void MainWindow::on_actionViewInstanceFolder_triggered()
 {
-    QString str = APPLICATION->settings()->get("InstanceDir").toString();
-    DesktopServices::openDirectory(str);
+    DesktopServices::openDirectory(APPLICATION->settings()->get("InstanceDir").toString());
 }
 
 void MainWindow::refreshInstances()
@@ -1651,7 +1650,7 @@ void MainWindow::refreshInstances()
 
 void MainWindow::on_actionViewCentralModsFolder_triggered()
 {
-    DesktopServices::openDirectory(APPLICATION->settings()->get("CentralModsDir").toString(), true);
+    DesktopServices::openDirectory(APPLICATION->settings()->get("CentralModsDir").toString());
 }
 
 void MainWindow::on_actionConfig_Folder_triggered()

launcher/ui/dialogs/IconPickerDialog.cpp

@@ -159,5 +159,5 @@ IconPickerDialog::~IconPickerDialog()
 
 void IconPickerDialog::openFolder()
 {
-    DesktopServices::openDirectory(APPLICATION->icons()->getDirectory(), true);
+    DesktopServices::openDirectory(APPLICATION->icons()->getDirectory());
 }

launcher/ui/pages/instance/ModFolderPage.cpp

@@ -344,12 +344,12 @@ void ModFolderPage::on_actionRemove_triggered()
 
 void ModFolderPage::on_actionView_configs_triggered()
 {
-    DesktopServices::openDirectory(m_inst->instanceConfigFolder(), true);
+    DesktopServices::openDirectory(m_inst->instanceConfigFolder());
 }
 
 void ModFolderPage::on_actionView_Folder_triggered()
 {
-    DesktopServices::openDirectory(m_mods->dir().absolutePath(), true);
+    DesktopServices::openDirectory(m_mods->dir().absolutePath());
 }
 
 void ModFolderPage::modCurrent(const QModelIndex &current, const QModelIndex &previous)

launcher/ui/pages/instance/ScreenshotsPage.cpp

@@ -305,7 +305,7 @@ void ScreenshotsPage::onItemActivated(QModelIndex index)
 
 void ScreenshotsPage::on_actionView_Folder_triggered()
 {
-    DesktopServices::openDirectory(m_folder, true);
+    DesktopServices::openDirectory(m_folder);
 }
 
 void ScreenshotsPage::on_actionUpload_triggered()

launcher/ui/pages/instance/VersionPage.cpp

@@ -591,12 +591,12 @@ void VersionPage::on_actionInstall_LiteLoader_triggered()
 
 void VersionPage::on_actionLibrariesFolder_triggered()
 {
-    DesktopServices::openDirectory(m_inst->getLocalLibraryPath(), true);
+    DesktopServices::openDirectory(m_inst->getLocalLibraryPath());
 }
 
 void VersionPage::on_actionMinecraftFolder_triggered()
 {
-    DesktopServices::openDirectory(m_inst->gameRoot(), true);
+    DesktopServices::openDirectory(m_inst->gameRoot());
 }
 
 void VersionPage::versionCurrent(const QModelIndex &current, const QModelIndex &previous)

launcher/ui/pages/instance/WorldListPage.cpp

@@ -173,7 +173,7 @@ void WorldListPage::on_actionRemove_triggered()
 
 void WorldListPage::on_actionView_Folder_triggered()
 {
-    DesktopServices::openDirectory(m_worlds->dir().absolutePath(), true);
+    DesktopServices::openDirectory(m_worlds->dir().absolutePath());
 }
 
 void WorldListPage::on_actionDatapacks_triggered()
@@ -190,7 +190,7 @@ void WorldListPage::on_actionDatapacks_triggered()
 
     auto fullPath = m_worlds->data(index, WorldList::FolderRole).toString();
 
-    DesktopServices::openDirectory(FS::PathCombine(fullPath, "datapacks"), true);
+    DesktopServices::openDirectory(FS::PathCombine(fullPath, "datapacks"));
 }