diff --git a/DESCRIPTION b/DESCRIPTION index 2e886b5..3d21c33 100644 --- a/DESCRIPTION +++ b/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", diff --git a/NEWS.md b/NEWS.md index bf185e8..1cf0758 100644 --- a/NEWS.md +++ b/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 diff --git a/R/find_children.R b/R/find_children.R index 503b5bd..a2866a3 100644 --- a/R/find_children.R +++ b/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) +} diff --git a/tests/testthat/_snaps/find_children.md b/tests/testthat/_snaps/find_children.md index e479992..4115d0a 100644 --- a/tests/testthat/_snaps/find_children.md +++ b/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 + ! There were errors in 3/4 links + ( ) Links must use HTTPS + ( ) Some linked internal files do not exist + + 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 + ! There were errors in 3/4 links + ( ) Links must use HTTPS + ( ) Some linked internal files do not exist + + 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 + ! There were errors in 3/4 links + ◌ Links must use HTTPS + ◌ Some linked internal files do not exist + + 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 + ! There were errors in 3/4 links + ◌ Links must use HTTPS + ◌ Some linked internal files do not exist + + 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) + diff --git a/tests/testthat/test-find_children.R b/tests/testthat/test-find_children.R index 05a0874..d6b3159 100644 --- a/tests/testthat/test-find_children.R +++ b/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)) +}) +