Skip to content

Commit ddedefe

Browse files
peterixpeterix
committed
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.
1 parent 989263a commit ddedefe

File tree

9 files changed

+33
-118
lines changed

9 files changed

+33
-118
lines changed

‎launcher/Application.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1225,7 +1225,7 @@ bool Application::openJsonEditor(const QString &filename)
12251225
const QString file = QDir::current().absoluteFilePath(filename);
12261226
if (m_settings->get("JsonEditor").toString().isEmpty())
12271227
{
1228-
return DesktopServices::openUrl(QUrl::fromLocalFile(file));
1228+
return DesktopServices::openFile(file);
12291229
}
12301230
else
12311231
{

‎launcher/DesktopServices.cpp‎

Lines changed: 21 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -3,146 +3,62 @@
33
#include <QDesktopServices>
44
#include <QProcess>
55
#include <QDebug>
6+
#include "FileSystem.h"
67

7-
/**
8-
* This shouldn't exist, but until QTBUG-9328 and other unreported bugs are fixed, it needs to be a thing.
9-
*/
10-
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
11-
12-
#include <unistd.h>
13-
#include <errno.h>
14-
#include <sys/types.h>
15-
#include <sys/wait.h>
16-
17-
template <typename T>
18-
bool IndirectOpen(T callable, qint64 *pid_forked = nullptr)
8+
namespace DesktopServices {
9+
bool openDirectory(const QString &path)
1910
{
20-
auto pid = fork();
21-
if(pid_forked)
22-
{
23-
if(pid > 0)
24-
*pid_forked = pid;
25-
else
26-
*pid_forked = 0;
27-
}
28-
if(pid == -1)
29-
{
30-
qWarning() << "IndirectOpen failed to fork: " << errno;
31-
return false;
32-
}
33-
// child - do the stuff
34-
if(pid == 0)
35-
{
36-
// unset all this garbage so it doesn't get passed to the child process
37-
qunsetenv("LD_PRELOAD");
38-
qunsetenv("LD_LIBRARY_PATH");
39-
qunsetenv("LD_DEBUG");
40-
qunsetenv("QT_PLUGIN_PATH");
41-
qunsetenv("QT_FONTPATH");
11+
QUrl url = QUrl::fromLocalFile(path);
12+
QString urlString = url.toString(QUrl::FullyEncoded);
4213

43-
// open the URL
44-
auto status = callable();
14+
qDebug() << "Opening directory" << path << "url" << urlString;
4515

46-
// detach from the parent process group.
47-
setsid();
48-
49-
// die. now. do not clean up anything, it would just hang forever.
50-
_exit(status ? 0 : 1);
51-
}
52-
else
16+
if(!FS::ensureFolderPathExists(path))
5317
{
54-
//parent - assume it worked.
55-
int status;
56-
while (waitpid(pid, &status, 0))
57-
{
58-
if(WIFEXITED(status))
59-
{
60-
return WEXITSTATUS(status) == 0;
61-
}
62-
if(WIFSIGNALED(status))
63-
{
64-
return false;
65-
}
66-
}
67-
return true;
18+
qDebug() << "Failed to create directory for opening:" << path << "url" << urlString;
19+
return false;
6820
}
69-
}
70-
#endif
7121

72-
namespace DesktopServices {
73-
bool openDirectory(const QString &path, bool ensureExists)
74-
{
75-
qDebug() << "Opening directory" << path;
76-
QDir parentPath;
77-
QDir dir(path);
78-
if (!dir.exists())
79-
{
80-
parentPath.mkpath(dir.absolutePath());
81-
}
82-
auto f = [&]()
83-
{
84-
return QDesktopServices::openUrl(QUrl::fromLocalFile(dir.absolutePath()));
85-
};
8622
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
87-
return IndirectOpen(f);
23+
return QProcess::startDetached("xdg-open", QStringList() << urlString);
8824
#else
89-
return f();
25+
return QDesktopServices::openUrl(url);
9026
#endif
9127
}
9228

9329
bool openFile(const QString &path)
9430
{
95-
qDebug() << "Opening file" << path;
96-
auto f = [&]()
97-
{
98-
return QDesktopServices::openUrl(QUrl::fromLocalFile(path));
99-
};
31+
QUrl url = QUrl::fromLocalFile(path);
32+
QString urlString = url.toString(QUrl::FullyEncoded);
33+
qDebug() << "Opening file" << path << " url " << urlString;
34+
10035
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
101-
return IndirectOpen(f);
36+
return QProcess::startDetached("xdg-open", QStringList() << urlString);
10237
#else
103-
return f();
38+
return QDesktopServices::openUrl(url);
10439
#endif
10540
}
10641

10742
bool openFile(const QString &application, const QString &path, const QString &workingDirectory, qint64 *pid)
10843
{
10944
qDebug() << "Opening file" << path << "using" << application;
110-
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
111-
// FIXME: the pid here is fake. So if something depends on it, it will likely misbehave
112-
return IndirectOpen([&]()
113-
{
114-
return QProcess::startDetached(application, QStringList() << path, workingDirectory);
115-
}, pid);
116-
#else
11745
return QProcess::startDetached(application, QStringList() << path, workingDirectory, pid);
118-
#endif
11946
}
12047

12148
bool run(const QString &application, const QStringList &args, const QString &workingDirectory, qint64 *pid)
12249
{
12350
qDebug() << "Running" << application << "with args" << args.join(' ');
124-
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
125-
// FIXME: the pid here is fake. So if something depends on it, it will likely misbehave
126-
return IndirectOpen([&]()
127-
{
128-
return QProcess::startDetached(application, args, workingDirectory);
129-
}, pid);
130-
#else
13151
return QProcess::startDetached(application, args, workingDirectory, pid);
132-
#endif
13352
}
13453

13554
bool openUrl(const QUrl &url)
13655
{
137-
qDebug() << "Opening URL" << url.toString();
138-
auto f = [&]()
139-
{
140-
return QDesktopServices::openUrl(url);
141-
};
56+
QString urlString = url.toString(QUrl::FullyEncoded);
57+
qDebug() << "Opening URL" << urlString;
14258
#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
143-
return IndirectOpen(f);
59+
return QProcess::startDetached("xdg-open", QStringList() << urlString);
14460
#else
145-
return f();
61+
return QDesktopServices::openUrl(url);
14662
#endif
14763
}
14864

‎launcher/DesktopServices.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ namespace DesktopServices
2727
/**
2828
* Open a directory
2929
*/
30-
bool openDirectory(const QString &path, bool ensureExists = false);
30+
bool openDirectory(const QString &path);
3131

3232
/**
3333
* Open the URL, most likely in a browser. Maybe.

‎launcher/ui/MainWindow.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1640,8 +1640,7 @@ void MainWindow::deleteGroup()
16401640

16411641
void MainWindow::on_actionViewInstanceFolder_triggered()
16421642
{
1643-
QString str = APPLICATION->settings()->get("InstanceDir").toString();
1644-
DesktopServices::openDirectory(str);
1643+
DesktopServices::openDirectory(APPLICATION->settings()->get("InstanceDir").toString());
16451644
}
16461645

16471646
void MainWindow::refreshInstances()
@@ -1651,7 +1650,7 @@ void MainWindow::refreshInstances()
16511650

16521651
void MainWindow::on_actionViewCentralModsFolder_triggered()
16531652
{
1654-
DesktopServices::openDirectory(APPLICATION->settings()->get("CentralModsDir").toString(), true);
1653+
DesktopServices::openDirectory(APPLICATION->settings()->get("CentralModsDir").toString());
16551654
}
16561655

16571656
void MainWindow::on_actionConfig_Folder_triggered()

‎launcher/ui/dialogs/IconPickerDialog.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,5 +159,5 @@ IconPickerDialog::~IconPickerDialog()
159159

160160
void IconPickerDialog::openFolder()
161161
{
162-
DesktopServices::openDirectory(APPLICATION->icons()->getDirectory(), true);
162+
DesktopServices::openDirectory(APPLICATION->icons()->getDirectory());
163163
}

‎launcher/ui/pages/instance/ModFolderPage.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,12 +344,12 @@ void ModFolderPage::on_actionRemove_triggered()
344344

345345
void ModFolderPage::on_actionView_configs_triggered()
346346
{
347-
DesktopServices::openDirectory(m_inst->instanceConfigFolder(), true);
347+
DesktopServices::openDirectory(m_inst->instanceConfigFolder());
348348
}
349349

350350
void ModFolderPage::on_actionView_Folder_triggered()
351351
{
352-
DesktopServices::openDirectory(m_mods->dir().absolutePath(), true);
352+
DesktopServices::openDirectory(m_mods->dir().absolutePath());
353353
}
354354

355355
void ModFolderPage::modCurrent(const QModelIndex &current, const QModelIndex &previous)

‎launcher/ui/pages/instance/ScreenshotsPage.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ void ScreenshotsPage::onItemActivated(QModelIndex index)
305305

306306
void ScreenshotsPage::on_actionView_Folder_triggered()
307307
{
308-
DesktopServices::openDirectory(m_folder, true);
308+
DesktopServices::openDirectory(m_folder);
309309
}
310310

311311
void ScreenshotsPage::on_actionUpload_triggered()

‎launcher/ui/pages/instance/VersionPage.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,12 +591,12 @@ void VersionPage::on_actionInstall_LiteLoader_triggered()
591591

592592
void VersionPage::on_actionLibrariesFolder_triggered()
593593
{
594-
DesktopServices::openDirectory(m_inst->getLocalLibraryPath(), true);
594+
DesktopServices::openDirectory(m_inst->getLocalLibraryPath());
595595
}
596596

597597
void VersionPage::on_actionMinecraftFolder_triggered()
598598
{
599-
DesktopServices::openDirectory(m_inst->gameRoot(), true);
599+
DesktopServices::openDirectory(m_inst->gameRoot());
600600
}
601601

602602
void VersionPage::versionCurrent(const QModelIndex &current, const QModelIndex &previous)

‎launcher/ui/pages/instance/WorldListPage.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ void WorldListPage::on_actionRemove_triggered()
173173

174174
void WorldListPage::on_actionView_Folder_triggered()
175175
{
176-
DesktopServices::openDirectory(m_worlds->dir().absolutePath(), true);
176+
DesktopServices::openDirectory(m_worlds->dir().absolutePath());
177177
}
178178

179179
void WorldListPage::on_actionDatapacks_triggered()
@@ -190,7 +190,7 @@ void WorldListPage::on_actionDatapacks_triggered()
190190

191191
auto fullPath = m_worlds->data(index, WorldList::FolderRole).toString();
192192

193-
DesktopServices::openDirectory(FS::PathCombine(fullPath, "datapacks"), true);
193+
DesktopServices::openDirectory(FS::PathCombine(fullPath, "datapacks"));
194194
}
195195

196196

0 commit comments

Comments
 (0)