Fix #37

The string trimming function didn't check for missing. Now it's
checked.

Also, to emulate the behavior of `readxl`, all space columns should be
detected as logical.

benchmark pending

src/CollectorGuess.cpp

@@ -33,11 +33,17 @@ bool canParse(const cpp11::strings& x, const canParseFun& canParseF, LocaleInfo*
   return true;
 }
 
-bool allMissing(const cpp11::strings& x) {
+bool allMissing(const cpp11::strings& x, bool trim_ws) {
   for (const auto & i : x) {
-    if (i != NA_STRING && i.size() > 0) {
+    if (!trim_ws && i != NA_STRING && i.size() > 0) {
       return false;
     }
+    if (trim_ws) {
+      auto istr = trimString(std::string(i));
+      if (i != NA_STRING && istr != "") {
+        return false;
+      }
+    }
   }
   return true;
 }
@@ -136,7 +142,7 @@ static bool isDateTime(const std::string& x, LocaleInfo* pLocale) {
     return "character";
   }
 
-  if (allMissing(input)) {
+  if (allMissing(input, trim_ws)) {
     return "logical";
   }
 

src/utils.h

@@ -84,8 +84,11 @@ inline bool starts_with_comment(
 inline std::string trimString(std::string const &str, std::string const &whitespace=" \r\n\t\v\f") {
   auto start = str.find_first_not_of(whitespace);
   auto end = str.find_last_not_of(whitespace);
-
-  return str.substr(start, end - start + 1);
+  if (start != std::string::npos) {
+    return str.substr(start, end - start + 1);
+  } else {
+    return "";
+  }
 }
 
 #endif

tests/testthat/test-parsing.R

@@ -24,3 +24,13 @@ test_that("parse_guess() trim_ws #32 or tidyverse/readr#1536", {
     expect_equal(parse_guess(c(" 1", "   2", "     3"), trim_ws = FALSE), c(" 1", "   2", "     3"))
     expect_equal(parse_guess(c(" TRUE", "FALSE ", " T "), trim_ws = TRUE), c(TRUE, FALSE, TRUE))
 })
+
+test_that("all empty with trim_ws won't crash, #37 and ropensci/readODS#211", {
+    expect_error(minty:::guess_parser(c(" ", "     "), trim_ws = TRUE), NA)
+    expect_error(minty:::guess_parser(c(" "), trim_ws = TRUE), NA)
+    expect_equal(minty:::guess_parser(c(" "), trim_ws = TRUE), "logical")
+    expect_equal(minty:::guess_parser(c(" ", "   "), trim_ws = TRUE), "logical")
+    expect_error(minty:::guess_parser(c(" "), trim_ws = FALSE), NA)
+    expect_error(minty:::guess_parser(c(" ", "a"), trim_ws = FALSE), NA)
+    expect_equal(minty:::guess_parser(c(" ", "a")), "character")
+})

tests/testthat/test-type-convert.R

@@ -98,3 +98,15 @@ test_that("r_is_string_cpp11", {
     expect_false(r_is_string_cpp11(NA_character_))
     expect_false(r_is_string_cpp11(NULL))
 })
+
+test_that("integration tests for #37, also ropensci/readODS#211", {
+    input <- structure(list(picture_archive_url = c("", "", "", "", ""),
+                            video_url = c(" ", " ", " ", " ", " ")),
+                       row.names = c(NA, -5L), class = "data.frame")
+
+    expect_error(type_convert(input, trim_ws = TRUE), NA)
+    output <- type_convert(input, trim_ws = TRUE)
+    expect_true(is.logical(output$video_url))
+    output <- type_convert(input, trim_ws = FALSE)
+    expect_false(is.logical(output$video_url))
+})