Merge pull request #140 from carpentries/no-bad-children-fix-139

do not error if child document cannot be processed

Author: zkamvar

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: pegboard
 Title: Explore and Manipulate Markdown Curricula from the Carpentries
-Version: 0.7.0
+Version: 0.7.1
 Authors@R: c(
     person(given = "Zhian N.",
            family = "Kamvar",

NEWS.md

@@ -1,3 +1,9 @@
+# pegboard 0.7.1 (2023-10-03)
+
+* child chunk options that would fail out of context no longer cause a failure.
+  (reported: @trhallam, https://github.com/carpentries/workbench/issues/74, 
+  #139; fixed: @zkamvar, #140)
+
 # pegboard 0.7.0 (2023-10-02)
 
 This release introduces automated processing of {knitr} child files, which

R/find_children.R

@@ -164,7 +164,20 @@ child_file_from_code_blocks <- function(nodes) {
   use_children <- xml2::xml_has_attr(nodes, "child")
   if (any(use_children)) {
     nodes <- nodes[use_children]
-    res <- gsub("[\"']", "", xml2::xml_attr(nodes, "child"))
+    children <- xml2::xml_attr(nodes, "child")
+    res <- vector(length = length(children), mode = "list")
+    for (child in seq_along(children)) {
+      res[[child]] <- eval(tryCatch({
+          eval(parse(text = children[child]))
+        }, 
+        error = function(e) {
+          msg <- "could not process child document {children[child]}"
+          pb_message(glue::glue(msg))
+          return(NULL)
+        }
+      ))
+    }
+    res <- unlist(res[lengths(res) > 0L], use.names = FALSE)
   } else {
     character(0)
   }
@@ -277,6 +290,10 @@ read_children <- function(parent, all_children = list(), ...) {
     known_children <- names(all_children)
     new_child <- !child %in% known_children
     if (new_child) {
+      is_valid <- validate_child(child, parent)
+      if (!is_valid) {
+        next
+      }
       # read in the new child with a parent listed
       this_child <- Episode$new(child, parent = list(parent), ...)
     } else {
@@ -332,3 +349,14 @@ add_parent <- function(child, parent) {
   return(invisible(NULL))
 }
 
+validate_child <- function(child, parent) {
+  if (fs::file_exists(child)) {
+    return(TRUE)
+  }
+  parent$children <- parent$children[parent$children != child]
+  child_name <- sQuote(fs::path_rel(child, parent$lesson), q = 2)
+  parent_name <- fs::path_rel(parent$path, parent$lesson)
+  msg <- "could not process child document {child_name} (in {parent_name})"
+  pb_message(glue::glue(msg))
+  return(FALSE)
+}

tests/testthat/_snaps/find_children.md

@@ -50,3 +50,55 @@
       learners/setup.md:26 [needs HTTPS]: [Terminal.app](http://example.com/terminal)
       episodes/files/child.md:2 [missing file (relative to episodes/)]: [broken link](this-file-does-not-exist.md)
 
+# missing children will not be read [plain]
+
+    Code
+      lnk <- lsn$validate_links()
+    Message <cliMessage>
+      ! There were errors in 3/4 links
+      ( ) Links must use HTTPS <https://https.cio.gov/everything/>
+      ( ) Some linked internal files do not exist <https://carpentries.github.io/sandpaper/articles/include-child-documents.html#workspace-consideration>
+      
+      learners/setup.md:18 [needs HTTPS]: [the PuTTY terminal](http://example.com/putty)
+      learners/setup.md:26 [needs HTTPS]: [Terminal.app](http://example.com/terminal)
+      episodes/files/child.md:2 [missing file (relative to episodes/)]: [broken link](this-file-does-not-exist.md)
+
+# missing children will not be read [ansi]
+
+    Code
+      lnk <- lsn$validate_links()
+    Message <cliMessage>
+      ! There were errors in 3/4 links
+      ( ) Links must use HTTPS <https://https.cio.gov/everything/>
+      ( ) Some linked internal files do not exist <https://carpentries.github.io/sandpaper/articles/include-child-documents.html#workspace-consideration>
+      
+      learners/setup.md:18 [needs HTTPS]: [the PuTTY terminal](http://example.com/putty)
+      learners/setup.md:26 [needs HTTPS]: [Terminal.app](http://example.com/terminal)
+      episodes/files/child.md:2 [missing file (relative to episodes/)]: [broken link](this-file-does-not-exist.md)
+
+# missing children will not be read [unicode]
+
+    Code
+      lnk <- lsn$validate_links()
+    Message <cliMessage>
+      ! There were errors in 3/4 links
+      ◌ Links must use HTTPS <https://https.cio.gov/everything/>
+      ◌ Some linked internal files do not exist <https://carpentries.github.io/sandpaper/articles/include-child-documents.html#workspace-consideration>
+      
+      learners/setup.md:18 [needs HTTPS]: [the PuTTY terminal](http://example.com/putty)
+      learners/setup.md:26 [needs HTTPS]: [Terminal.app](http://example.com/terminal)
+      episodes/files/child.md:2 [missing file (relative to episodes/)]: [broken link](this-file-does-not-exist.md)
+
+# missing children will not be read [fancy]
+
+    Code
+      lnk <- lsn$validate_links()
+    Message <cliMessage>
+      ! There were errors in 3/4 links
+      ◌ Links must use HTTPS <https://https.cio.gov/everything/>
+      ◌ Some linked internal files do not exist <https://carpentries.github.io/sandpaper/articles/include-child-documents.html#workspace-consideration>
+      
+      learners/setup.md:18 [needs HTTPS]: [the PuTTY terminal](http://example.com/putty)
+      learners/setup.md:26 [needs HTTPS]: [Terminal.app](http://example.com/terminal)
+      episodes/files/child.md:2 [missing file (relative to episodes/)]: [broken link](this-file-does-not-exist.md)
+

tests/testthat/test-find_children.R

@@ -110,3 +110,52 @@ cli::test_that_cli("children are validated along with parents", {
   expect_equal(lnk$enforce_https, c(FALSE, FALSE, TRUE, TRUE))
 })
 
+cli::test_that_cli("missing children will not be read", {
+  # setup -------------------------------------------------------------------
+  tmp <- withr::local_tempdir()
+  test_dir <- fs::path(tmp, "sandpaper-fragment")
+  fs::dir_copy(lesson_fragment("sandpaper-fragment"), test_dir)
+  fs::dir_copy(
+    test_path("examples", "child-example", "files"),
+    fs::path(test_dir, "episodes")
+  )
+  # create two files in the lesson with the same children -------------------
+  parent_file <- fs::path(test_dir, "episodes", "intro.Rmd")
+  second_parent_file <- fs::path(test_dir, "episodes", "next.Rmd")
+  fs::file_copy(
+    test_path("examples", "child-example", "parent.Rmd"),
+    parent_file,
+    overwrite = TRUE
+  )
+  fs::file_copy(
+    test_path("examples", "child-example", "parent.Rmd"),
+    second_parent_file,
+    overwrite = TRUE
+  )
+  cat("```{r child=file.path(snippets, \"test.Rmd\")}\n```\n", 
+    file = parent_file,
+    append = TRUE
+  )
+  # set the order in the config --------------------------------------------
+  cfg <- readLines(fs::path(test_dir, "config.yaml"))
+  eplist <- which(endsWith(cfg, "intro.Rmd"))
+  cfg[eplist] <- paste0(cfg[eplist], "\n  - next.Rmd")
+  writeLines(cfg, fs::path(test_dir, "config.yaml"))
+  children_names <- fs::path(
+    test_dir, "episodes", "files",
+    c("child.md", "child-2.Rmd", "child-3.md")
+  )
+
+  expect_message(lsn <- Lesson$new(test_dir, jekyll = FALSE),
+    "could not process child document file.path(snippets, \"test.Rmd\")",
+    fixed = TRUE
+  )
+
+  expect_snapshot(lnk <- lsn$validate_links())
+  expect_s3_class(lnk, "data.frame")
+  expect_paths <- c("learners/setup.md", "learners/setup.md", "episodes/files/child.md", "episodes/files/child-3.md")
+  expect_equal(lnk$filepath, fs::path(expect_paths))
+  expect_equal(lnk$internal_file, c(TRUE, TRUE, FALSE, TRUE))
+  expect_equal(lnk$enforce_https, c(FALSE, FALSE, TRUE, TRUE))
+})
+