Push re-encoding of xls path into XlsWorkBook constructor (#674)

tidyverse · Jan 3, 2022 · 4540dff · 4540dff
1 parent d8f789e
commit 4540dff
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 12 deletions.
diff --git a/R/excel-sheets.R b/R/excel-sheets.R
@@ -18,7 +18,7 @@
 excel_sheets <- function(path) {
   path <- check_file(path)
   format <- check_format(path)
-  path <- normalize_path(path)
+  path <- normalizePath(path)
 
   switch(format,
     xls = xls_sheets(path),

diff --git a/R/read_excel.R b/R/read_excel.R
@@ -194,7 +194,7 @@ read_excel_ <- function(path, sheet = NULL, range = NULL,
     sheets_fun <- xlsx_sheets
     read_fun <- read_xlsx_
   }
-  path <- normalize_path(path)
+  path <- normalizePath(path)
   sheet <- standardise_sheet(sheet, range, sheets_fun(path))
   shim <- !is.null(range)
   limits <- standardise_limits(

diff --git a/R/utils.R b/R/utils.R
@@ -9,10 +9,6 @@ check_file <- function(path) {
   path
 }
 
-normalize_path <- function(path) {
-  enc2native(normalizePath(path))
-}
-
 is_integerish <- function(x) {
   floor(x) == x
 }

diff --git a/src/XlsWorkBook.h b/src/XlsWorkBook.h
@@ -4,6 +4,7 @@
 #include <sstream>
 #include "ColSpec.h"
 #include "libxls/xls.h"
+#include "cpp11/r_string.hpp"
 
 class XlsWorkBook {
 
@@ -19,7 +20,12 @@ class XlsWorkBook {
 public:
 
   XlsWorkBook(const std::string& path) {
-    path_ = path;
+    // the user's path has probably been translated to UTF-8 by
+    // normalizePath() on the R side
+    // even if that were not true, cpp11 does this automatically when
+    // constructing a std::string
+    // but we need to pass the path to libxls in the native encoding
+    path_ = std::string(Rf_translateChar(cpp11::r_string(path)));
 
     xls::xls_error_t error = xls::LIBXLS_OK;
     xls::xlsWorkBook* pWB_ = xls::xls_open_file(path_.c_str(), "UTF-8", &error);

diff --git a/src/XlsWorkSheet.cpp b/src/XlsWorkSheet.cpp
@@ -5,15 +5,14 @@
 #include "libxls/xls.h"
 
 [[cpp11::register]]
-cpp11::list read_xls_(cpp11::strings path, int sheet_i,
+cpp11::list read_xls_(std::string path, int sheet_i,
                       cpp11::integers limits, bool shim,
                       cpp11::sexp col_names, cpp11::strings col_types,
                       std::vector<std::string> na, bool trim_ws,
                       int guess_max = 1000, bool progress = true) {
 
-  std::string filename(Rf_translateChar(STRING_ELT(path, 0)));
   // Construct worksheet ----------------------------------------------
-  XlsWorkSheet ws(filename, sheet_i, limits, shim, progress);
+  XlsWorkSheet ws(path, sheet_i, limits, shim, progress);
 
   // catches empty sheets and sheets where requested rectangle contains no data
   if (ws.nrow() == 0 && ws.ncol() == 0) {

diff --git a/src/cpp11.cpp b/src/cpp11.cpp
@@ -20,10 +20,10 @@ extern "C" SEXP _readxl_xls_date_formats(SEXP path) {
   END_CPP11
 }
 // XlsWorkSheet.cpp
-cpp11::list read_xls_(cpp11::strings path, int sheet_i, cpp11::integers limits, bool shim, cpp11::sexp col_names, cpp11::strings col_types, std::vector<std::string> na, bool trim_ws, int guess_max, bool progress);
+cpp11::list read_xls_(std::string path, int sheet_i, cpp11::integers limits, bool shim, cpp11::sexp col_names, cpp11::strings col_types, std::vector<std::string> na, bool trim_ws, int guess_max, bool progress);
 extern "C" SEXP _readxl_read_xls_(SEXP path, SEXP sheet_i, SEXP limits, SEXP shim, SEXP col_names, SEXP col_types, SEXP na, SEXP trim_ws, SEXP guess_max, SEXP progress) {
   BEGIN_CPP11
-    return cpp11::as_sexp(read_xls_(cpp11::as_cpp<cpp11::decay_t<cpp11::strings>>(path), cpp11::as_cpp<cpp11::decay_t<int>>(sheet_i), cpp11::as_cpp<cpp11::decay_t<cpp11::integers>>(limits), cpp11::as_cpp<cpp11::decay_t<bool>>(shim), cpp11::as_cpp<cpp11::decay_t<cpp11::sexp>>(col_names), cpp11::as_cpp<cpp11::decay_t<cpp11::strings>>(col_types), cpp11::as_cpp<cpp11::decay_t<std::vector<std::string>>>(na), cpp11::as_cpp<cpp11::decay_t<bool>>(trim_ws), cpp11::as_cpp<cpp11::decay_t<int>>(guess_max), cpp11::as_cpp<cpp11::decay_t<bool>>(progress)));
+    return cpp11::as_sexp(read_xls_(cpp11::as_cpp<cpp11::decay_t<std::string>>(path), cpp11::as_cpp<cpp11::decay_t<int>>(sheet_i), cpp11::as_cpp<cpp11::decay_t<cpp11::integers>>(limits), cpp11::as_cpp<cpp11::decay_t<bool>>(shim), cpp11::as_cpp<cpp11::decay_t<cpp11::sexp>>(col_names), cpp11::as_cpp<cpp11::decay_t<cpp11::strings>>(col_types), cpp11::as_cpp<cpp11::decay_t<std::vector<std::string>>>(na), cpp11::as_cpp<cpp11::decay_t<bool>>(trim_ws), cpp11::as_cpp<cpp11::decay_t<int>>(guess_max), cpp11::as_cpp<cpp11::decay_t<bool>>(progress)));
   END_CPP11
 }
 // XlsxWorkBook.cpp