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 | // We later mutate urls, so we need to store if it was empty originally | ||||
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… | |||||
144 | const bool startedWithURLs = !urls.isEmpty(); | ||||
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`. | |||||
145 | | ||||
142 | 146 | | |||
143 | if (parser.isSet(QStringLiteral("daemon"))) { | 147 | if (parser.isSet(QStringLiteral("daemon"))) { | ||
144 | KDBusService dolphinDBusService; | 148 | KDBusService dolphinDBusService; | ||
145 | DBusInterface interface; | 149 | DBusInterface interface; | ||
146 | interface.setAsDaemon(); | 150 | interface.setAsDaemon(); | ||
147 | return app.exec(); | 151 | return app.exec(); | ||
148 | } | 152 | } | ||
149 | 153 | | |||
150 | if (!parser.isSet(QStringLiteral("new-window"))) { | 154 | if (!parser.isSet(QStringLiteral("new-window"))) { | ||
151 | if (Dolphin::attachToExistingInstance(urls, openFiles, splitView)) { | 155 | if (Dolphin::attachToExistingInstance(urls, openFiles, splitView)) { | ||
152 | // Successfully attached to existing instance of Dolphin | 156 | // Successfully attached to existing instance of Dolphin | ||
153 | return 0; | 157 | return 0; | ||
154 | } | 158 | } | ||
155 | } | 159 | } | ||
156 | 160 | | |||
157 | if (urls.isEmpty()) { | 161 | if (!startedWithURLs) { | ||
158 | // We need at least one URL to open Dolphin | 162 | // We need at least one URL to open Dolphin | ||
159 | urls.append(Dolphin::homeUrl()); | 163 | urls.append(Dolphin::homeUrl()); | ||
160 | } | 164 | } | ||
161 | 165 | | |||
162 | if (splitView && urls.size() < 2) { | 166 | if (splitView && urls.size() < 2) { | ||
163 | // Split view does only make sense if we have at least 2 URLs | 167 | // Split view does only make sense if we have at least 2 URLs | ||
164 | urls.append(urls.last()); | 168 | urls.append(urls.last()); | ||
165 | } | 169 | } | ||
166 | 170 | | |||
167 | DolphinMainWindow* mainWindow = new DolphinMainWindow(); | 171 | DolphinMainWindow* mainWindow = new DolphinMainWindow(); | ||
168 | 172 | | |||
169 | if (openFiles) { | 173 | if (openFiles) { | ||
170 | mainWindow->openFiles(urls, splitView); | 174 | mainWindow->openFiles(urls, splitView); | ||
171 | } else { | 175 | } else { | ||
172 | mainWindow->openDirectories(urls, splitView); | 176 | mainWindow->openDirectories(urls, splitView); | ||
173 | } | 177 | } | ||
174 | 178 | | |||
175 | mainWindow->show(); | 179 | mainWindow->show(); | ||
176 | 180 | | |||
177 | if (app.isSessionRestored()) { | 181 | if (!app.isSessionRestored()) { | ||
182 | KConfigGui::setSessionConfig(QStringLiteral("dolphin"), QStringLiteral("dolphin")); | ||||
183 | } | ||||
184 | | ||||
185 | // Only restore session if: | ||||
186 | // 1. Dolphin was not started with command line args | ||||
187 | // 2. There is a session available to restore | ||||
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 | |||||
188 | // 3. The "remember state" setting is enabled | ||||
189 | if (!startedWithURLs && (app.isSessionRestored() || GeneralSettings::rememberOpenedTabs()) ) { | ||||
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. | |||||
190 | // Get saved state date for the last-closed Dolphin instance | ||||
191 | const QString serviceName = QStringLiteral("org.kde.dolphin-%1").arg(QCoreApplication::applicationPid()); | ||||
192 | if (Dolphin::dolphinInstanceData(serviceName).size() > 0) { | ||||
elvisangelaccio: What is this check needed for? | |||||
ngraham: To not match daemonized instances | |||||
178 | const QString className = KXmlGuiWindow::classNameOfToplevel(1); | 193 | const QString className = KXmlGuiWindow::classNameOfToplevel(1); | ||
179 | if (className == QLatin1String("DolphinMainWindow")) { | 194 | if (className == QLatin1String("DolphinMainWindow")) { | ||
180 | mainWindow->restore(1); | 195 | mainWindow->restore(1); | ||
181 | } else { | 196 | } else { | ||
182 | qCWarning(DolphinDebug) << "Unknown class " << className << " in session saved data!"; | 197 | qCWarning(DolphinDebug) << "Unknown class " << className << " in session saved data!"; | ||
feverfew: Why is the debug output removed? (is it genuinely useless?) | |||||
183 | } | 198 | } | ||
184 | } | 199 | } | ||
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 | |||||
200 | } | ||||
185 | 201 | | |||
186 | KDBusService dolphinDBusService; | 202 | KDBusService dolphinDBusService; | ||
187 | DBusInterface interface; | 203 | DBusInterface interface; | ||
188 | 204 | | |||
189 | return app.exec(); // krazy:exclude=crash; | 205 | return app.exec(); // krazy:exclude=crash; | ||
190 | } | 206 | } |
I don't understand the point of this variable, just use urls.isEmpty() when needed