Fixed crash with Settings dialog

- User Configuration gets completely written, instead of being just
overrides

8 files changed, 164 insertions, 152 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c6c9235..600ac8e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -3,6 +3,7 @@ project(smolbote)
 
 # Options
 option(UseLibCpp "Use libc++ over stdlibc++ (requires clang)" OFF)
+option(CLikeConfig "Use a C-like style for the config" ON)
 
 # Libraries
 find_package(Qt5Core REQUIRED)
diff --git a/docs/DESIGN.md b/docs/DESIGN.md
deleted file mode 100644
index 58aa149..0000000
--- a/docs/DESIGN.md
+++ /dev/null
@@ -1,41 +0,0 @@
-# smolbote
-
-The aim of smolbote is to be a lightweight and fast, to the point web browser.
-
-## Principles
-
-### Simple is good.
-'without unnecessary additions'. Do one thing and do it well.
-
-### User-centric is user-friendly.
-Software is there to serve the user, not the other way around.
-
-### Code correctness is truth.
-Use up-to-date standards and compilers.
-
-## Version 1.0
-
-### Browser
-* Profiles
-  - Application-level profile manager
-* Configuration
-  - Default configuration and user overrides
-  - Auto-generating settings dialog
-
-### Main Window
-* Tabs
-  - Moving tabs between windows [todo]
-  - Tab context menu [todo]
-* Page actions
-  - Search [todo]
-  - Zoom [todo]
-  - Print [todo]
-* Navigation bar
-  - Back and Forward history [todo]
-* Address bar
-
-### WebEngine
-* UrlRequestInterceptor
-  - Support for AdBlock-style lists
-* Cookie Filter
-
diff --git a/lib/settings/CMakeLists.txt b/lib/settings/CMakeLists.txt
index 983e8ca..26d8525 100644
--- a/lib/settings/CMakeLists.txt
+++ b/lib/settings/CMakeLists.txt
@@ -7,4 +7,8 @@ add_library(configuration
         settingsdialog.h
         settingsdialog.ui)
 
+if (CLikeConfig)
+    target_compile_definitions(configuration PRIVATE C_LIKE_CONFIG)
+endif (CLikeConfig)
+
 target_link_libraries(configuration config++ Qt5::Widgets)
diff --git a/lib/settings/configuration.cpp b/lib/settings/configuration.cpp
index 4603779..1fa1fbf 100644
--- a/lib/settings/configuration.cpp
+++ b/lib/settings/configuration.cpp
@@ -21,15 +21,14 @@ Configuration::Configuration()
     // fsync after writing and before closing
     m_userCfg->setOption(Config::OptionFsync, true);
 
+    // make sure the cfgPath is empty
+    m_userCfgPath = std::string();
+
     m_defaultCfg = new Config();
 }
 
 Configuration::~Configuration()
 {
-    if(!m_userCfgPath.empty()) {
-        m_userCfg->writeFile(m_userCfgPath.c_str());
-    }
-
     delete m_userCfg;
     delete m_defaultCfg;
 }
@@ -47,22 +46,42 @@ bool Configuration::readUserConfiguration(const std::string &path)
     return  true;
 }
 
+bool Configuration::parse(const std::string &contents)
+{
+    try {
+        m_userCfg->readString(contents);
+    } catch (ParseException) {
+        return false;
+    }
+    return true;
+}
+
 bool Configuration::writeUserConfiguration(const std::string &path)
 {
     m_userCfgPath = path;
     try {
-        m_userCfg->writeFile(path.c_str());
+        m_userCfg->writeFile(m_userCfgPath.c_str());
     } catch (FileIOException) {
         return false;
     }
     return true;
 }
 
