diff options
| -rw-r--r-- | plugingui/pluginconfig.h | 4 | ||||
| -rw-r--r-- | src/configfile.cc | 21 | ||||
| -rw-r--r-- | src/configfile.h | 9 | ||||
| -rw-r--r-- | test/Makefile.am | 3 |
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 |