Changeset View
Changeset View
Standalone View
Standalone View
src/main.cpp
Show All 25 Lines | |||||
26 | #include "dolphinmainwindow.h" | 26 | #include "dolphinmainwindow.h" | ||
27 | #include "global.h" | 27 | #include "global.h" | ||
28 | 28 | | |||
29 | #include <KAboutData> | 29 | #include <KAboutData> | ||
30 | #include <KCrash> | 30 | #include <KCrash> | ||
31 | #include <KDBusService> | 31 | #include <KDBusService> | ||
32 | #include <KLocalizedString> | 32 | #include <KLocalizedString> | ||
33 | #include <Kdelibs4ConfigMigrator> | 33 | #include <Kdelibs4ConfigMigrator> | ||
34 | #include <KConfigGui> | ||||
34 | 35 | | |||
35 | #include <QApplication> | 36 | #include <QApplication> | ||
36 | #include <QCommandLineParser> | 37 | #include <QCommandLineParser> | ||
37 | #include <QDBusConnection> | 38 | #include <QDBusConnection> | ||
38 | #include <QDBusInterface> | 39 | #include <QDBusInterface> | ||
39 | #include <QDBusAbstractInterface> | 40 | #include <QDBusAbstractInterface> | ||
40 | #include <QDBusConnectionInterface> | 41 | #include <QDBusConnectionInterface> | ||
41 | 42 | | |||
▲ Show 20 Lines • Show All 92 Lines • ▼ Show 20 Line(s) | 61 | #endif | |||
134 | 135 | | |||
135 | parser.process(app); | 136 | parser.process(app); | ||
136 | aboutData.processCommandLine(&parser); | 137 | aboutData.processCommandLine(&parser); | ||
137 | 138 | | |||
138 | const bool splitView = parser.isSet(QStringLiteral("split")) || GeneralSettings::splitView(); | 139 | const bool splitView = parser.isSet(QStringLiteral("split")) || GeneralSettings::splitView(); | ||
139 | const bool openFiles = parser.isSet(QStringLiteral("select")); | 140 | const bool openFiles = parser.isSet(QStringLiteral("select")); | ||
140 | const QStringList args = parser.positionalArguments(); | 141 | const QStringList args = parser.positionalArguments(); | ||
141 | QList<QUrl> urls = Dolphin::validateUris(args); | 142 | QList<QUrl> urls = Dolphin::validateUris(args); | ||
143 | bool urlCommandLineArguments = !urls.isEmpty(); | ||||
feverfew: I don't understand the point of this variable, just use `urls.isEmpty()` when needed | |||||
It's because we later mutate urls, so we need to know if it was empty originally, not that it's currently empty. I'll add a comment for clarity. ngraham: It's because we later mutate `urls`, so we need to know if it was empty originally, not that… | |||||
142 | 144 | | |||
Please give it a descriptive name, e.g. startedWithUrls, and make it const. elvisangelaccio: Please give it a descriptive name, e.g. `startedWithUrls`, and make it `const`. | |||||
143 | if (parser.isSet(QStringLiteral("daemon"))) { | 145 | if (parser.isSet(QStringLiteral("daemon"))) { | ||
144 | KDBusService dolphinDBusService; | 146 | KDBusService dolphinDBusService; | ||
145 | DBusInterface interface; | 147 | DBusInterface interface; | ||
146 | return app.exec(); | 148 | return app.exec(); | ||
147 | } | 149 | } | ||
148 | 150 | | |||
149 | if (!parser.isSet(QStringLiteral("new-window"))) { | 151 | if (!parser.isSet(QStringLiteral("new-window"))) { | ||
150 | if (Dolphin::attachToExistingInstance(urls, openFiles, splitView)) { | 152 | if (Dolphin::attachToExistingInstance(urls, openFiles, splitView)) { | ||
Show All 17 Lines | |||||
168 | if (openFiles) { | 170 | if (openFiles) { | ||
169 | mainWindow->openFiles(urls, splitView); | 171 | mainWindow->openFiles(urls, splitView); | ||
170 | } else { | 172 | } else { | ||
171 | mainWindow->openDirectories(urls, splitView); | 173 | mainWindow->openDirectories(urls, splitView); | ||
172 | } | 174 | } | ||
173 | 175 | | |||
174 | mainWindow->show(); | 176 | mainWindow->show(); | ||
175 | 177 | | |||
176 | if (app.isSessionRestored()) { | 178 | if (!app.isSessionRestored()) { | ||
179 | KConfigGui::setSessionConfig(QStringLiteral("dolphin"), QStringLiteral("dolphin")); | ||||
180 | } | ||||
181 | | ||||
182 | if (!urlCommandLineArguments && (app.isSessionRestored() || GeneralSettings::rememberOpenedTabs()) ) { | ||||
183 | // Only restore if there are no Dolphin windows already open; in this case | ||||
184 | // we want to open a new fresh window | ||||
Why the double negative? I believe removing ! and isEmpty() will have the same effect here. feverfew: Why the double negative? I believe removing `!` and `isEmpty()` will have the same effect here. | |||||
elvisangelaccio: Typo: available | |||||
185 | if (Dolphin::dolphinInstanceData(QStringLiteral("org.kde.dolphin-%1").arg(QCoreApplication::applicationPid())).isEmpty()) { | ||||
177 | const QString className = KXmlGuiWindow::classNameOfToplevel(1); | 186 | const QString className = KXmlGuiWindow::classNameOfToplevel(1); | ||
The comment and the code say two different things, no? Looking at the comment I'd think all three conditions are required, but looking at the if I see that the 2nd and 3rd conditions are not both required. elvisangelaccio: The comment and the code say two different things, no? Looking at the comment I'd think all… | |||||
app.isSessionRestored() refers to session restoration after reboot. I'll clarify the comment. ngraham: `app.isSessionRestored()` refers to session restoration after reboot. I'll clarify the comment. | |||||
178 | if (className == QLatin1String("DolphinMainWindow")) { | 187 | if (className == QLatin1String("DolphinMainWindow")) { | ||
179 | mainWindow->restore(1); | 188 | mainWindow->restore(1); | ||
180 | } else { | 189 | } | ||
elvisangelaccio: What is this check needed for? | |||||
ngraham: To not match daemonized instances | |||||
181 | qCWarning(DolphinDebug) << "Unknown class " << className << " in session saved data!"; | | |||
feverfew: Why is the debug output removed? (is it genuinely useless?) | |||||
182 | } | 190 | } | ||
I believe some comments are required here, I'm struggling to follow what's going on here feverfew: I believe some comments are required here, I'm struggling to follow what's going on here | |||||
183 | } | 191 | } | ||
184 | 192 | | |||
185 | KDBusService dolphinDBusService; | 193 | KDBusService dolphinDBusService; | ||
186 | DBusInterface interface; | 194 | DBusInterface interface; | ||
187 | 195 | | |||
188 | return app.exec(); // krazy:exclude=crash; | 196 | return app.exec(); // krazy:exclude=crash; | ||
189 | } | 197 | } |
I don't understand the point of this variable, just use urls.isEmpty() when needed