-bool Configuration::readDefaultConfiguration(const std::string &data)
+bool Configuration::writeIfNeeded()
 {
     try {
-        m_defaultCfg->readString(data);
-    } catch(ParseException) {
+        m_userCfg->writeFile(m_userCfgPath.c_str());
+    } catch (FileIOException) {
+        return false;
+    }
+    return true;
+}
+
+bool Configuration::parseDefaultConfiguration(const std::string &contents)
+{
+    try {
+        m_defaultCfg->readString(contents);
+    } catch (ParseException) {
         return false;
     }
     return true;
@@ -71,7 +90,7 @@ bool Configuration::readDefaultConfiguration(const std::string &data)
 std::vector<std::string> Configuration::childrenSettings(const char* name)
 {
     std::vector<std::string> groupNames;
-    const Setting &root = m_defaultCfg->lookup(name);
+    const Setting &root = m_userCfg->lookup(name);
 
     for(const Setting &setting : root) {
         if(setting.getType() != Setting::TypeGroup) {
@@ -85,7 +104,7 @@ std::vector<std::string> Configuration::childrenSettings(const char* name)
 std::vector<std::string> Configuration::childrenGroups(const char* name)
 {
     std::vector<std::string> groupNames;
-    const Setting &root = m_defaultCfg->lookup(name);
+    const Setting &root = m_userCfg->lookup(name);
 
     for(const Setting &setting : root) {
         if(setting.getType() == Setting::TypeGroup) {
@@ -98,20 +117,45 @@ std::vector<std::string> Configuration::childrenGroups(const char* name)
 void Configuration::resetValue(const char *path)
 {
     if(m_userCfg->exists(path)) {
-        m_userCfg->getRoot().remove(path);
+        Setting &v = m_userCfg->lookup(path);
+        switch (v.getType()) {
+        case Setting::TypeNone:
+            break;
+        case Setting::TypeInt:
+            v = static_cast<int>(m_defaultCfg->lookup(path));
+            break;
+        case Setting::TypeInt64:
+            v = static_cast<long>(m_defaultCfg->lookup(path));
+            break;
+        case Setting::TypeFloat:
+            v = static_cast<double>(m_defaultCfg->lookup(path));
+            break;
+        case Setting::TypeString:
+            v = static_cast<const char *>(m_defaultCfg->lookup(path));
+            break;
+        case Setting::TypeBoolean:
+            v = static_cast<bool>(m_defaultCfg->lookup(path));
+            break;
+        case Setting::TypeGroup:
+            break;
+        case Setting::TypeArray:
+            break;
+        case Setting::TypeList:
+            break;
+        }
     }
 }
 
 template<typename T>
 std::optional<T> Configuration::value(const char* path) const
 {
-    Config *conf = getConfig(path);
-    if(conf == nullptr) {
+    // if setting doesn't exist, give back a nullopt
+    if(!m_userCfg->exists(path)) {
         return std::nullopt;
     }
 
     // setting was found
-    const Setting &setting = conf->lookup(path);
+    const Setting &setting = m_userCfg->lookup(path);
     std::optional<T> r;
 
     // cast depending on type
@@ -207,7 +251,7 @@ template std::optional<bool> Configuration::value<bool>(const char* path) const;
 template std::optional<std::string> Configuration::value<std::string>(const char* path) const;
 
 template<typename T>
-void Configuration::setValue(std::string path, const T &val)
+bool Configuration::setValue(std::string path, const T &val)
 {
     if(m_userCfg->exists(path)) {
         Setting &setting = m_userCfg->lookup(path);
@@ -215,70 +259,69 @@ void Configuration::setValue(std::string path, const T &val)
         if constexpr(std::is_unsigned_v<T> && !std::is_same_v<T, bool>) {
             setting = static_cast<typename std::make_signed_t<T>>(val);
         } else if constexpr(std::is_same_v<T, std::string>) {
-            setting = static_cast<std::string>(val).c_str();
+            switch (setting.getType()) {
+            case Setting::TypeNone:
+                break;
+
+            case Setting::TypeInt:
+            case Setting::TypeInt64:
+                setting = std::stoi(static_cast<std::string>(val));
+                break;
+
+            case Setting::TypeFloat:
+                setting = std::stod(static_cast<std::string>(val));
+                break;
+
+            case Setting::TypeString:
+                setting = static_cast<std::string>(val).c_str();
+                break;
+
+            case Setting::TypeBoolean:
+                if(static_cast<std::string>(val) == "true") {
+                    setting = true;
+                } else if (static_cast<std::string>(val) == "false") {
+                    setting = false;
+                }
+                break;
+
+            case Setting::TypeGroup:
+                break;
+            case Setting::TypeArray:
+                break;
+            case Setting::TypeList:
+                break;
+            }
+
         } else {
             setting = val;
         }
-        return;
-    }
-
-    // the entry doesn't exist, so we need to create it
-    // libconfig can't create an entry from a path, we need to get its root, and then add it
-
-    // this will error out if entry being added is not in the default config
-
-    std::istringstream p(path);
-    Setting *userSetting = &m_userCfg->getRoot();
-    Setting *defaultSetting = &m_defaultCfg->getRoot();
-
-    std::string i;
-    while (std::getline(p, i, '.')) {
-        defaultSetting = &defaultSetting->lookup(i);
-        // check if user setting exists, if not, create it
-        if(!userSetting->exists(i)) {
-            userSetting = &userSetting->add(i, defaultSetting->getType());
-        }
-    }
 
-    if constexpr(std::is_unsigned_v<T> && !std::is_same_v<T, bool>) {
-        *userSetting = static_cast<typename std::make_signed_t<T>>(val);
-    } else if constexpr(std::is_same_v<T, std::string>) {
-        *userSetting = static_cast<std::string>(val).c_str();
-    } else {
-        *userSetting = val;
+        changed = true;
+        return true;
     }
 
+    // the entry doesn't exist
+    return false;
 }
 
-template void Configuration::setValue<int>(std::string path, const int &val);
-template void Configuration::setValue<unsigned int>(std::string path, const unsigned int &val);
-template void Configuration::setValue<long>(std::string path, const long &val);
-template void Configuration::setValue<unsigned long>(std::string path, const unsigned long &val);
-
-template void Configuration::setValue<long long>(std::string path, const long long &val);
-template void Configuration::setValue<unsigned long long>(std::string path, const unsigned long long &val);
+template bool Configuration::setValue<int>(std::string path, const int &val);
+template bool Configuration::setValue<unsigned int>(std::string path, const unsigned int &val);
+template bool Configuration::setValue<long>(std::string path, const long &val);
+template bool Configuration::setValue<unsigned long>(std::string path, const unsigned long &val);
 
-template void Configuration::setValue<float>(std::string path, const float &val);
-template void Configuration::setValue<double>(std::string path, const double &val);
+template bool Configuration::setValue<long long>(std::string path, const long long &val);
+template bool Configuration::setValue<unsigned long long>(std::string path, const unsigned long long &val);
 
-template void Configuration::setValue<bool>(std::string path, const bool &val);
+template bool Configuration::setValue<float>(std::string path, const float &val);
+template bool Configuration::setValue<double>(std::string path, const double &val);
 
-template void Configuration::setValue<std::string>(std::string path, const std::string &val);
+template bool Configuration::setValue<bool>(std::string path, const bool &val);
 
-Config *Configuration::getConfig(const char* path) const
-{
-    if(m_userCfg->exists(path)) {
-        return m_userCfg;
-    } else if(m_defaultCfg->exists(path)) {
-        return m_defaultCfg;
-    } else {
-        return nullptr;
-    }
-}
+template bool Configuration::setValue<std::string>(std::string path, const std::string &val);
 
 std::string &patchHome(std::string &path, const std::string &home)
 {
-    const size_t location = path.find("~");
+    const size_t location = path.find('~');
     if(location != std::string::npos) {
         return path.replace(location, 1, home);
     }
diff --git a/lib/settings/configuration.h b/lib/settings/configuration.h
index 1909eb8..b258cb3 100644
--- a/lib/settings/configuration.h
+++ b/lib/settings/configuration.h
@@ -25,9 +25,11 @@ public:
     ~Configuration();
 
     bool readUserConfiguration(const std::string &path);
+    bool parse(const std::string &contents);
     bool writeUserConfiguration(const std::string &path);
+    bool writeIfNeeded();
 
-    bool readDefaultConfiguration(const std::string &data);
+    bool parseDefaultConfiguration(const std::string &contents);
 
     std::vector<std::string> childrenSettings(const char *name = "");
     std::vector<std::string> childrenGroups(const char *name = "");
@@ -38,14 +40,12 @@ public:
     std::optional<T> value(const char* path) const;
 
     template<typename T>
-    void setValue(std::string path, const T &val);
+    bool setValue(std::string path, const T &val);
 
 private:
-    libconfig::Config* getConfig(const char* path) const;
-
+    bool changed = false;
     std::string m_userCfgPath;
-    libconfig::Config *m_userCfg;
-    libconfig::Config *m_defaultCfg;
+    libconfig::Config *m_userCfg, *m_defaultCfg;
 };
 
 // replace ~ with home
@@ -71,19 +71,19 @@ extern template std::optional<bool> Configuration::value<bool>(const char* path)
 extern template std::optional<std::string> Configuration::value<std::string>(const char* path) const;
 
 // Settings::setValue<>
-extern template void Configuration::setValue<int>(std::string path, const int &val);
-extern template void Configuration::setValue<unsigned int>(std::string path, const unsigned int &val);
-extern template void Configuration::setValue<long>(std::string path, const long &val);
-extern template void Configuration::setValue<unsigned long>(std::string path, const unsigned long &val);
+extern template bool Configuration::setValue<int>(std::string path, const int &val);
+extern template bool Configuration::setValue<unsigned int>(std::string path, const unsigned int &val);
+extern template bool Configuration::setValue<long>(std::string path, const long &val);
+extern template bool Configuration::setValue<unsigned long>(std::string path, const unsigned long &val);
 
-extern template void Configuration::setValue<long long>(std::string path, const long long &val);
-extern template void Configuration::setValue<unsigned long long>(std::string path, const unsigned long long &val);
+extern template bool Configuration::setValue<long long>(std::string path, const long long &val);
+extern template bool Configuration::setValue<unsigned long long>(std::string path, const unsigned long long &val);
 
-extern template void Configuration::setValue<float>(std::string path, const float &val);
-extern template void Configuration::setValue<double>(std::string path, const double &val);
+extern template bool Configuration::setValue<float>(std::string path, const float &val);
+extern template bool Configuration::setValue<double>(std::string path, const double &val);
 
-extern template void Configuration::setValue<bool>(std::string path, const bool &val);
+extern template bool Configuration::setValue<bool>(std::string path, const bool &val);
 
-extern template void Configuration::setValue<std::string>(std::string path, const std::string &val);
+extern template bool Configuration::setValue<std::string>(std::string path, const std::string &val);
 
 #endif // CONFIGURATION_H
diff --git a/lib/settings/settingsdialog.cpp b/lib/settings/settingsdialog.cpp
index 6de3894..9a589d6 100644
--- a/lib/settings/settingsdialog.cpp
+++ b/lib/settings/settingsdialog.cpp
@@ -11,7 +11,6 @@
 #include "configuration.h"
 #include <QScrollArea>
 #include <QFormLayout>
-#include <QHBoxLayout>
 #include <QLineEdit>
 #include <QToolButton>
 #include <QGroupBox>
diff --git a/src/browser.cpp b/src/browser.cpp
index d7834f4..aad80bb 100644
--- a/src/browser.cpp
+++ b/src/browser.cpp
@@ -13,8 +13,6 @@
 #include "mainwindow.h"
 #include <QDir>
 
-#include <QWebEngineDownloadItem>
-
 Browser::Browser(int &argc, char *argv[]) :
     SingleApplication(argc, argv)
 {
@@ -26,6 +24,9 @@ Browser::Browser(int &argc, char *argv[]) :
 
 Browser::~Browser()
 {
+    if(m_config) {
+        m_config->writeIfNeeded();
+    }
     if(m_bookmarksManager) {
         m_bookmarksManager->save();
     }
diff --git a/src/main.cpp b/src/main.cpp
index ffb7a4d..fb9eeba 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -22,7 +22,7 @@
 #endif
 
 // read config into std::string, supports qrc
-inline std::string readConfig(QString path)
+inline std::string readConfig(const QString &path)
 {
     QFile conf(path);
     std::string ret;
@@ -33,7 +33,7 @@ inline std::string readConfig(QString path)
     return ret;
 }
 
-bool writeUserConfig(const std::string &path, Configuration &config)
+void bootstrapUserConfig(const std::string &path, Configuration &config)
 {
     // set firstRun to false so we don't re-init on every run
     config.setValue<bool>("browser.firstRun", false);
@@ -56,8 +56,6 @@ bool writeUserConfig(const std::string &path, Configuration &config)
     // downloads.path
     std::string downloadsPath = config.value<std::string>("downloads.path").value();
     config.setValue<std::string>("downloads.path", patchHome(downloadsPath, home.toStdString()));
-
-    return config.writeUserConfiguration(path);
 }
 
 int main(int argc, char *argv[])
@@ -116,6 +114,8 @@ int main(int argc, char *argv[])
 #ifdef QT_DEBUG
     qDebug("config=%s", qUtf8Printable(parser.value(configOption)));
     qDebug("default-config=%s", qUtf8Printable(parser.value(defaultConfigOption)));
+    qDebug("socket=%s", qUtf8Printable(parser.value(socketOption)));
+    qDebug("profile=%s", qUtf8Printable(parser.value(profileOption)));
 #endif
 
     if(parser.isSet(printDefaultConfigOption)) {
@@ -124,50 +124,55 @@ int main(int argc, char *argv[])
         return 0;
     }
 
-    // TODO: check for other instances
-    // if we socket hasn't been disabled (socket is not none)
-    if(parser.value(socketOption) != "none") {
-        bool bindOk = instance.bindLocalSocket(parser.value(socketOption));
-        if(bindOk) {
-            qDebug("Connected to local socket: %s", qUtf8Printable(instance.serverName()));
-        } else {
-            // pass arguments to new instance
-            return instance.sendMessage(parser.value(profileOption), parser.isSet(newWindowOption), parser.positionalArguments());
-        }
-    }
-
     std::shared_ptr<Configuration> config = std::make_shared<Configuration>();
-    config->readDefaultConfiguration(readConfig(parser.value(defaultConfigOption)));
     config->readUserConfiguration(parser.value(configOption).toStdString());
+    config->parseDefaultConfiguration(readConfig(parser.value(defaultConfigOption)));
 
     // check if first run
-    if(config->value<bool>("browser.firstRun").value() || parser.isSet(generateUserConfigOption)) {
+    if(config->value<bool>("browser.firstRun").value_or(true) || parser.isSet(generateUserConfigOption)) {
         // create a user config file
+
         QString path = parser.value(configOption);
-        // there may be no smolbote.cfg
+        // make sure we have a path
         if(path.isEmpty()) {
             path = QStandardPaths::writableLocation(QStandardPaths::AppConfigLocation) + "/smolbote.cfg";
         }
-
+        // make sure the directory exists
         QDir configDir = QFileInfo(path).absoluteDir();
         if(!configDir.exists()) {
-            bool mkpathSuccess = configDir.mkpath(".");
-#ifdef QT_DEBUG
-            qDebug("mkpath %s: %s", qUtf8Printable(configDir.absolutePath()), mkpathSuccess ? "ok" : "failed");
-#endif
+            configDir.mkpath(".");
         }
 
-#ifdef QT_DEBUG
-        qDebug("Generating user config on first run to %s", qUtf8Printable(path));
-#endif
+        // remove any existing config
+        if(QFile::exists(path)) {
+            QFile::remove(path);
+        }
+
+        config->parse(readConfig(parser.value(defaultConfigOption)));
 
-        qDebug("Saving config to: %s - %s", qUtf8Printable(path), writeUserConfig(path.toStdString(), *config) ? "ok" : "failed");
+        // patch paths
+        bootstrapUserConfig(path.toStdString(), *config);
 
+        std::cout << "Generating config to: " << qUtf8Printable(path) << (config->writeUserConfiguration(path.toStdString()) ? " ok" : " failed") << std::endl;
+
+        // exit if this is the only thing we needed to do
         if(parser.isSet(generateUserConfigOption)) {
             return 0;
         }
     }
 
+    // check for other instances
+    // if we socket hasn't been disabled (socket is not none)
+    if(parser.value(socketOption) != "none") {
+        bool bindOk = instance.bindLocalSocket(parser.value(socketOption));
+        if(bindOk) {
+            qDebug("Connected to local socket: %s", qUtf8Printable(instance.serverName()));
+        } else {
+            // pass arguments to new instance
+            return instance.sendMessage(parser.value(profileOption), parser.isSet(newWindowOption), parser.positionalArguments());
+        }
+    }
+
     instance.setConfiguration(config);
 
     instance.createSession(parser.value(profileOption), parser.isSet(newWindowOption), parser.positionalArguments());