Skip to content

Commit

Permalink
Always include trailing slash in windows root directories
Browse files Browse the repository at this point in the history
A windows path without the trailing slash is interpreted as the working
directory, which is unlikely to be the intent.

Fixes #124
  • Loading branch information
jimhester committed Jul 24, 2018
1 parent 1528958 commit 0f2de7a
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 5 deletions.
3 changes: 3 additions & 0 deletions 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).

Expand Down
22 changes: 21 additions & 1 deletion src/utils.cc
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions 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));
}

Expand All @@ -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);
15 changes: 13 additions & 2 deletions tests/testthat/test-path.R
Expand Up @@ -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_))
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit 0f2de7a

Please sign in to comment.