fix: properly handle CLI arguments (#2368)

Fix CLI arguments not being respected. This happened due to the addition of category-based logging (--help, --version) and changes to the window loading ( --channels), respectively.

When handling --channels, I took the liberty to refactor the previous version of window description (which relied on generating JSON) to directly building the WindowLayout.

CHANGELOG.md

@@ -25,7 +25,7 @@
 - Minor: Made the current channels emotes appear at the top of the emote picker popup. (#2057)
 - Minor: Added viewer list button to twitch channel header. (#1978)
 - Minor: Added followage and subage information to usercard. (#2023)
-- Minor: Added an option to only open channels specified in command line with `-c` parameter. You can also use `--help` to display short help message (#1940)
+- Minor: Added an option to only open channels specified in command line with `-c` parameter. You can also use `--help` to display short help message (#1940, #2368)
 - Minor: Added customizable timeout buttons to the user info popup
 - Minor: Deprecate loading of "v1" window layouts. If you haven't updated Chatterino in more than 2 years, there's a chance you will lose your window layout.
 - Minor: User popup will now automatically display messages as they are received
@@ -68,6 +68,7 @@
 - Bugfix: Fix a crash bug that occurred when moving splits across windows and closing the "parent tab" (#2249, #2259)
 - Bugfix: Fix a crash bug that occurred when the "Limit message height" setting was enabled and a message was being split up into multiple lines. IRC only. (#2329)
 - Bugfix: Fix anonymous users being pinged by "username" justinfan64537 (#2156, #2352)
+- Bugfix: Fix CLI arguments (`--help`, `--version`, `--channels`) not being respected (#2368, #2190)
 - Dev: Updated minimum required Qt framework version to 5.12. (#2210)
 - Dev: Migrated `Kraken::getUser` to Helix (#2260)
 - Dev: Migrated `TwitchAccount::(un)followUser` from Kraken to Helix and moved it to `Helix::(un)followUser`. (#2306)

src/common/Args.cpp

@@ -3,11 +3,12 @@
 #include <QApplication>
 #include <QCommandLineParser>
 #include <QDebug>
-#include <QJsonArray>
-#include <QJsonDocument>
-#include <QJsonObject>
 #include <QStringList>
 #include "common/QLogging.hpp"
+#include "singletons/Paths.hpp"
+#include "singletons/WindowManager.hpp"
+#include "util/CombinePath.hpp"
+#include "widgets/Window.hpp"
 
 namespace chatterino {
 
@@ -45,7 +46,7 @@ Args::Args(const QApplication &app)
 
     if (parser.isSet("help"))
     {
-        qCInfo(chatterinoArgs).noquote() << parser.helpText();
+        qInfo().noquote() << parser.helpText();
         ::exit(EXIT_SUCCESS);
     }
 
@@ -56,44 +57,86 @@ Args::Args(const QApplication &app)
 
     if (parser.isSet("c"))
     {
-        QJsonArray channelArray;
+        this->applyCustomChannelLayout(parser.value("c"));
-        QStringList channelArgList = parser.value("c").split(";");
-        for (QString channelArg : channelArgList)
-        {
-            // Twitch is default platform
-            QString platform = "t";
-            QString channelName = channelArg;
-
-            const QRegExp regExp("(.):(.*)");
-            if (regExp.indexIn(channelArg) != -1)
-            {
-                platform = regExp.cap(1);
-                channelName = regExp.cap(2);
-            }
-
-            // Twitch (default)
-            if (platform == "t")
-            {
-                // TODO: try not to parse JSON
-                QString channelObjectString =
-                    "{\"splits2\": { \"data\": { \"name\": \"" + channelName +
-                    "\", \"type\": \"twitch\" }, \"type\": \"split\" }}";
-                channelArray.push_back(
-                    QJsonDocument::fromJson(channelObjectString.toUtf8())
-                        .object());
-            }
-        }
-        if (channelArray.size() > 0)
-        {
-            this->dontSaveSettings = true;
-            this->channelsToJoin = channelArray;
-        }
     }
 
     this->printVersion = parser.isSet("v");
     this->crashRecovery = parser.isSet("crash-recovery");
 }
 
+void Args::applyCustomChannelLayout(const QString &argValue)
+{
+    WindowLayout layout;
+    WindowDescriptor window;
+
+    /*
+     * There is only one window that is loaded from the --channels
+     * argument so that is what we use as the main window.
+     */
+    window.type_ = WindowType::Main;
+
+    // Load main window layout from config file so we can use the same geometry
+    const QRect configMainLayout = [] {
+        const QString windowLayoutFile =
+            combinePath(getPaths()->settingsDirectory,
+                        WindowManager::WINDOW_LAYOUT_FILENAME);
+
+        const WindowLayout configLayout =
+            WindowLayout::loadFromFile(windowLayoutFile);
+
+        for (const WindowDescriptor &window : configLayout.windows_)
+        {
+            if (window.type_ != WindowType::Main)
+                continue;
+
+            return window.geometry_;
+        }
+
+        return QRect(-1, -1, -1, -1);
+    }();
+
+    window.geometry_ = std::move(configMainLayout);
+
+    QStringList channelArgList = argValue.split(";");
+    for (const QString &channelArg : channelArgList)
+    {
+        if (channelArg.isEmpty())
+            continue;
+
+        // Twitch is default platform
+        QString platform = "t";
+        QString channelName = channelArg;
+
+        const QRegExp regExp("(.):(.*)");
+        if (regExp.indexIn(channelArg) != -1)
+        {
+            platform = regExp.cap(1);
+            channelName = regExp.cap(2);
+        }
+
+        // Twitch (default)
+        if (platform == "t")
+        {
+            TabDescriptor tab;
+
+            // Set first tab as selected
+            tab.selected_ = window.tabs_.empty();
+            tab.rootNode_ = SplitNodeDescriptor{"twitch", channelName};
+
+            window.tabs_.emplace_back(std::move(tab));
+        }
+    }
+
+    // Only respect --channels if we could actually parse any channels
+    if (!window.tabs_.empty())
+    {
+        this->dontSaveSettings = true;
+
+        layout.windows_.emplace_back(std::move(window));
+        this->customChannelLayout = std::move(layout);
+    }
+}
+
 static Args *instance = nullptr;
 
 void initArgs(const QApplication &app)

src/common/Args.hpp

@@ -1,7 +1,8 @@
 #pragma once
 
 #include <QApplication>
-#include <QJsonArray>
+#include <boost/optional.hpp>
+#include "common/WindowDescriptors.hpp"
 
 namespace chatterino {
 
@@ -15,7 +16,10 @@ public:
     bool crashRecovery{};
     bool shouldRunBrowserExtensionHost{};
     bool dontSaveSettings{};
-    QJsonArray channelsToJoin{};
+    boost::optional<WindowLayout> customChannelLayout;
+
+private:
+    void applyCustomChannelLayout(const QString &argValue);
 };
 
 void initArgs(const QApplication &app);

src/main.cpp

@@ -27,6 +27,33 @@ int main(int argc, char **argv)
     QCoreApplication::setApplicationVersion(CHATTERINO_VERSION);
     QCoreApplication::setOrganizationDomain("https://www.chatterino.com");
 
+    Paths *paths{};
+
+    try
+    {
+        paths = new Paths;
+    }
+    catch (std::runtime_error &error)
+    {
+        QMessageBox box;
+        if (Modes::instance().isPortable)
+        {
+            box.setText(
+                error.what() +
+                QStringLiteral(
+                    "\n\nInfo: Portable mode requires the application to "
+                    "be in a writeable location. If you don't want "
+                    "portable mode reinstall the application. "
+                    "https://chatterino.com."));
+        }
+        else
+        {
+            box.setText(error.what());
+        }
+        box.exec();
+        return 1;
+    }
+
     initArgs(a);
 
     // run in gui mode or browser extension host mode
@@ -37,13 +64,12 @@ int main(int argc, char **argv)
     else if (getArgs().printVersion)
     {
         auto version = Version::instance();
-        qCInfo(chatterinoMain).noquote()
+        qInfo().noquote() << QString("%1 (commit %2%3)")
-            << QString("%1 (commit %2%3)")
+                                 .arg(version.fullVersion())
-                   .arg(version.fullVersion())
+                                 .arg(version.commitHash())
-                   .arg(version.commitHash())
+                                 .arg(Modes::instance().isNightly
-                   .arg(Modes::instance().isNightly
+                                          ? ", " + version.dateOfBuild()
-                            ? ", " + version.dateOfBuild()
+                                          : "");
-                            : "");
     }
     else
     {
@@ -51,33 +77,6 @@ int main(int argc, char **argv)
         Helix::initialize();
         Kraken::initialize();
 
-        Paths *paths{};
-
-        try
-        {
-            paths = new Paths;
-        }
-        catch (std::runtime_error &error)
-        {
-            QMessageBox box;
-            if (Modes::instance().isPortable)
-            {
-                box.setText(
-                    error.what() +
-                    QStringLiteral(
-                        "\n\nInfo: Portable mode requires the application to "
-                        "be in a writeable location. If you don't want "
-                        "portable mode reinstall the application. "
-                        "https://chatterino.com."));
-            }
-            else
-            {
-                box.setText(error.what());
-            }
-            box.exec();
-            return 1;
-        }
-
         Settings settings(paths->settingsDirectory);
 
         runGui(a, *paths, settings);

src/singletons/WindowManager.cpp

@@ -37,8 +37,6 @@
 namespace chatterino {
 namespace {
 
-    const QString WINDOW_LAYOUT_FILENAME(QStringLiteral("window-layout.json"));
-
     boost::optional<bool> &shouldMoveOutOfBoundsWindow()
     {
         static boost::optional<bool> x;
@@ -47,6 +45,9 @@ namespace {
 
 }  // namespace
 
+const QString WindowManager::WINDOW_LAYOUT_FILENAME(
+    QStringLiteral("window-layout.json"));
+
 using SplitNode = SplitContainer::Node;
 using SplitDirection = SplitContainer::Direction;
 
@@ -83,8 +84,8 @@ void WindowManager::showAccountSelectPopup(QPoint point)
 }
 
 WindowManager::WindowManager()
-    : windowLayoutFilePath(
+    : windowLayoutFilePath(combinePath(getPaths()->settingsDirectory,
-          combinePath(getPaths()->settingsDirectory, WINDOW_LAYOUT_FILENAME))
+                                       WindowManager::WINDOW_LAYOUT_FILENAME))
 {
     qCDebug(chatterinoWindowmanager) << "init WindowManager";
 
@@ -297,7 +298,16 @@ void WindowManager::initialize(Settings &settings, Paths &paths)
     assert(!this->initialized_);
 
     {
-        auto windowLayout = this->loadWindowLayoutFromFile();
+        WindowLayout windowLayout;
+
+        if (getArgs().customChannelLayout)
+        {
+            windowLayout = getArgs().customChannelLayout.value();
+        }
+        else
+        {
+            windowLayout = this->loadWindowLayoutFromFile();
+        }
 
         this->emotePopupPos_ = windowLayout.emotePopupPos_;
 
@@ -305,10 +315,10 @@ void WindowManager::initialize(Settings &settings, Paths &paths)
     }
 
     // No main window has been created from loading, create an empty one
-    if (mainWindow_ == nullptr)
+    if (this->mainWindow_ == nullptr)
     {
-        mainWindow_ = &this->createWindow(WindowType::Main);
+        this->mainWindow_ = &this->createWindow(WindowType::Main);
-        mainWindow_->getNotebook().addPage(true);
+        this->mainWindow_->getNotebook().addPage(true);
     }
 
     settings.timestampFormat.connect([this](auto, auto) {

src/singletons/WindowManager.hpp

@@ -23,6 +23,8 @@ enum class SettingsDialogPreference;
 class WindowManager final : public Singleton
 {
 public:
+    static const QString WINDOW_LAYOUT_FILENAME;
+
     WindowManager();
 
     static void encodeChannel(IndirectChannel channel, QJsonObject &obj);