reapack

commit b2b635b5dbb15b7dab85c0548516e94a47c2e16f
parent bba28f69642a4438da073202fdb6ddb4acc1bdc8
Author: cfillion <cfillion@users.noreply.github.com>
Date:   Mon, 30 Jan 2023 11:34:51 -0500

improve invalid repository name error message shown when importing

Closes #63

Co-authored-by: Mike Kot <to@myrrc.dev>

Diffstat:
M
src/remote.cpp
 | 
14
++++++++------
M
test/remote.cpp
 | 
25
++++++++++++++++++++-----

2 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/src/remote.cpp b/src/remote.cpp
@@ -103,10 +103,12 @@ Remote::Remote(const std::string &name, const std::string &url,
 
 void Remote::setName(const std::string &name)
 {
-  if(!validateName(name))
-    throw reapack_error("invalid name");
-  else
-    m_name = name;
+  if(name.empty())
+    throw reapack_error("empty repository name");
+  if(!validateName(name)) // also checks for emptyness
+    throw reapack_error(String::format("invalid repository name '%s'", name.c_str()));
+
+  m_name = name;
 }
 
 void Remote::setUrl(const std::string &url)
@@ -115,8 +117,8 @@ void Remote::setUrl(const std::string &url)
     throw reapack_error("invalid url");
   else if(m_protected && url != m_url)
     throw reapack_error("cannot change the URL of a protected repository");
-  else
-    m_url = url;
+
+  m_url = url;
 }
 
 bool Remote::autoInstall(bool fallback) const
diff --git a/test/remote.cpp b/test/remote.cpp
@@ -14,9 +14,18 @@ TEST_CASE("construct remote", M) {
 }
 
 TEST_CASE("remote name validation", M) {
+  SECTION("empty") {
+    try {
+      Remote remote("", "url");
+      FAIL("empty name was allowed");
+    }
+    catch(const reapack_error &e) {
+      REQUIRE(std::string{e.what()} == "empty repository name");
+    }
+  }
+
   SECTION("invalid") {
     const std::string invalidNames[] {
-      "",
       "ab/cd",
       "ab\\cd",
       "..",
@@ -41,7 +50,7 @@ TEST_CASE("remote name validation", M) {
         FAIL("'" + name + "' was allowed");
       }
       catch(const reapack_error &e) {
-        REQUIRE(std::string{e.what()} == "invalid name");
+        REQUIRE(std::string{e.what()} == "invalid repository name '" + name + "'");
       }
     }
   }
@@ -95,7 +104,7 @@ TEST_CASE("set invalid values", M) {
       FAIL();
     }
     catch(const reapack_error &e) {
-      REQUIRE(std::string{e.what()} == "invalid name");
+      REQUIRE(std::string{e.what()} == "invalid repository name '/'");
     }
   }
 
@@ -226,10 +235,16 @@ TEST_CASE("get remote by name", M) {
 }
 
 TEST_CASE("unserialize remote", M) {
-  SECTION("invalid name")
-    REQUIRE_FALSE(Remote::fromString("&"));
+  SECTION("incomplete")
+    REQUIRE_FALSE(Remote::fromString("foo"));
+
+  SECTION("empty name")
+    REQUIRE_FALSE(Remote::fromString("|url|1"));
 
   SECTION("invalid name")
+    REQUIRE_FALSE(Remote::fromString("/|url|1"));
+
+  SECTION("invalid url")
     REQUIRE_FALSE(Remote::fromString("name||false"));
 
   SECTION("enabled") {