Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make sure path is in native encoding #477

Merged
merged 8 commits into from
May 11, 2018
Merged

Make sure path is in native encoding #477

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bcaea8f
Create temporary test sheet with non-ASCII character in filename
jennybc May 4, 2018
945178d
Don't normalizePath() on xls filepaths
jennybc May 4, 2018
8a8c061
Explicit enc2native()
jennybc May 4, 2018
f698235
Work on the test
jennybc May 5, 2018
96cd855
Extend test to xlsx
jennybc May 11, 2018
2afc45e
Don't modify xls path on the C++ side
jennybc May 11, 2018
098c3b0
Ensure all paths are in native encoding at the handoff
jennybc May 11, 2018
a15391b
Edit NEWS
jennybc May 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/XlsWorkBook.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
inline std::string normalizePath(std::string path) {
Rcpp::Environment baseEnv = Rcpp::Environment::base_env();
Rcpp::Function normalizePath = baseEnv["normalizePath"];
return Rcpp::as<std::string>(normalizePath(path, "/", true));
Rcpp::Function enc2native = baseEnv["enc2native"];
return Rcpp::as<std::string>(enc2native(normalizePath(path, "/", true)));
}

class XlsWorkBook {
Expand Down
15 changes: 15 additions & 0 deletions tests/testthat/test-read-excel.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,18 @@ test_that("trim_ws must be a logical", {
"`trim_ws` must be either TRUE or FALSE"
)
})

## https://github.com/tidyverse/readxl/issues/476
test_that("non-ASCII xls filenames can be read", {
skip_on_cran()
## chosen to be non-ASCII but
## [1] representable in Windows-1252 and
## [2] not any of the few differences between Windows-1252 and ISO-8859-1
## a-grave + e-diaeresis + Eth + '.xls'
filename <- "\u00C0\u00CB\u00D0\u002E\u0078\u006C\u0073"
tricky_file <- file.path(tempdir(), filename)
file.copy(test_sheet("mtcars.xls"), tricky_file)
expect_true(file.exists(tricky_file))
on.exit(file.remove(tricky_file))
expect_error_free(read_xls(tricky_file))
})