Fix create --output regression

fbdtemme · Feb 19, 2021 · 9c00e2f · 9c00e2f
1 parent e7d5707
commit 9c00e2f
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 12 deletions.
diff --git a/include/argument_parsers.hpp b/include/argument_parsers.hpp
@@ -21,7 +21,9 @@ std::vector<dottorrent::dht_node> dht_node_transformer(const std::vector<std::st
 
 std::unordered_set<dottorrent::hash_function> checksum_transformer(const std::vector<std::string>& s);
 
-std::filesystem::path target_transformer(const std::vector<std::string>& v, bool check_exists = true);
+std::filesystem::path target_transformer(const std::vector<std::string>& v,
+                                         bool check_exists = true,
+                                         bool keep_trailing = true);
 
 std::filesystem::path metafile_transformer(const std::vector<std::string>& v);
 

diff --git a/src/argument_parsers.cpp b/src/argument_parsers.cpp
@@ -167,7 +167,7 @@ dottorrent::protocol protocol_transformer(const std::vector<std::string>& v, boo
     throw std::invalid_argument(fmt::format("Invalid bittorrent protocol: {}", s));
 }
 
-std::filesystem::path target_transformer(const std::vector<std::string>& v, bool check_exists)
+std::filesystem::path target_transformer(const std::vector<std::string>& v, bool check_exists, bool keep_trailing)
 {
     if (v.size() != 1) {
         throw std::invalid_argument("Multiple targets given.");
@@ -178,12 +178,21 @@ std::filesystem::path target_transformer(const std::vector<std::string>& v, bool
 
     auto f = std::filesystem::path(v.front());
     if (check_exists && !std::filesystem::exists(f)) {
-        throw std::invalid_argument("Path does not exist.");
+        throw std::invalid_argument(fmt::format("Path does not exist: {}", f.string()));
     }
+
     if (check_exists) {
         return std::filesystem::canonical(f);
-    } else {
-        return std::filesystem::weakly_canonical(f);
+    }
+    else {
+        bool has_trailing_dir_seperator = f.filename().empty();
+        bool is_directory = std::filesystem::is_directory(f);
+        auto canonical_f = std::filesystem::weakly_canonical(f);
+
+        if (keep_trailing && (has_trailing_dir_seperator || is_directory)) {
+            return canonical_f / "";
+        }
+        return canonical_f;
     }
 }
 

diff --git a/src/create.cpp b/src/create.cpp
@@ -349,9 +349,9 @@ fs::path get_destination_path(dottorrent::metafile& m, std::optional<fs::path> d
             destination = *destination_path;
             return destination;
         }
-            // option is only a destination directory and not a filename
+        // option is only a destination directory and not a filename
         else {
-            destination_directory = *destination_path;
+            destination_directory = destination_path->parent_path();
         }
     }
     else {

diff --git a/tests/test_create.cpp b/tests/test_create.cpp
@@ -163,6 +163,7 @@ TEST_CASE("test create app argument parsing")
             CHECK(*create_options.is_private);
         }
     }
+
     SECTION("piece size") {
         SECTION("as power of two") {
             auto cmd = fmt::format("create {} --piece-size {}", file, 20);
@@ -439,16 +440,28 @@ TEST_CASE("test create app argument parsing")
             CHECK_FALSE(create_options.io_block_size.has_value());
         }
     }
-    SECTION("stdout") {
+    SECTION("output") {
         SECTION("default") {
             auto cmd = fmt::format("create {}", file);
             PARSE_ARGS(cmd);
-            CHECK_FALSE(create_options.write_to_stdout);
+            CHECK_FALSE(create_options.destination);
         }
-        SECTION("option given") {
-            auto cmd = fmt::format("create {} --include-hidden", file);
+        SECTION("full path") {
+            auto cmd = fmt::format("create {} --output {}", file, "/home/test/output.torrent");
             PARSE_ARGS(cmd);
-            CHECK(create_options.include_hidden_files);
+            CHECK(create_options.destination == "/home/test/output.torrent");
+        }
+        SECTION("Directory with trailing slash") {
+            auto cmd = fmt::format("create {} --output {}", file,  "/home/test/Downloads/");
+            PARSE_ARGS(cmd);
+            CHECK(create_options.destination->string() == "/home/test/Downloads/");
+        }
+        SECTION("Existing directory -> append trailing slash") {
+            temporary_directory tmp_dir {};
+
+            auto cmd = fmt::format("create {} --output {}", file,  tmp_dir.path());
+            PARSE_ARGS(cmd);
+            CHECK(create_options.destination->string() == tmp_dir.path() / "");
         }
     }
 }