summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndré Nusser <andre.nusser@googlemail.com>2020-04-10 18:17:51 +0200
committerBent Bisballe Nyeng <deva@aasimon.org>2020-04-10 19:08:26 +0200
commit33f1c0443680bc77ab3edfde19fd70dc6661f650 (patch)
tree5d548b8a14a92d087ed6dca7ed21c83f2134a465
parentd3d333b252925fbc95dd39fe73c5ce12a0017228 (diff)
Changes after discussion about review on IRC.configfile_refactor
-rw-r--r--plugingui/pluginconfig.h4
-rw-r--r--src/configfile.cc21
-rw-r--r--src/configfile.h9
-rw-r--r--test/Makefile.am3
4 files changed, 10 insertions, 27 deletions
diff --git a/plugingui/pluginconfig.h b/plugingui/pluginconfig.h
index 67d0ee8..db248e1 100644
--- a/plugingui/pluginconfig.h
+++ b/plugingui/pluginconfig.h
@@ -38,8 +38,8 @@ public:
Config();
~Config();
- bool load();
- bool save();
+ bool load() override;
+ bool save() override;
std::string defaultKitPath;
};
diff --git a/src/configfile.cc b/src/configfile.cc
index 6a5f176..436d090 100644
--- a/src/configfile.cc
+++ b/src/configfile.cc
@@ -28,6 +28,7 @@
#include <sys/stat.h>
+#include "directory.h"
#include "platform.h"
#if DG_PLATFORM == DG_PLATFORM_WINDOWS
@@ -52,9 +53,6 @@ const std::string sep = "/";
#if DG_PLATFORM == DG_PLATFORM_WINDOWS
const std::string config_dir_name = "DrumGizmo";
#else
-// TODO: Should we use this on OSX or is there another standard naming?
-// Response: Apparently this is also fine under OSX. There are also other conventions, but
-// one of the is dot files. However, feel free to check this, I am not sure about best practices.
const std::string config_dir_name = ".drumgizmo";
#endif
@@ -62,8 +60,7 @@ const std::string config_dir_name = ".drumgizmo";
// Return the path containing the config files.
std::string getConfigPath()
{
- // TODO: Move this utility function as a static function into directory.cc/h?
- // This seems like something we could use in other places as well.
+ // TODO: Move this utility function as a static function into directory.cc/h at some point?
std::string configpath;
#if DG_PLATFORM == DG_PLATFORM_WINDOWS
TCHAR szPath[256];
@@ -85,8 +82,7 @@ bool createConfigPath()
{
const std::string configpath = getConfigPath();
- struct stat st;
- if(stat(configpath.c_str(), &st) != 0)
+ if(!Directory::exists(configpath))
{
DEBUG(configfile, "No configuration exists, creating directory '%s'\n",
configpath.c_str());
@@ -121,9 +117,7 @@ ConfigFile::~ConfigFile()
}
}
-// TODO: Perhaps return a (homemade) optional carrying an error description on
-// failure? We might want to show the error in the UI and not just in the
-// terminal.
+// TODO: Make this return a homemade error variant when we have this project-wide.
bool ConfigFile::load()
{
DEBUG(configfile, "Loading config file...\n");
@@ -135,13 +129,6 @@ bool ConfigFile::load()
values.clear();
std::string line;
- // TODO: What happens if the parser encounters a windows newline on linux, or
- // the other way round?
- // Perhaps a unit-test for this would be in order?
- // Response: Why? Windows will probably not recognize the newline and Linux will
- // read the \r into the end of the line (which was the bug we had). What's the
- // point of testing undefined behavior? Do you want to catch it somehow and
- // warn the user in this case or what's the idea?
while(std::getline(current_file, line))
{
if(!parseLine(line))
diff --git a/src/configfile.h b/src/configfile.h
index a2f8db5..c17811b 100644
--- a/src/configfile.h
+++ b/src/configfile.h
@@ -39,19 +39,14 @@ public:
virtual bool load();
virtual bool save();
- virtual std::string getValue(const std::string& key) const;
- virtual void setValue(const std::string& key, const std::string& value);
+ std::string getValue(const std::string& key) const;
+ void setValue(const std::string& key, const std::string& value);
protected:
std::map<std::string, std::string> values;
std::string filename;
std::fstream current_file;
- // TODO: Does this have to be virtual?
- // Response: This is actually the only member function that has to be virtual.
- // I find it very ugly that we have any virtual functions in here, but it
- // seems that they were made virtual such that we can test this class properly.
- // What do you think? Test differently and make it all non-virtual?
virtual bool open(std::ios_base::openmode mode);
std::string readLine();
bool parseLine(const std::string& line);
diff --git a/test/Makefile.am b/test/Makefile.am
index d0f718d..8332cbe 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -132,10 +132,11 @@ lv2_SOURCES = \
configfile_CXXFLAGS = -DOUTPUT=\"configfile\" \
$(DEBUG_FLAGS) \
- -I$(top_srcdir)/hugin
+ -I$(top_srcdir)/hugin -I$(top_srcdir)/src
configfile_LDFLAGS =
configfile_SOURCES = \
$(top_srcdir)/src/configfile.cc \
+ $(top_srcdir)/src/directory.cc \
$(top_srcdir)/hugin/hugin.c \
dgtest.cc \
configtest.cc