commit 7a618ddc95ac73f91010d26254bb8275b2a25415
parent 08f3b172bd25299babb7c9bd67e7e620de697a4b
Author: cfillion <cfillion@users.noreply.github.com>
Date: Thu, 15 Dec 2016 00:48:20 -0500
refactoring – remove useless logic for imcomplete trees
Diffstat:
9 files changed, 82 insertions(+), 208 deletions(-)
diff --git a/src/index.hpp b/src/index.hpp
@@ -73,7 +73,7 @@ private:
class Category {
public:
- Category(const std::string &name, const Index * = nullptr);
+ Category(const std::string &name, const Index *);
~Category();
const Index *index() const { return m_index; }
diff --git a/src/package.hpp b/src/package.hpp
@@ -41,7 +41,7 @@ public:
static const std::string &displayName(const std::string &name,
const std::string &desc, bool enableDesc = true);
- Package(const Type, const std::string &name, const Category * = nullptr);
+ Package(const Type, const std::string &name, const Category *);
~Package();
const Category *category() const { return m_category; }
diff --git a/src/source.cpp b/src/source.cpp
@@ -57,45 +57,28 @@ Source::Source(const string &file, const string &url, const Version *ver)
throw reapack_error("empty source url");
}
-const Package *Source::package() const
-{
- return m_version ? m_version->package() : nullptr;
-}
-
Package::Type Source::type() const
{
if(m_type)
return m_type;
- else if(const Package *pkg = package())
- return pkg->type();
else
- return Package::UnknownType;
+ return m_version->package()->type();
}
const string &Source::file() const
{
if(!m_file.empty())
return m_file;
-
- if(const Package *pkg = package())
- return pkg->name();
else
- throw reapack_error("empty source file name and no package");
+ return m_version->package()->name();
}
void Source::setSections(int sections)
{
if(type() != Package::ScriptType)
return;
- else if(sections == ImplicitSection) {
- const Package *pkg = package();
- const Category *cat = pkg ? pkg->category() : nullptr;
-
- if(!cat)
- throw reapack_error("cannot resolve implicit section: category is unset");
-
- sections = detectSection(cat->name());
- }
+ else if(sections == ImplicitSection)
+ sections = detectSection(m_version->package()->category()->name());
m_sections = sections;
}
@@ -112,14 +95,6 @@ string Source::fullName() const
Path Source::targetPath() const
{
- const Package *pkg = package();
- if(!pkg)
- throw reapack_error("no package associated with this source");
-
- const Category *cat = pkg->category();
- if(!cat || !cat->index())
- throw reapack_error("category or index is unset");
-
Path path;
const auto type = this->type();
@@ -144,23 +119,25 @@ Path Source::targetPath() const
path.append("LangPack");
break;
case Package::UnknownType:
- // The package has an unsupported type, so we return an empty path.
+ // The package has an unsupported type, so we make an empty path.
// The empty path won't be used because the category will reject
// this package right away. Maybe the parser should not bother with loading
// unsupported packages at all anyway... But then in the future
// we might want to display unsupported packages in the interface.
- return path;
+ break;
}
// append the rest of the path
switch(type) {
case Package::ScriptType:
- case Package::EffectType:
+ case Package::EffectType: {
+ const Category *cat = m_version->package()->category();
path.append(cat->index()->name());
// only allow directory traversal up to the index name
path += Path(cat->name()) + file();
break;
+ }
case Package::ExtensionType:
case Package::ThemeType:
case Package::DataType:
@@ -168,7 +145,7 @@ Path Source::targetPath() const
path.append(file(), false);
break;
case Package::UnknownType:
- break; // will never happens, but compiler are dumb
+ break;
}
return path;
diff --git a/src/source.hpp b/src/source.hpp
@@ -38,8 +38,7 @@ public:
static Section getSection(const char *);
static Section detectSection(const std::string &category);
- Source(const std::string &file, const std::string &url,
- const Version * = nullptr);
+ Source(const std::string &file, const std::string &url, const Version *);
void setPlatform(Platform p) { m_platform = p; }
Platform platform() const { return m_platform; }
@@ -53,7 +52,6 @@ public:
int sections() const { return m_sections; }
const Version *version() const { return m_version; }
- const Package *package() const;
std::string fullName() const;
Path targetPath() const;
@@ -64,6 +62,7 @@ private:
std::string m_file;
std::string m_url;
int m_sections;
+ Path m_targetPath;
const Version *m_version;
};
diff --git a/src/version.cpp b/src/version.cpp
@@ -103,8 +103,11 @@ bool Version::tryParse(const string &str)
string Version::fullName() const
{
- const string fName = 'v' + m_name;
- return m_package ? m_package->fullName() + " " + fName : fName;
+ string name = m_package->fullName();
+ name += " v";
+ name += m_name;
+
+ return name;
}
string Version::displayAuthor() const
diff --git a/test/index.cpp b/test/index.cpp
@@ -151,11 +151,11 @@ TEST_CASE("add a package", M) {
}
TEST_CASE("add owned package", M) {
- Category cat1("a");
+ Category cat1("a", nullptr);
Package *pack = new Package(Package::ScriptType, "name", &cat1);
try {
- Category cat2("b");
+ Category cat2("b", nullptr);
cat2.addPackage(pack);
FAIL();
}
@@ -166,14 +166,14 @@ TEST_CASE("add owned package", M) {
}
TEST_CASE("drop empty package", M) {
- Category cat("a");
+ Category cat("a", nullptr);
const Package pkg(Package::ScriptType, "name", &cat);
REQUIRE_FALSE(cat.addPackage(&pkg));
REQUIRE(cat.packages().empty());
}
TEST_CASE("drop unknown package", M) {
- Category cat("a");
+ Category cat("a", nullptr);
const Package pkg(Package::UnknownType, "name", &cat);
REQUIRE_FALSE(cat.addPackage(&pkg));
REQUIRE(cat.packages().size() == 0);
@@ -190,9 +190,6 @@ TEST_CASE("empty category name", M) {
}
TEST_CASE("category full name", M) {
- Category cat1("Category Name");
- REQUIRE(cat1.fullName() == "Category Name");
-
Index ri("Remote Name");
Category cat2("Category Name", &ri);
REQUIRE(cat2.fullName() == "Remote Name/Category Name");
diff --git a/test/package.cpp b/test/package.cpp
@@ -64,7 +64,7 @@ TEST_CASE("package type to string", M) {
TEST_CASE("invalid package name", M) {
SECTION("empty") {
try {
- Package pack(Package::ScriptType, string());
+ Package pack(Package::ScriptType, string(), nullptr);
FAIL();
}
catch(const reapack_error &e) {
@@ -74,7 +74,7 @@ TEST_CASE("invalid package name", M) {
SECTION("slash") {
try {
- Package pack(Package::ScriptType, "hello/world");
+ Package pack(Package::ScriptType, "hello/world", nullptr);
FAIL();
}
catch(const reapack_error &e) {
@@ -84,7 +84,7 @@ TEST_CASE("invalid package name", M) {
SECTION("backslash") {
try {
- Package pack(Package::ScriptType, "hello\\world");
+ Package pack(Package::ScriptType, "hello\\world", nullptr);
FAIL();
}
catch(const reapack_error &e) {
@@ -180,7 +180,7 @@ TEST_CASE("pre-release updates", M) {
}
TEST_CASE("drop empty version", M) {
- Package pack(Package::ScriptType, "a");
+ Package pack(Package::ScriptType, "a", nullptr);
const Version ver("1", &pack);
REQUIRE_FALSE(pack.addVersion(&ver));
REQUIRE(pack.versions().empty());
@@ -188,8 +188,8 @@ TEST_CASE("drop empty version", M) {
}
TEST_CASE("add owned version", M) {
- Package pack1(Package::ScriptType, "a");
- Package pack2(Package::ScriptType, "a");
+ Package pack1(Package::ScriptType, "a", nullptr);
+ Package pack2(Package::ScriptType, "a", nullptr);
Version *ver = new Version("1", &pack1);
@@ -240,36 +240,19 @@ TEST_CASE("find matching version", M) {
REQUIRE(pack.findVersion(Version("2")) == nullptr);
}
-TEST_CASE("full name", M) {
- SECTION("no category") {
- const Package pack(Package::ScriptType, "file.name");
- REQUIRE(pack.fullName() == "file.name");
- }
-
- SECTION("with category") {
- const Category cat("Category Name");
- const Package pack(Package::ScriptType, "file.name", &cat);
- REQUIRE(pack.fullName() == "Category Name/file.name");
- }
+TEST_CASE("package full name", M) {
+ const Index ri("Index Name");
+ const Category cat("Category Name", &ri);
+ Package pack(Package::ScriptType, "file.name", &cat);
- SECTION("with index") {
- const Index ri("Remote Name");
- const Category cat("Category Name", &ri);
- const Package pack(Package::ScriptType, "file.name", &cat);
+ REQUIRE(pack.fullName() == "Index Name/Category Name/file.name");
- REQUIRE(pack.fullName() == "Remote Name/Category Name/file.name");
- }
-
- SECTION("with description") {
- const Category cat("Category Name");
- Package pack(Package::ScriptType, "file.name", &cat);
- pack.setDescription("Hello World");
- REQUIRE(pack.fullName() == "Category Name/Hello World");
- }
+ pack.setDescription("Hello World");
+ REQUIRE(pack.fullName() == "Index Name/Category Name/Hello World");
}
TEST_CASE("package description", M) {
- Package pack(Package::ScriptType, "test.lua");
+ Package pack(Package::ScriptType, "test.lua", nullptr);
REQUIRE(pack.description().empty());
pack.setDescription("hello world");
@@ -277,7 +260,7 @@ TEST_CASE("package description", M) {
}
TEST_CASE("package display name", M) {
- Package pack(Package::ScriptType, "test.lua");
+ Package pack(Package::ScriptType, "test.lua", nullptr);
REQUIRE(pack.displayName() == "test.lua");
REQUIRE(pack.displayName(false) == "test.lua");
REQUIRE(pack.displayName(true) == "test.lua");
diff --git a/test/source.cpp b/test/source.cpp
@@ -10,8 +10,16 @@ using namespace std;
static const char *M = "[source]";
+#define MAKE_VERSION \
+ Index ri("Index Name"); \
+ Category cat("Category Name", &ri); \
+ Package pkg(Package::DataType, "Package Name", &cat); \
+ Version ver("1.0", &pkg);
+
TEST_CASE("source platform", M) {
- Source src({}, "url");
+ MAKE_VERSION;
+
+ Source src({}, "url", &ver);
REQUIRE(src.platform() == Platform::GenericPlatform);
src.setPlatform(Platform::UnknownPlatform);
@@ -22,8 +30,10 @@ TEST_CASE("source platform", M) {
}
TEST_CASE("source type override", M) {
- Source src({}, "url");
- REQUIRE(src.type() == Package::UnknownType);
+ MAKE_VERSION;
+
+ Source src({}, "url", &ver);
+ REQUIRE(src.type() == Package::DataType);
REQUIRE(src.typeOverride() == Package::UnknownType);
src.setTypeOverride(Package::ScriptType);
@@ -32,29 +42,17 @@ TEST_CASE("source type override", M) {
}
TEST_CASE("source type from package", M) {
- Package pack(Package::EffectType, "package name");
- Version ver("1.0", &pack);
+ MAKE_VERSION;
Source src({}, "url", &ver);
REQUIRE(src.version() == &ver);
- REQUIRE(src.type() == Package::EffectType);
+ REQUIRE(src.type() == Package::DataType);
+
src.setTypeOverride(Package::ScriptType);
REQUIRE(src.type() == Package::ScriptType);
}
-TEST_CASE("empty source file name and no package", M) {
- const Source source({}, "url");
-
- try {
- (void)source.file();
- FAIL();
- }
- catch(const reapack_error &e) {
- REQUIRE(string(e.what()) == "empty source file name and no package");
- }
-}
-
TEST_CASE("parse file section", M) {
REQUIRE(-1 == Source::getSection("true"));
REQUIRE(0 == Source::getSection("hello"));
@@ -63,40 +61,32 @@ TEST_CASE("parse file section", M) {
}
TEST_CASE("explicit source section", M) {
- SECTION("script type override") {
- Source source("filename", "url");
+ MAKE_VERSION;
+
+ SECTION("script type") {
+ Source source("filename", "url", &ver);
REQUIRE(source.sections() == 0);
source.setTypeOverride(Package::ScriptType);
+ CHECK(source.type() == Package::ScriptType);
+
source.setSections(Source::MainSection | Source::MIDIEditorSection);
REQUIRE(source.sections() == (Source::MainSection | Source::MIDIEditorSection));
}
- SECTION("other type override") {
- Source source("filename", "url");
- source.setTypeOverride(Package::EffectType);
- source.setSections(Source::MainSection);
- REQUIRE(source.sections() == 0);
- }
-
- SECTION("package type") {
- Package pack(Package::ScriptType, "package name");
- Version ver("1.0", &pack);
+ SECTION("other type") {
Source source("filename", "url", &ver);
source.setSections(Source::MainSection);
- REQUIRE(source.sections() == Source::MainSection);
- }
-
- SECTION("no package, no type override") {
- Source source("filename", "url");
- source.setSections(Source::MainSection);
- // should not crash!
+ CHECK(source.type() == Package::DataType);
+ REQUIRE(source.sections() == 0);
}
}
TEST_CASE("implicit source section") {
+ Index ri("Index Name");
+
SECTION("main") {
- Category cat("Category Name");
+ Category cat("Category Name", &ri);
Package pack(Package::ScriptType, "package name", &cat);
Version ver("1.0", &pack);
@@ -106,7 +96,7 @@ TEST_CASE("implicit source section") {
}
SECTION("midi editor") {
- Category cat("MIDI Editor");
+ Category cat("MIDI Editor", &ri);
Package pack(Package::ScriptType, "package name", &cat);
Version ver("1.0", &pack);
@@ -114,17 +104,6 @@ TEST_CASE("implicit source section") {
source.setSections(Source::ImplicitSection);
REQUIRE(source.sections() == Source::MIDIEditorSection);
}
-
- SECTION("no category") {
- Source source("filename", "url");
- source.setTypeOverride(Package::ScriptType);
-
- try {
- source.setSections(Source::ImplicitSection);
- FAIL(); // should throw (or crash if buggy, but not do nothing)
- }
- catch(const reapack_error &) {}
- }
}
TEST_CASE("implicit section detection", M) {
@@ -137,8 +116,10 @@ TEST_CASE("implicit section detection", M) {
}
TEST_CASE("empty source url", M) {
+ MAKE_VERSION;
+
try {
- const Source source("filename", {});
+ const Source source("filename", {}, &ver);
FAIL();
}
catch(const reapack_error &e) {
@@ -146,34 +127,16 @@ TEST_CASE("empty source url", M) {
}
}
-TEST_CASE("full name without version", M) {
- SECTION("with file name") {
- const Source source("path/to/file", "b");
- REQUIRE(source.fullName() == "file");
- }
-
- SECTION("without file name") {
- try {
- const Source source({}, "b");
- (void)source.fullName();
- FAIL();
- }
- catch(const reapack_error &e) {
- REQUIRE(string(e.what()) == "empty source file name and no package");
- }
- }
-}
+TEST_CASE("source full name", M) {
+ MAKE_VERSION;
-TEST_CASE("full name with version", M) {
SECTION("with file name") {
- Version ver("1.0");
const Source source("path/to/file", "b", &ver);
- REQUIRE(source.fullName() == "v1.0 (file)");
+ REQUIRE(source.fullName() == ver.fullName() + " (file)");
}
SECTION("without file name") {
- Version ver("1.0");
const Source source({}, "b", &ver);
REQUIRE(source.fullName() == ver.fullName());
@@ -181,10 +144,7 @@ TEST_CASE("full name with version", M) {
}
TEST_CASE("source target path", M) {
- Index ri("Index Name");
- Category cat("Category Name", &ri);
- Package pack(Package::UnknownType, "package name", &cat);
- Version ver("1.0", &pack);
+ MAKE_VERSION;
Source source("file.name", "url", &ver);
@@ -251,17 +211,6 @@ TEST_CASE("target path with parent directory traversal", M) {
}
}
-TEST_CASE("target path without package", M) {
- try {
- const Source source("a", "b");
- (void)source.targetPath();
- FAIL();
- }
- catch(const reapack_error &e) {
- REQUIRE(string(e.what()) == "no package associated with this source");
- }
-}
-
TEST_CASE("target path for unknown package type", M) {
Index ri("name");
Category cat("name", &ri);
@@ -286,17 +235,3 @@ TEST_CASE("directory traversal in category name", M) {
REQUIRE(src.targetPath() == expected);
}
-
-TEST_CASE("target path without category", M) {
- Package pack(Package::ScriptType, "file.name");
- Version ver("1.0", &pack);
- Source src({}, "url", &ver);
-
- try {
- src.targetPath();
- FAIL();
- }
- catch(const reapack_error &e) {
- REQUIRE(string(e.what()) == "category or index is unset");
- }
-}
diff --git a/test/version.cpp b/test/version.cpp
@@ -190,34 +190,12 @@ TEST_CASE("prerelease versions", M) {
}
TEST_CASE("version full name", M) {
- SECTION("no package") {
- Version ver("1.0");
- REQUIRE(ver.fullName() == "v1.0");
- }
-
- SECTION("with package") {
- Package pkg(Package::UnknownType, "file.name");
- Version ver("1.0", &pkg);
-
- REQUIRE(ver.fullName() == "file.name v1.0");
- }
-
- SECTION("with category") {
- Category cat("Category Name");
- Package pkg(Package::UnknownType, "file.name", &cat);
- Version ver("1.0", &pkg);
+ Index ri("Index Name");
+ Category cat("Category Name", &ri);
+ Package pkg(Package::UnknownType, "file.name", &cat);
+ Version ver("1.0", &pkg);
- REQUIRE(ver.fullName() == "Category Name/file.name v1.0");
- }
-
- SECTION("with index") {
- Index ri("Remote Name");
- Category cat("Category Name", &ri);
- Package pkg(Package::UnknownType, "file.name", &cat);
- Version ver("1.0", &pkg);
-
- REQUIRE(ver.fullName() == "Remote Name/Category Name/file.name v1.0");
- }
+ REQUIRE(ver.fullName() == "Index Name/Category Name/file.name v1.0");
}
TEST_CASE("add source", M) {
@@ -302,7 +280,9 @@ TEST_CASE("version date", M) {
}
TEST_CASE("copy version constructor", M) {
- const Package pkg(Package::UnknownType, "Hello");
+ Index ri("Remote Name");
+ Category cat("Category Name", &ri);
+ Package pkg(Package::ScriptType, "Hello", &cat);
Version original("1.1test", &pkg);
original.setAuthor("John Doe");
