From 1e57c2b3f6bd2a4f1756150e669eedffc29cf13d Mon Sep 17 00:00:00 2001 From: Aqua-sama Date: Wed, 3 Jan 2018 22:29:17 +0100 Subject: Fixed crash with Settings dialog - User Configuration gets completely written, instead of being just overrides --- CMakeLists.txt | 1 + docs/DESIGN.md | 41 ---------- lib/settings/CMakeLists.txt | 4 + lib/settings/configuration.cpp | 171 +++++++++++++++++++++++++--------------- lib/settings/configuration.h | 32 ++++---- lib/settings/settingsdialog.cpp | 1 - src/browser.cpp | 5 +- src/main.cpp | 61 +++++++------- 8 files changed, 164 insertions(+), 152 deletions(-) delete mode 100644 docs/DESIGN.md 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 Configuration::childrenSettings(const char* name) { std::vector 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 Configuration::childrenSettings(const char* name) std::vector Configuration::childrenGroups(const char* name) { std::vector 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 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(m_defaultCfg->lookup(path)); + break; + case Setting::TypeInt64: + v = static_cast(m_defaultCfg->lookup(path)); + break; + case Setting::TypeFloat: + v = static_cast(m_defaultCfg->lookup(path)); + break; + case Setting::TypeString: + v = static_cast(m_defaultCfg->lookup(path)); + break; + case Setting::TypeBoolean: + v = static_cast(m_defaultCfg->lookup(path)); + break; + case Setting::TypeGroup: + break; + case Setting::TypeArray: + break; + case Setting::TypeList: + break; + } } } template std::optional 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 r; // cast depending on type @@ -207,7 +251,7 @@ template std::optional Configuration::value(const char* path) const; template std::optional Configuration::value(const char* path) const; template -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 && !std::is_same_v) { setting = static_cast>(val); } else if constexpr(std::is_same_v) { - setting = static_cast(val).c_str(); + switch (setting.getType()) { + case Setting::TypeNone: + break; + + case Setting::TypeInt: + case Setting::TypeInt64: + setting = std::stoi(static_cast(val)); + break; + + case Setting::TypeFloat: + setting = std::stod(static_cast(val)); + break; + + case Setting::TypeString: + setting = static_cast(val).c_str(); + break; + + case Setting::TypeBoolean: + if(static_cast(val) == "true") { + setting = true; + } else if (static_cast(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 && !std::is_same_v) { - *userSetting = static_cast>(val); - } else if constexpr(std::is_same_v) { - *userSetting = static_cast(val).c_str(); - } else { - *userSetting = val; + changed = true; + return true; } + // the entry doesn't exist + return false; } -template void Configuration::setValue(std::string path, const int &val); -template void Configuration::setValue(std::string path, const unsigned int &val); -template void Configuration::setValue(std::string path, const long &val); -template void Configuration::setValue(std::string path, const unsigned long &val); - -template void Configuration::setValue(std::string path, const long long &val); -template void Configuration::setValue(std::string path, const unsigned long long &val); +template bool Configuration::setValue(std::string path, const int &val); +template bool Configuration::setValue(std::string path, const unsigned int &val); +template bool Configuration::setValue(std::string path, const long &val); +template bool Configuration::setValue(std::string path, const unsigned long &val); -template void Configuration::setValue(std::string path, const float &val); -template void Configuration::setValue(std::string path, const double &val); +template bool Configuration::setValue(std::string path, const long long &val); +template bool Configuration::setValue(std::string path, const unsigned long long &val); -template void Configuration::setValue(std::string path, const bool &val); +template bool Configuration::setValue(std::string path, const float &val); +template bool Configuration::setValue(std::string path, const double &val); -template void Configuration::setValue(std::string path, const std::string &val); +template bool Configuration::setValue(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 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 childrenSettings(const char *name = ""); std::vector childrenGroups(const char *name = ""); @@ -38,14 +40,12 @@ public: std::optional value(const char* path) const; template - 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 Configuration::value(const char* path) extern template std::optional Configuration::value(const char* path) const; // Settings::setValue<> -extern template void Configuration::setValue(std::string path, const int &val); -extern template void Configuration::setValue(std::string path, const unsigned int &val); -extern template void Configuration::setValue(std::string path, const long &val); -extern template void Configuration::setValue(std::string path, const unsigned long &val); +extern template bool Configuration::setValue(std::string path, const int &val); +extern template bool Configuration::setValue(std::string path, const unsigned int &val); +extern template bool Configuration::setValue(std::string path, const long &val); +extern template bool Configuration::setValue(std::string path, const unsigned long &val); -extern template void Configuration::setValue(std::string path, const long long &val); -extern template void Configuration::setValue(std::string path, const unsigned long long &val); +extern template bool Configuration::setValue(std::string path, const long long &val); +extern template bool Configuration::setValue(std::string path, const unsigned long long &val); -extern template void Configuration::setValue(std::string path, const float &val); -extern template void Configuration::setValue(std::string path, const double &val); +extern template bool Configuration::setValue(std::string path, const float &val); +extern template bool Configuration::setValue(std::string path, const double &val); -extern template void Configuration::setValue(std::string path, const bool &val); +extern template bool Configuration::setValue(std::string path, const bool &val); -extern template void Configuration::setValue(std::string path, const std::string &val); +extern template bool Configuration::setValue(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 #include -#include #include #include #include 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 -#include - 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("browser.firstRun", false); @@ -56,8 +56,6 @@ bool writeUserConfig(const std::string &path, Configuration &config) // downloads.path std::string downloadsPath = config.value("downloads.path").value(); config.setValue("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 config = std::make_shared(); - 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("browser.firstRun").value() || parser.isSet(generateUserConfigOption)) { + if(config->value("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()); -- cgit v1.2.1