diff --git a/NEWS.md b/NEWS.md index ae1f072c5..2e92c8cf7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ ## Bugfixes +* `path_tidy()` now always includes a trailing slash for the windows root + directory, e.g. `C:/` (#124). + * `path_ext()`, `path_ext_set()` and `path_ext_remove()` now handle paths with non-ASCII characters (#120). diff --git a/src/utils.cc b/src/utils.cc index cfd489719..3d649aeb3 100644 --- a/src/utils.cc +++ b/src/utils.cc @@ -45,7 +45,17 @@ get_dirent_type(const char* path, const uv_dirent_type_t& entry_type) { return entry_type; } -std::string path_tidy_(const std::string in) { +bool is_windows_path(const std::string& x) { + if (x.length() < 2) { + return false; + } + char first = x.at(0); + + return ((first >= 'A' && first <= 'Z') || (first >= 'a' && first <= 'z')) && + x.at(1) == ':'; +} + +std::string path_tidy_(const std::string& in) { std::string out; out.reserve(in.size()); char prev = '\0'; @@ -75,6 +85,16 @@ std::string path_tidy_(const std::string in) { out.push_back(prev); } + if (is_windows_path(out)) { + // Append a / if this is a root path + if (out.length() == 2) { + out.push_back('/'); + } else if (out.length() > 3 && *out.rbegin() == '/') { + out.erase(out.end() - 1); + } + return out; + } + // Remove trailing / from paths (that aren't also the beginning) if (out.length() > 1 && *out.rbegin() == '/') { out.erase(out.end() - 1); diff --git a/src/utils.h b/src/utils.h index ae05faf1f..58edd5728 100644 --- a/src/utils.h +++ b/src/utils.h @@ -1,7 +1,7 @@ #include "Rcpp.h" #include "uv.h" -inline Rcpp::CharacterVector asCharacterVector(std::string x) { +inline Rcpp::CharacterVector asCharacterVector(const std::string& x) { return Rcpp::CharacterVector(Rf_mkCharCE(x.c_str(), CE_UTF8)); } @@ -10,4 +10,4 @@ inline Rcpp::CharacterVector asCharacterVector(std::string x) { uv_dirent_type_t get_dirent_type( const char* path, const uv_dirent_type_t& entry_type = UV_DIRENT_UNKNOWN); -std::string path_tidy_(const std::string in); +std::string path_tidy_(const std::string& in); diff --git a/tests/testthat/test-path.R b/tests/testthat/test-path.R index b12176a92..47cf8fe7d 100644 --- a/tests/testthat/test-path.R +++ b/tests/testthat/test-path.R @@ -105,6 +105,17 @@ describe("path_tidy", { expect_equal(path_tidy("foo\\\\bar\\\\baz\\\\"), "foo/bar/baz") }) + it("always appends windows root paths with /", { + expect_equal(path_tidy("C:"), "C:/") + expect_equal(path_tidy("c:"), "c:/") + expect_equal(path_tidy("X:"), "X:/") + expect_equal(path_tidy("x:"), "x:/") + expect_equal(path_tidy("c:/"), "c:/") + expect_equal(path_tidy("c://"), "c:/") + expect_equal(path_tidy("c:\\"), "c:/") + expect_equal(path_tidy("c:\\\\"), "c:/") + }) + it("passes NA along", { expect_equal(path_tidy(NA_character_), NA_character_) expect_equal(path_tidy(c("foo/bar", NA_character_)), c("foo/bar", NA_character_)) @@ -238,9 +249,9 @@ describe("path_norm", { expect_equal(path_norm("."), ".") expect_equal(path_norm(""), ".") expect_equal(path_norm("/"), "/") - expect_equal(path_norm("c:/"), "c:") + expect_equal(path_norm("c:/"), "c:/") expect_equal(path_norm("/../.././.."), "/") - expect_equal(path_norm("c:/../../.."), "c:") + expect_equal(path_norm("c:/../../.."), "c:/") expect_equal(path_norm("../.././.."), "../../..") expect_equal(path_norm("C:////a/b"), "C:/a/b") expect_equal(path_norm("//machine/share//a/b"), "//machine/share/a/b")