Add tests for Filepath::set_extension()

4 files changed, 42 insertions, 18 deletions

diff --git a/include/filepath.h b/include/filepath.h
index 66b7d9c7..6b3f1f79 100644
--- a/include/filepath.h
+++ b/include/filepath.h
@@ -93,8 +93,11 @@ public:
 	// Return `true` if path is absolute.
 	bool is_absolute() const;
 
-	// Return `false` and do nothing if Filepath is empty, set extension and return `true` otherwise.
-	bool set_extension(const std::string& str);
+	// Return `false` and do nothing if Filepath is empty, set extension and
+	// return `true` otherwise.
+	//
+	// \note `ext` is interpreted as bytes in locale encoding.
+	bool set_extension(const std::string& ext);
 
 	// Return `true` if Filepath start with `str`, `false` otherwise.
 	bool starts_with(const std::string& str) const;
diff --git a/rust/libnewsboat-ffi/src/filepath.rs b/rust/libnewsboat-ffi/src/filepath.rs
index ef1db66a..a8ead317 100644
--- a/rust/libnewsboat-ffi/src/filepath.rs
+++ b/rust/libnewsboat-ffi/src/filepath.rs
@@ -22,7 +22,7 @@ mod bridged {
         fn push(filepath: &mut PathBuf, component: &PathBuf);
         fn clone(filepath: &PathBuf) -> Box<PathBuf>;
         fn is_absolute(filepath: &PathBuf) -> bool;
-        fn set_extension(filepath: &mut PathBuf, extension: &str) -> bool;
+        fn set_extension(filepath: &mut PathBuf, extension: Vec<u8>) -> bool;
         fn starts_with(filepath: &PathBuf, str: &str) -> bool;
         fn file_name(filepath: &PathBuf) -> Vec<u8>;
 
@@ -93,8 +93,10 @@ fn is_absolute(filepath: &PathBuf) -> bool {
     filepath.0.is_absolute()
 }
 
-fn set_extension(filepath: &mut PathBuf, extension: &str) -> bool {
-    filepath.0.set_extension(extension)
+fn set_extension(filepath: &mut PathBuf, extension: Vec<u8>) -> bool {
+    use std::ffi::OsStr;
+    use std::os::unix::ffi::OsStrExt;
+    filepath.0.set_extension(OsStr::from_bytes(&extension))
 }
 
 fn starts_with(filepath: &PathBuf, str: &str) -> bool {
diff --git a/src/filepath.cpp b/src/filepath.cpp
index 488cdc27..93c07fe6 100644
--- a/src/filepath.cpp
+++ b/src/filepath.cpp
@@ -2,6 +2,20 @@
 
 namespace newsboat {
 
+inline namespace {
+
+rust::Vec<std::uint8_t> string_to_vec(const std::string& input)
+{
+	rust::Vec<std::uint8_t> result;
+	result.reserve(input.length());
+	for (const auto byte : input) {
+		result.push_back(byte);
+	}
+	return result;
+}
+
+}
+
 Filepath::Filepath()
 	: rs_object(filepath::bridged::create_empty())
 {
@@ -9,14 +23,8 @@ Filepath::Filepath()
 
 Filepath Filepath::from_locale_string(const std::string& filepath)
 {
-	rust::Vec<std::uint8_t> data;
-	data.reserve(filepath.length());
-	for (const auto byte : filepath) {
-		data.push_back(byte);
-	}
-
 	Filepath result;
-	result.rs_object = filepath::bridged::create(std::move(data));
+	result.rs_object = filepath::bridged::create(string_to_vec(filepath));
 	return result;
 }
 
@@ -72,9 +80,9 @@ bool Filepath::is_absolute() const
 	return filepath::bridged::is_absolute(*rs_object);
 }
 
-bool Filepath::set_extension(const std::string& str)
+bool Filepath::set_extension(const std::string& ext)
 {
-	return filepath::bridged::set_extension(*rs_object, str);
+	return filepath::bridged::set_extension(*rs_object, string_to_vec(ext));
 }
 
 bool Filepath::starts_with(const std::string& str) const
diff --git a/test/filepath.cpp b/test/filepath.cpp
index 36cbea08..6f5c968c 100644
--- a/test/filepath.cpp
+++ b/test/filepath.cpp
@@ -74,14 +74,25 @@ TEST_CASE("Can be copied", "[Filepath]")
 	}
 }
 
-TEST_CASE("Set extension", "[Filepath]")
+TEST_CASE("Can't set extension for an empty path", "[Filepath]")
 {
 	Filepath path;
 	REQUIRE_FALSE(path.set_extension("exe"));
+}
+
+TEST_CASE("Can set extension for non-empty path", "[Filepath]")
+{
+	Filepath path("file");
 
-	path.push("file");
-	REQUIRE(path.set_extension("exe"));
-	REQUIRE(path == "file.exe");
+	SECTION("extension is UTF-8") {
+		REQUIRE(path.set_extension("exe"));
+		REQUIRE(path == "file.exe");
+	}
+
+	SECTION("extension is not a valid UTF-8 string") {
+		REQUIRE(path.set_extension("\x80"));
+		REQUIRE(path == Filepath::from_locale_string("file.\x80"));
+	}
 }
 
 TEST_CASE("Can check if path is absolute", "[Filepath]")