From c2061ef9e3876c55a9a7bf577ba517f2330d8856 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 8 Aug 2023 15:38:16 -0700 Subject: [PATCH 01/39] change criteria for carpentries lesson --- R/utils-paths.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/R/utils-paths.R b/R/utils-paths.R index c2cd227be..062a35602 100644 --- a/R/utils-paths.R +++ b/R/utils-paths.R @@ -1,5 +1,5 @@ root_path <- function(path) { - rprojroot::find_root(rprojroot::has_dir("episodes"), path) + rprojroot::find_root(rprojroot::has_file("config.yaml", contents = "carpentry:"), path) } no_readme <- function() "(? Date: Tue, 8 Aug 2023 16:45:09 -0700 Subject: [PATCH 02/39] update root criteria This will address https://github.com/carpentries/workbench/issues/65 by adding fallbacks to the episodes directory criteria. I've added the other directories we expect so that when the episodes directory does not exist (in the case of overview pages), then we can check that the site directory or any of the other directories exists. I had initially attempted this by setting the config.yaml as the criteria, but I forgot that I had also copied the config.yaml over to the site/built folder, so there were a lot of failures from this one change because it was getting confused where the root of the project. Another tactic I considered was to check if `.git/` was a directory, but then I realized this wouldn't work for submodules. --- R/utils-paths.R | 8 +++++++- tests/testthat/helper-hash.R | 2 +- tests/testthat/test-utils-paths.R | 26 ++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 tests/testthat/test-utils-paths.R diff --git a/R/utils-paths.R b/R/utils-paths.R index 062a35602..40982e703 100644 --- a/R/utils-paths.R +++ b/R/utils-paths.R @@ -1,5 +1,11 @@ root_path <- function(path) { - rprojroot::find_root(rprojroot::has_file("config.yaml", contents = "carpentry:"), path) + criteria <- rprojroot::has_dir("episodes") | + rprojroot::has_dir("site") | + rprojroot::has_dir("learners") | + rprojroot::has_dir("instructors") | + rprojroot::has_dir("profiles") + + rprojroot::find_root(criteria, path) } no_readme <- function() "(? Date: Tue, 8 Aug 2023 16:53:10 -0700 Subject: [PATCH 03/39] add shortcut to syllabus creation --- R/get_syllabus.R | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/R/get_syllabus.R b/R/get_syllabus.R index b8eeb5755..7bd4c9e78 100644 --- a/R/get_syllabus.R +++ b/R/get_syllabus.R @@ -32,6 +32,16 @@ get_syllabus <- function(path = ".", questions = FALSE, use_built = TRUE) { } create_syllabus <- function(episodes, lesson, path, questions = TRUE) { + if (is.null(episodes)) { + out <- data.frame( + episode = character(0), + timings = character(0), + path = character(0), + percents = character(0), + stringsAsFactors = FALSE + ) + return(out) + } sched <- fs::path_file(episodes) # We have to invalidate the cache if the syllabus is mis-matched cache_invalid <- !setequal(sched, names(lesson$episodes)) From 780836b5689a11a57cd3e6c5bb908817a81fe900 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 8 Aug 2023 16:53:50 -0700 Subject: [PATCH 04/39] add shortcut to format_syllabus --- R/build_home.R | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/R/build_home.R b/R/build_home.R index f19fa8433..91c5d0235 100644 --- a/R/build_home.R +++ b/R/build_home.R @@ -1,7 +1,8 @@ build_home <- function(pkg, quiet, next_page = NULL) { page_globals <- setup_page_globals() path <- root_path(pkg$src_path) - syl <- format_syllabus(get_syllabus(path, questions = TRUE), use_col = FALSE) + syl <- format_syllabus(get_syllabus(path, questions = TRUE), + use_col = FALSE) idx <- fs::path(pkg$src_path, "built", "index.md") readme <- fs::path(pkg$src_path, "built", "README.md") idx_file <- if (fs::file_exists(idx)) idx else readme @@ -44,17 +45,20 @@ build_home <- function(pkg, quiet, next_page = NULL) { nav$pagetitle <- NULL page_globals$metadata$update(nav) - build_html(template = "syllabus", pkg = pkg, nodes = list(html, setup), + build_html(template = "syllabus", pkg = pkg, nodes = list(html, setup), global_data = page_globals, path_md = "index.html", quiet = quiet) } format_syllabus <- function(syl, use_col = TRUE) { + if (nrow(syl) == 0L) { + return("

") + } syl$questions <- gsub("\n", "
", syl$questions) syl$number <- sprintf("%2d\\. ", seq(nrow(syl))) links <- glue::glue_data( - syl[-nrow(syl), c("number", "episode", "path")], + syl[-nrow(syl), c("number", "episode", "path")], "{gsub('^[ ]', ' ', number)}{episode}" ) if (use_col) { @@ -73,5 +77,5 @@ format_syllabus <- function(syl, use_col = TRUE) { tmp <- tempfile(fileext = ".md") on.exit(unlink(tmp), add = TRUE) writeLines(out, tmp) - render_html(tmp) + return(render_html(tmp)) } From b83278f8f38de7ad42909bda62e3ef1f5fa3d19f Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 8 Aug 2023 16:54:27 -0700 Subject: [PATCH 05/39] add shortcut to build_instructor_notes --- R/build_instructor_notes.R | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/R/build_instructor_notes.R b/R/build_instructor_notes.R index d19721eef..39b9a5408 100644 --- a/R/build_instructor_notes.R +++ b/R/build_instructor_notes.R @@ -3,7 +3,7 @@ #' (for future use) build_instructor_notes <- function(pkg, pages = NULL, built = NULL, quiet) { path <- root_path(pkg$src_path) - this_lesson(path) + lsn <- this_lesson(path) outpath <- fs::path(pkg$dst_path, "instructor-notes.html") already_built <- template_check$valid() && fs::file_exists(outpath) && @@ -35,6 +35,11 @@ build_instructor_notes <- function(pkg, pages = NULL, built = NULL, quiet) { build_html(template = "extra", pkg = pkg, nodes = html, global_data = page_globals, path_md = "instructor-notes.html", quiet = TRUE) } + # shortcut if we don't have any episodes + is_overview <- lsn$overview && length(lsn$episodes) == 0 + if (is_overview) { + return(invisible(NULL)) + } agg <- "/div[contains(@class, 'instructor-note')]//*[@class='accordion-body' or @class='accordion-header']" build_agg_page(pkg = pkg, pages = pages, From 6706d5e03fb69ab2a6b12bc4df57ec7044b6a2cb Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 8 Aug 2023 16:54:59 -0700 Subject: [PATCH 06/39] only process episode-dependent pages if we are not in an overview lesson --- R/build_site.R | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/R/build_site.R b/R/build_site.R index d67a2387a..1b284c89c 100644 --- a/R/build_site.R +++ b/R/build_site.R @@ -17,7 +17,8 @@ build_site <- function(path = ".", quiet = !interactive(), preview = TRUE, overr # that pandoc exists and to provision the global lesson components if they do # not yet exist. check_pandoc(quiet) - this_lesson(path) + lsn <- this_lesson(path) + not_overview <- !(lsn$overview && length(lsn$episodes) == 0L) # One feature of The Workbench is a global common links file that will be # appended to the markdown files before they are sent to be rendered into # HTML so that they will render the links correctly. @@ -54,12 +55,21 @@ build_site <- function(path = ".", quiet = !interactive(), preview = TRUE, overr db <- get_built_db(fs::path(built_path, "md5sum.txt")) # filter out files that we will combine to generate db <- reserved_db(db) - # Find all the episodes and get their range - er <- range(grep("episodes/", db$file, fixed = TRUE)) + if (not_overview) { + # Find all the episodes and get their range + er <- range(grep("episodes/", db$file, fixed = TRUE)) + } else { + # otherwise, just give us the index for all files + er <- seq_along(db$file) + } # Get absolute paths for pandoc to understand abs_md <- fs::path(path, db$built) abs_src <- fs::path(path, db$file) - chapters <- abs_md[seq(er[1], er[2])] + if (not_overview) { + chapters <- abs_md[seq(er[1], er[2])] + } else { + chapters <- character(0) + } # If we are only processing one file, then the output should be that one file if (is.null(slug)) { out <- "index.html" @@ -71,8 +81,12 @@ build_site <- function(path = ".", quiet = !interactive(), preview = TRUE, overr # Rebuilding Episodes and generated files ------------------------------------ # Get percentages from the syllabus table - pct <- get_syllabus(path, questions = TRUE)$percents - names(pct) <- db$file[er[1]:er[2]] + if (not_overview) { + pct <- get_syllabus(path, questions = TRUE)$percents + names(pct) <- db$file[er[1]:er[2]] + } else { + pct <- character(0) + } # ------------------------ shim for downlit ---------------------------- # Bypass certain downlit functions that produce unintented effects such # as linking function documentation. @@ -140,12 +154,16 @@ build_site <- function(path = ".", quiet = !interactive(), preview = TRUE, overr # Once we have the pre-processed templates and HTML content, we can pass these # to our aggregator functions: - describe_progress("Creating keypoints summary", quiet = quiet) - build_keypoints(pkg, pages = html_pages, quiet = quiet) - describe_progress("Creating All-in-one page", quiet = quiet) - build_aio(pkg, pages = html_pages, quiet = quiet) - describe_progress("Creating Images page", quiet = quiet) - build_images(pkg, pages = html_pages, quiet = quiet) + if (not_overview) { + describe_progress("Creating keypoints summary", quiet = quiet) + build_keypoints(pkg, pages = html_pages, quiet = quiet) + + describe_progress("Creating All-in-one page", quiet = quiet) + build_aio(pkg, pages = html_pages, quiet = quiet) + + describe_progress("Creating Images page", quiet = quiet) + build_images(pkg, pages = html_pages, quiet = quiet) + } describe_progress("Creating Instructor Notes", quiet = quiet) build_instructor_notes(pkg, pages = html_pages, built = built, quiet = quiet) From 9c51a0b7073e9a52d840aeaa7661693b88d31511 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 8 Aug 2023 16:58:19 -0700 Subject: [PATCH 07/39] bump version --- DESCRIPTION | 2 +- NEWS.md | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index c716fece3..c161f71ae 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: sandpaper Title: Create and Curate Carpentries Lessons -Version: 0.12.5 +Version: 0.13.0 Authors@R: c( person(given = "Zhian N.", family = "Kamvar", diff --git a/NEWS.md b/NEWS.md index 9a095814b..4787458b1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,17 @@ +# sandpaper 0.13.0 (unreleased) + +## NEW FEATURES + +* Overview style lessons that do not have episodic content can now be processed, + analysed, and built by {sandpaper}. To make your lesson an overview lesson, + you can add `overview: true` to your `config.yaml` + +## BUG FIX + +- Internal function `root_path()` will no longer fail if the `episodes/` folder + does not exist as long as one of the other four folders (`site/`, `learners/`, + `instructors/`, `profiles/`) exists. + # sandpaper 0.12.5 (unreleased) ## BUG FIX From 676aabd2d0996bef5afa253e6d926236eb142c31 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 08:29:14 -0700 Subject: [PATCH 08/39] provision PR version of pegboard for this to work --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index c161f71ae..6f4791e38 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -68,7 +68,7 @@ Suggests: Additional_repositories: https://carpentries.r-universe.dev/ Remotes: ropensci/tinkr, - carpentries/pegboard, + carpentries/pegboard@allow-overview-pages, carpentries/varnish SystemRequirements: pandoc (>= 2.11.4) - https://pandoc.org Encoding: UTF-8 From e7fa55e37b81d1cc46e93f7de691084748c86312 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 08:55:31 -0700 Subject: [PATCH 09/39] first test of overview lessons --- tests/testthat/test-overview.R | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/testthat/test-overview.R diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R new file mode 100644 index 000000000..77bbd7d4c --- /dev/null +++ b/tests/testthat/test-overview.R @@ -0,0 +1,26 @@ +lsn <- restore_fixture() + + +test_that("Lessons without episodes can be built", { + # remove first episode + sandpaper::reset_episodes(lsn) + # add overview to config + cat("\noverview: true\n", file = fs::path(lsn, "config.yaml"), append = TRUE) + # delete episodes folder + fs::dir_delete(fs::path(lsn, "episodes")) + fs::dir_delete(fs::path(lsn, "renv")) + + expect_false(fs::dir_exists(fs::path(lsn, "episodes"))) + expect_false(fs::dir_exists(fs::path(lsn, "site", "docs"))) + expect_true(get_config(lsn)$overview) + + + withr::local_options(list("sandpaper.use_renv" = FALSE)) + sandpaper::build_lesson(lsn, quiet = TRUE, preview = FALSE) + + expect_true(fs::dir_exists(fs::path(lsn, "site", "built"))) + expect_true(fs::dir_exists(fs::path(lsn, "site", "docs"))) + +}) + + From 3a3695e2117f29391e448a3a2b7ea717965f6568 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 09:01:10 -0700 Subject: [PATCH 10/39] fix mac issue --- tests/testthat/test-utils-paths.R | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/testthat/test-utils-paths.R b/tests/testthat/test-utils-paths.R index 8619179ad..d8eaba5c0 100644 --- a/tests/testthat/test-utils-paths.R +++ b/tests/testthat/test-utils-paths.R @@ -3,24 +3,26 @@ res <- restore_fixture() test_that("root path will find the root of the lesson locally", { - here <- as.character(res) - expect_identical(root_path(here), here) + here <- fs::path_file(as.character(res)) + + expect_identical(fs::path_file(root_path(res)), here) + there <- fs::path(res, "episodes") - expect_identical(root_path(there), here) + expect_identical(fs::path_file(root_path(there)), here) there <- fs::path(res, "learners", "setup.md") - expect_identical(root_path(there), here) + expect_identical(fs::path_file(root_path(there)), here) # if we insert a config.yaml file into the built folder, it will not cause a # problem there <- fs::path(res, "site", "built") fs::file_copy(fs::path(res, "config.yaml"), there) - expect_identical(root_path(there), here) + expect_identical(fs::path_file(root_path(there)), here) # removing the episodes folder does not invalidate the lesson - fs::dir_delete(fs::path(here, "episodes")) - expect_identical(root_path(there), here) + fs::dir_delete(fs::path(res, "episodes")) + expect_identical(fs::path_file(root_path(there)), here) }) From fde80e41866051f36eec59cd9a1b91915df2a6f0 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 10:12:53 -0700 Subject: [PATCH 11/39] add fix for windows --- tests/testthat/test-overview.R | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index 77bbd7d4c..7721f7bec 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -24,3 +24,23 @@ test_that("Lessons without episodes can be built", { }) + +test_that("top level fig, files, and data directories are copied over", { + + fs::dir_create(fs::path(lsn, c("fig", "files", "data"))) + fs::file_touch(fs::path(lsn, c("fig", "files", "data"), "hello.png")) + + withr::local_options(list("sandpaper.use_renv" = FALSE)) + sandpaper::build_lesson(lsn, quiet = TRUE, preview = FALSE) + + expect_true(fs::dir_exists(fs::path(lsn, "site", "docs", "fig"))) + expect_true(fs::dir_exists(fs::path(lsn, "site", "docs", "files"))) + expect_true(fs::dir_exists(fs::path(lsn, "site", "docs", "data"))) + + expect_true(fs::file_exists(fs::path(lsn, "site", "docs", "fig", "hello.png"))) + expect_true(fs::file_exists(fs::path(lsn, "site", "docs", "files", "hello.png"))) + expect_true(fs::file_exists(fs::path(lsn, "site", "docs", "data", "hello.png"))) +}) + + + From 90fb3122edabbf7264a251da7c8826823f0d2b80 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 10:44:02 -0700 Subject: [PATCH 12/39] ugh THIS was the windows fix but now everything fails because I deemed it to oh well --- tests/testthat/test-overview.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index 7721f7bec..a4e2b0919 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -1,6 +1,5 @@ lsn <- restore_fixture() - test_that("Lessons without episodes can be built", { # remove first episode sandpaper::reset_episodes(lsn) @@ -8,7 +7,9 @@ test_that("Lessons without episodes can be built", { cat("\noverview: true\n", file = fs::path(lsn, "config.yaml"), append = TRUE) # delete episodes folder fs::dir_delete(fs::path(lsn, "episodes")) - fs::dir_delete(fs::path(lsn, "renv")) + if (renv_is_allowed()) { + fs::dir_delete(fs::path(lsn, "renv")) + } expect_false(fs::dir_exists(fs::path(lsn, "episodes"))) expect_false(fs::dir_exists(fs::path(lsn, "site", "docs"))) From 11e0ed941e1d5d2a76c3b4e6b447120c59b163c0 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 11:24:20 -0700 Subject: [PATCH 13/39] do not remove renv folder in overview test This is causing {renv} to be angry --- tests/testthat/test-overview.R | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index a4e2b0919..915ab986d 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -7,9 +7,6 @@ test_that("Lessons without episodes can be built", { cat("\noverview: true\n", file = fs::path(lsn, "config.yaml"), append = TRUE) # delete episodes folder fs::dir_delete(fs::path(lsn, "episodes")) - if (renv_is_allowed()) { - fs::dir_delete(fs::path(lsn, "renv")) - } expect_false(fs::dir_exists(fs::path(lsn, "episodes"))) expect_false(fs::dir_exists(fs::path(lsn, "site", "docs"))) From f831cea1980f63c498106af93d7d95ac039e0482 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 11:24:56 -0700 Subject: [PATCH 14/39] modify page_location to account for zero range I've also removed the percent calculation from this function as we no longer use it anymore --- R/utils-sidebar.R | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/R/utils-sidebar.R b/R/utils-sidebar.R index 77daa59b5..c0749bfd0 100644 --- a/R/utils-sidebar.R +++ b/R/utils-sidebar.R @@ -3,14 +3,17 @@ # return the surrounding pages for the navbar links page_location <- function(i, abs_md, er) { - idx <- fs::path(fs::path_dir(abs_md[er[1]]), "index.md") + if (sum(er) == 0L) { + idx <- fs::path(fs::path_dir(abs_md[[1]]), "index.md") + } else { + idx <- fs::path(fs::path_dir(abs_md[er[1]]), "index.md") + } if (!i %w% er) { - return(c(back = idx, forward = idx, progress = "")) + return(c(back = idx, forward = idx)) } back <- if (i > er[1]) abs_md[i - 1] else idx fwd <- if (i < er[2]) abs_md[i + 1] else idx - pct <- sprintf("%1.0f", (i - er[1])/(er[2] - er[1]) * 100) - c(back = back, forward = fwd, progress = pct, index = i - er[1]) + c(back = back, forward = fwd, index = i - er[1]) } From 2c9d6b5276e3b1e67a711a0be6022d8221dcb47c Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 11:26:12 -0700 Subject: [PATCH 15/39] refactor build_site - consolodated `not_overview` control structures - removed unused `chapters` variable - added catch for `build_home()` and `progress` --- R/build_site.R | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/R/build_site.R b/R/build_site.R index 1b284c89c..b0e90de5e 100644 --- a/R/build_site.R +++ b/R/build_site.R @@ -55,20 +55,19 @@ build_site <- function(path = ".", quiet = !interactive(), preview = TRUE, overr db <- get_built_db(fs::path(built_path, "md5sum.txt")) # filter out files that we will combine to generate db <- reserved_db(db) - if (not_overview) { - # Find all the episodes and get their range - er <- range(grep("episodes/", db$file, fixed = TRUE)) - } else { - # otherwise, just give us the index for all files - er <- seq_along(db$file) - } # Get absolute paths for pandoc to understand abs_md <- fs::path(path, db$built) abs_src <- fs::path(path, db$file) if (not_overview) { - chapters <- abs_md[seq(er[1], er[2])] + # Find all the episodes and get their range + er <- range(grep("episodes/", db$file, fixed = TRUE)) + # get percentages from the syllabus table + pct <- get_syllabus(path, questions = TRUE)$percents + names(pct) <- db$file[er[1]:er[2]] } else { - chapters <- character(0) + # otherwise, the expected range for episodes is zero + er <- c(0, 0) + pct <- NULL } # If we are only processing one file, then the output should be that one file if (is.null(slug)) { @@ -80,13 +79,6 @@ build_site <- function(path = ".", quiet = !interactive(), preview = TRUE, overr } # Rebuilding Episodes and generated files ------------------------------------ - # Get percentages from the syllabus table - if (not_overview) { - pct <- get_syllabus(path, questions = TRUE)$percents - names(pct) <- db$file[er[1]:er[2]] - } else { - pct <- character(0) - } # ------------------------ shim for downlit ---------------------------- # Bypass certain downlit functions that produce unintented effects such # as linking function documentation. @@ -102,12 +94,13 @@ build_site <- function(path = ".", quiet = !interactive(), preview = TRUE, overr # ------------------------ end downlit shim ---------------------------- for (i in files_to_render) { location <- page_location(i, abs_md, er) + progress <- if (not_overview) pct[db$file[i]] else pct build_episode_html( path_md = abs_md[i], path_src = abs_src[i], page_back = location["back"], page_forward = location["forward"], - page_progress = pct[db$file[i]], + page_progress = progress, date = db$date[i], pkg = pkg, quiet = quiet @@ -130,7 +123,8 @@ build_site <- function(path = ".", quiet = !interactive(), preview = TRUE, overr # # 2. home page which concatenates index.md and learners/setup.md describe_progress("Creating homepage", quiet = quiet) - build_home(pkg, quiet = quiet, next_page = abs_md[er[1]]) + home_next <- if (not_overview) abs_md[er[1]] else NULL + build_home(pkg, quiet = quiet, next_page = home_next) # Generated content ---------------------------------------------------------- # From e20969070fbdb282b01dde4df88fa16b8ee50a04 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 13:49:31 -0700 Subject: [PATCH 16/39] add skip --- tests/testthat/test-overview.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index 915ab986d..33da62131 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -25,6 +25,8 @@ test_that("Lessons without episodes can be built", { test_that("top level fig, files, and data directories are copied over", { + skip("Still working on the implementation") + fs::dir_create(fs::path(lsn, c("fig", "files", "data"))) fs::file_touch(fs::path(lsn, c("fig", "files", "data"), "hello.png")) From 9c80e877166f8ec82d8b0616d00a8e27e30c047f Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 16:17:11 -0700 Subject: [PATCH 17/39] add setup token so renv stops complaining --- tests/testthat/setup.R | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/testthat/setup.R b/tests/testthat/setup.R index c68f9bdf7..6c9776117 100644 --- a/tests/testthat/setup.R +++ b/tests/testthat/setup.R @@ -6,6 +6,11 @@ rmt <- fs::file_temp(pattern = "REMOTE-") setup_local_remote(repo = tmp, remote = rmt, verbose = FALSE) + no_gh_token <- !nzchar(Sys.getenv("GITHUB_PAT")) + if (no_gh_token && requireNamespace("gh", quietly = TRUE)) { + Sys.setenv("GITHUB_PAT" = gh::gh_token()) + withr::defer(Sys.unsetenv("GITHUB_PAT"), teardown_env()) + } noise <- interactive() || Sys.getenv("CI") == "true" if (noise) { cli::cli_alert_info("Current RENV_PATHS_ROOT {Sys.getenv('RENV_PATHS_ROOT')}") From d5503c9ca2f881c0c02b1ed6b11047844b64dbc3 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 9 Aug 2023 16:18:02 -0700 Subject: [PATCH 18/39] attempt to use different template (will not work) --- R/build_home.R | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/R/build_home.R b/R/build_home.R index 91c5d0235..6235ba2f3 100644 --- a/R/build_home.R +++ b/R/build_home.R @@ -22,7 +22,7 @@ build_home <- function(pkg, quiet, next_page = NULL) { setup <- xml2::read_html(setup) fix_nodes(setup) - nav <- get_nav_data(idx_file, fs::path_file(idx_file), page_forward = next_page) + nav <- get_nav_data(idx_file, page_forward = next_page) needs_title <- nav$pagetitle == "" if (needs_title) { @@ -44,8 +44,17 @@ build_home <- function(pkg, quiet, next_page = NULL) { nav$pagetitle <- NULL page_globals$metadata$update(nav) + is_overview <- identical(page_globals$metadata$get()$overview, TRUE) + if (is_overview) { + page_globals$instructor$set("overview", is_overview) + page_globals$learner$set("overview", is_overview) + template <- "overview" + } else { + template <- "syllabus" + } + cli::cli_alert("IS OVERVIEW? {page_globals$metadata$get()$overview}") - build_html(template = "syllabus", pkg = pkg, nodes = list(html, setup), + build_html(template = template, pkg = pkg, nodes = list(html, setup), global_data = page_globals, path_md = "index.html", quiet = quiet) } From e470defc887d6b63be69af8b5ce3c9d46916ced4 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 11 Aug 2023 16:08:32 -0700 Subject: [PATCH 19/39] add test for fixing overview metadata context It turns out that if there is _extra_ metadata, it will be carried over into the next lesson as the metadata is not cleared. This puts a patch for the most important ones, but I am going to need to include something that clears out the old metadata within the lesson validation process. --- tests/testthat/test-overview.R | 63 +++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index 33da62131..959c7e0cb 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -1,6 +1,68 @@ lsn <- restore_fixture() +test_that("We can switch between overview and regular lesson metadata", { + # CONTEXT --------------------------------------------------- + # I discovered that if I had built a _regular_ lesson after an overview lesson + # then the regular lesson accidentally inherited some metadata from the + # overview lesson. This extended to fields like overview and url, so I am + # explicitly checking these, but it does point at a bigger question. + # END CONTEXT ----------------------------------------------- + # make a duplicate of this lesson + tmp <- withr::local_tempfile() + fs::dir_copy(lsn, tmp) + this_metadata$clear() + clear_this_lesson() + + # check lesson defaults and norms --------------------------- + lcfg <- get_config(lsn) + expect_null(lcfg$overview) + expect_null(lcfg$url) + + # when we register the global variables for the lesson, they should NOT match + # the overview + less <- this_lesson(lsn) + lmeta <- this_metadata$get() + expect_type(lmeta, "list") + expect_false(less$overview) + expect_false(lmeta$overview) + expect_match(lmeta$url, "/lesson-example/", fixed = TRUE) + + # setup overview lesson ------------------------------------- + # remove first episode + reset_episodes(tmp) + # add overview to config + cat("\noverview: true\nurl: https://example.com/\n", file = fs::path(tmp, "config.yaml"), append = TRUE) + # delete episodes folder + fs::dir_delete(fs::path(tmp, "episodes")) + + expect_false(fs::dir_exists(fs::path(tmp, "episodes"))) + expect_false(fs::dir_exists(fs::path(tmp, "site", "docs"))) + + ocfg <- get_config(tmp) + expect_true(ocfg$overview) + expect_equal(ocfg$url, "https://example.com/") + + # when we register the global variables for the overview, they should match + # what is in our config + over <- this_lesson(tmp) + ometa <- this_metadata$get() + expect_type(ometa, "list") + expect_true(over$overview) + expect_true(ometa$overview) + expect_equal(ometa$url, "https://example.com/") + + # retest lesson to make sure the variables are reset + less <- this_lesson(lsn) + lmeta <- this_metadata$get() + expect_type(ometa, "list") + expect_false(less$overview) + expect_false(lmeta$overview) + expect_match(lmeta$url, "/lesson-example/", fixed = TRUE) + +}) + test_that("Lessons without episodes can be built", { + # remove first episode sandpaper::reset_episodes(lsn) # add overview to config @@ -22,7 +84,6 @@ test_that("Lessons without episodes can be built", { }) - test_that("top level fig, files, and data directories are copied over", { skip("Still working on the implementation") From 963516966bf659c6030cbe1028d7e1fea3821732 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 11 Aug 2023 16:10:47 -0700 Subject: [PATCH 20/39] set overview for metadata to be boolean --- R/utils-store.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/utils-store.R b/R/utils-store.R index b127b8bf0..25c0e0565 100644 --- a/R/utils-store.R +++ b/R/utils-store.R @@ -193,6 +193,8 @@ clear_resource_list <- function(path) { # set the global storage for {varnish} so that we do not have to recompute # things like the sidebar set_globals(path) + # kludge to make sure overview status is accurate + this_metadata$set("overview", .this_lesson$overview) # resource list of files for the lesson via `get_resource_list()` set_resource_list(path) instructor_globals$set("syllabus", From 55f497be7505c8092e8653e5834032a045e8d987 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 11 Aug 2023 16:11:08 -0700 Subject: [PATCH 21/39] always set URL for metadata --- R/utils-metadata.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/R/utils-metadata.R b/R/utils-metadata.R index 698caad56..bd50e7cf6 100644 --- a/R/utils-metadata.R +++ b/R/utils-metadata.R @@ -29,11 +29,11 @@ metadata_url <- function(cfg) { initialise_metadata <- function(path = ".") { cfg <- get_config(path) - if (length(this_metadata$get()) == 0) { + old <- this_metadata$get() + if (length(old) == 0L) { this_metadata$set(NULL, cfg) this_metadata$set("metadata_template", readLines(template_metadata())) this_metadata$set("pagetitle", cfg$title) - this_metadata$set("url", metadata_url(cfg)) this_metadata$set("keywords", cfg$keywords) created <- cfg$created %||% tail(gert::git_log(max = 1e6, repo = path)$time, 1) this_metadata$set(c("date", "created"), format(as.Date(created), "%F")) @@ -43,6 +43,7 @@ initialise_metadata <- function(path = ".") { } else { this_metadata$update(cfg) } + this_metadata$set("url", metadata_url(cfg)) this_metadata$set(c("date", "modified"), format(Sys.Date(), "%F")) this_metadata$set(c("date", "published"), format(Sys.Date(), "%F")) } From 4270bdcd932ec02b2c16aec9a3fd6958eec9d107 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 11 Aug 2023 16:19:28 -0700 Subject: [PATCH 22/39] fix this weird little test thing --- tests/testthat/test-set_dropdown.R | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/testthat/test-set_dropdown.R b/tests/testthat/test-set_dropdown.R index 2f0128428..d7540f4a3 100644 --- a/tests/testthat/test-set_dropdown.R +++ b/tests/testthat/test-set_dropdown.R @@ -33,7 +33,7 @@ cli::test_that_cli("set_config() will write items", { test_that("custom keys will return an error with default", { suppressMessages({ expect_snapshot_error( - set_config(c("test-key" = "hey!", "keytest" = "yo?"), + set_config(c("test-key" = "hey!", "keytest" = "yo?"), path = tmp, write = TRUE, create = FALSE) ) }) @@ -138,7 +138,7 @@ test_that("the schedule can be rearranged", { }) test_that("yaml lists are preserved with other schedule updates", { - + set_episodes(tmp, c("second-episode.Rmd", "introduction.Rmd"), write = TRUE) # regression test for https://github.com/carpentries/sandpaper/issues/53 expect_equal(get_episodes(tmp), c("second-episode.Rmd", "introduction.Rmd")) @@ -160,10 +160,12 @@ test_that("the schedule can be truncated", { # build the lesson here just to be absolutely sure withr::defer(use_package_cache(prompt = FALSE, quiet = TRUE)) - no_package_cache() - expect_silent(build_lesson(tmp, quiet = TRUE, preview = FALSE)) + suppressMessages({ + no_package_cache() + build_lesson(tmp, quiet = TRUE, preview = FALSE) + }) html <- xml2::read_html(fs::path(tmp, "site/docs/index.html")) - episodes <- xml2::xml_find_all(html, + episodes <- xml2::xml_find_all(html, ".//div[contains(@class, 'accordion-header')]/a") links <- xml2::xml_attr(episodes, "href") expect_length(links, 1) From ae8582d1ced7733c603fa15a2ddedd87043ae570 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 11 Aug 2023 16:20:34 -0700 Subject: [PATCH 23/39] set varnish version --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 6f4791e38..d8c604498 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -69,7 +69,7 @@ Additional_repositories: https://carpentries.r-universe.dev/ Remotes: ropensci/tinkr, carpentries/pegboard@allow-overview-pages, - carpentries/varnish + carpentries/varnish@allow-overview-pages SystemRequirements: pandoc (>= 2.11.4) - https://pandoc.org Encoding: UTF-8 LazyData: true From 6129e7c240534428fb800a8af2585327546aaa4e Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 22 Aug 2023 07:49:41 -0700 Subject: [PATCH 24/39] vectorize enforce_dir The underlying functions were already vectorized, so it makes sense that we can do that here and reduce the need for external for/vapply loops --- R/utils-paths.R | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/R/utils-paths.R b/R/utils-paths.R index 40982e703..967059af0 100644 --- a/R/utils-paths.R +++ b/R/utils-paths.R @@ -24,11 +24,12 @@ make_here <- function(ROOT) { } # creates a directory if it doesn't exist -enforce_dir <- function(path) { - if (!fs::dir_exists(path)) { - fs::dir_create(path) +enforce_dir <- function(paths) { + to_create <- !fs::dir_exists(paths) + if (any(to_create)) { + fs::dir_create(paths[to_create]) } - invisible(path) + invisible(paths) } path_site <- function(path = NULL) { From 50ebd6bd06c8b86e7b9a5641ee5fd96e51319737 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 22 Aug 2023 07:51:10 -0700 Subject: [PATCH 25/39] copy overview assets if they exist This was a manual process in build_markdown() that was taking valuable mental real-estate and it was frustrating to read because when I would look at it, it didn't immediately tell me what it was doing (copy all assets related to the build including non-markdown files and folders). With the addition of the overview pages, it was time to move this into a function so that I could have a bit more room to be verbose about what I was doing without distracting from build_markdown(). --- R/build_markdown.R | 37 ++++++++++++++++++++++------------ R/utils-paths-source.R | 2 +- tests/testthat/test-overview.R | 2 -- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/R/build_markdown.R b/R/build_markdown.R index d96385fdd..32b238171 100644 --- a/R/build_markdown.R +++ b/R/build_markdown.R @@ -25,10 +25,9 @@ build_markdown <- function(path = ".", rebuild = FALSE, quiet = FALSE, slug = NU create_site(path) } # check if the lesson needs to be reset - this_lesson(path) + lsn <- this_lesson(path) - episode_path <- path_episodes(path) - outdir <- path_built(path) + outdir <- path_built(path) # Determine build status for the episodes ------------------------------------ source_list <- .resources$get() %||% get_resource_list(path, warn = !quiet) @@ -64,16 +63,7 @@ build_markdown <- function(path = ".", rebuild = FALSE, quiet = FALSE, slug = NU }, add = TRUE) # Copy the files to the assets directory ------------------------------------- - artifacts <- get_artifacts(path, "episodes") - to_copy <- vapply( - c("data", "files", "fig"), - FUN = function(i) enforce_dir(fs::path(episode_path, i)), - FUN.VALUE = character(1) - ) - to_copy <- c(to_copy, artifacts) - for (f in to_copy) { - copy_assets(f, outdir) - } + copy_build_assets(path, outdir, overview = lsn$overview) # Remove detritus ------------------------------------------------------------ remove <- db$remove @@ -154,6 +144,27 @@ build_markdown <- function(path = ".", rebuild = FALSE, quiet = FALSE, slug = NU invisible(db$build) } +copy_build_assets <- function(path, outdir, overview = FALSE) { + path <- root_path(path) + # get all the non-markdown files + artifacts <- get_source_artifacts(path, "episodes") + resource_folders <- c("data", "files", "fig") + # enforce dir will create a directory if it doesn't exist, so that it's + # always available for the user, even if git is not tracking it. + to_copy <- enforce_dir(fs::path(path, "episodes", resource_folders)) + to_copy <- c(to_copy, artifacts) + if (overview) { + # overview lessons are special, so we are going to explicitly search the top + # directory for the resource folders and then copy them only if they exist + needed <- fs::dir_ls(path, type = "directory") + needed <- needed[fs::path_file(needed) %in% resource_folders] + to_copy <- c(needed, to_copy) + } + for (f in to_copy) { + copy_assets(f, outdir) + } +} + remove_rendered_html <- function(episodes) { htmls <- fs::path_ext_set(episodes, "html") exists <- fs::file_exists(htmls) diff --git a/R/utils-paths-source.R b/R/utils-paths-source.R index 1a7858ea3..a4828bfb5 100644 --- a/R/utils-paths-source.R +++ b/R/utils-paths-source.R @@ -144,7 +144,7 @@ get_sources <- function(path, subfolder = "episodes") { fs::path_abs(fs::dir_ls(pe, regexp = "*R?md")) } -get_artifacts <- function(path, subfolder = "episodes") { +get_source_artifacts <- function(path, subfolder = "episodes") { pe <- enforce_dir(fs::path(root_path(path), subfolder)) fs::dir_ls(pe, regexp = "*R?md", invert = TRUE, diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index 959c7e0cb..ee9131960 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -86,8 +86,6 @@ test_that("Lessons without episodes can be built", { test_that("top level fig, files, and data directories are copied over", { - skip("Still working on the implementation") - fs::dir_create(fs::path(lsn, c("fig", "files", "data"))) fs::file_touch(fs::path(lsn, c("fig", "files", "data"), "hello.png")) From e2bc41ff7c0c8064e142538e7466f978b334e230 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 22 Aug 2023 11:19:24 -0700 Subject: [PATCH 26/39] update some of the build component docs --- R/build_home.R | 20 +++++++++++++- R/build_html.R | 46 +++++++++++++++++++++++++++++--- _pkgdown.yml | 27 ++++++++++++------- man/build_home.Rd | 36 +++++++++++++++++++++++++ man/build_html.Rd | 67 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 183 insertions(+), 13 deletions(-) create mode 100644 man/build_home.Rd create mode 100644 man/build_html.Rd diff --git a/R/build_home.R b/R/build_home.R index 6235ba2f3..244dfc938 100644 --- a/R/build_home.R +++ b/R/build_home.R @@ -1,3 +1,22 @@ +#' Build a home page for a lesson +#' +#' @param pkg a list generated from [pkgdown::as_pkgdown()] from the `site/` +#' folder of a lesson. +#' @param quiet a boolean passed to [build_html()]. if `TRUE`, this will have +#' pkgdown report what files are being built +#' @param next_page the next page file name. This will allow the navigation +#' element to be set up correctly on the navigation bar +#' @return nothing. This is used for its side-effect +#' +#' @keywords internal +#' @details The index page of the lesson is a combination of two pages: +#' +#' 1. index.md (or README if the index does not exist) +#' 2. learners/setup.md +#' +#' This function uses [render_html()] to convert the page into HTML, which gets +#' passed on to the "syllabus" or "overview" templates in {varnish} (via the +#' [build_html()] function as the `{{{ readme }}}` and `{{{ setup }}}` keys. build_home <- function(pkg, quiet, next_page = NULL) { page_globals <- setup_page_globals() path <- root_path(pkg$src_path) @@ -52,7 +71,6 @@ build_home <- function(pkg, quiet, next_page = NULL) { } else { template <- "syllabus" } - cli::cli_alert("IS OVERVIEW? {page_globals$metadata$get()$overview}") build_html(template = template, pkg = pkg, nodes = list(html, setup), global_data = page_globals, path_md = "index.html", quiet = quiet) diff --git a/R/build_html.R b/R/build_html.R index b09a59563..e285f8d6a 100644 --- a/R/build_html.R +++ b/R/build_html.R @@ -1,3 +1,43 @@ +#' Build instructor and learner HTML page +#' +#' @param template the name of the {varnish} template to use. Defaults to +#' "chapter" +#' @param pkg an object created from [pkgdown::as_pkgdown()] +#' @param nodes an `xml_document` object. In the case of using this for the +#' index page (from [build_home()]), `nodes` will be a list of two +#' `xml_documents`; one for instructors and one for learners so that the +#' instructors have the schedule available to them (however, this might be a +#' relic, Zhian needs to inspect it). +#' @param global_data a list store object that contains copies of the global +#' variables for the page, including metadata, navigation, and variables for +#' the {varnish} templates. +#' @param path_md the path (absolute, relative, or filename) the current +#' markdown file being processed. +#' @param quiet This parameter is passed to [pkgdown::render_page()] and will +#' print the progress if `TRUE` (default). +#' @return `TRUE` if the page was built and `NULL` if it did not need to be +#' rebuilt +#' @keywords internal +#' +#' @details This function is a central workhorse that connects the global +#' lesson metadata and the global variables for each page to the rendering +#' engine: {pkgdown}. It will perform the global operations that includes +#' setting up the navigation, adding metadata, and building both the instructor +#' and learner versions of the page. +#' +#' In the Workbench, there are three types of pages: +#' +#' 1. primary content pages: these are primary content with a 1:1 relationship +#' between the source and the output. These are episodes along with custom +#' learner and instructor content +#' 2. aggregate content pages: pages that are aggregated from other pages such +#' as key points, all-in-one, images +#' 3. concatenated content pages: concatenations of source files and potentially +#' aggregate data. Examples are index, learner profiles, and the instructor +#' notes pages. +#' +#' Each of these types of pages have their own process for setting up content, +#' which gets processed before its passed here. build_html <- function(template = "chapter", pkg, nodes, global_data, path_md, quiet = TRUE) { ipath <- fs::path(pkg$dst_path, "instructor") if (!fs::dir_exists(ipath)) fs::dir_create(ipath) @@ -5,7 +45,7 @@ build_html <- function(template = "chapter", pkg, nodes, global_data, path_md, q this_page <- as_html(path_md, instructor = TRUE) meta <- global_data$metadata base_url <- meta$get()$url - + # Handle the differences between instructor and learner views for the index page if (inherits(nodes, "xml_document")) { instructor_nodes <- nodes @@ -19,7 +59,7 @@ build_html <- function(template = "chapter", pkg, nodes, global_data, path_md, q update_sidebar(global_data$instructor, instructor_nodes, path_md) meta$set("url", paste0(base_url, this_page)) global_data$instructor$set("json", fill_metadata_template(meta)) - modified <- pkgdown::render_page(pkg, + modified <- pkgdown::render_page(pkg, template, data = global_data$instructor$get(), depth = 1L, @@ -33,7 +73,7 @@ build_html <- function(template = "chapter", pkg, nodes, global_data, path_md, q update_sidebar(global_data$learner, learner_nodes, path_md) meta$set("url", paste0(base_url, this_page)) global_data$learner$set("json", fill_metadata_template(meta)) - pkgdown::render_page(pkg, + pkgdown::render_page(pkg, template, data = global_data$learner$get(), depth = 0L, diff --git a/_pkgdown.yml b/_pkgdown.yml index dc509bfc3..beb2380d9 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -18,7 +18,7 @@ code: reference: - title: "Lesson Creation" desc: > - Provision new lessons and/or episodes. These functions will likely only + Provision new lessons and/or episodes. These functions will likely only be used once. - contents: - create_lesson @@ -35,7 +35,7 @@ reference: - title: "Lesson Development Helpers" desc: > Functions to programmatically assess and modify configuration and source - elements of a lesson. These are often used when developing a lesson. + elements of a lesson. These are often used when developing a lesson. - contents: - get_drafts - get_config @@ -50,7 +50,7 @@ reference: - strip_prefix - title: "The Package Cache" desc: > - Lessons with generated content (R Markdown lessons) have an extra file + Lessons with generated content (R Markdown lessons) have an extra file called `renv/profiles/lesson-requirments/renv.lock` that records the package versions used to build the lesson. These functions provide ways for you to manage these packages and turn it on or off while previewing the @@ -77,18 +77,27 @@ reference: - ci_build_site - ci_session_info - git_worktree_setup - - title: "[Internal] Build Components" - desc: > - Individual components to provision files and build a lesson locally + - title: "[Internal] Markdown Build Components" + desc: > + Internal functions used to provision and build the markdown components + of a lesson. - contents: - this_lesson - build_handout - build_markdown - - build_site - build_episode_md - - build_episode_html - - render_html - sandpaper.options + - title: "[Internal] HTML Build Components" + desc: > + Internal functions used to provision and build the HTML components + of a lesson assuming that the markdown components have been built. + (this is non-exhaustive) + - contents: + - render_html + - build_site + - build_episode_html + - build_home + - build_html - title: "[Internal] Post-build Aggregation Components" desc: > Components to build aggregate pages such as All in One and Keypoints diff --git a/man/build_home.Rd b/man/build_home.Rd new file mode 100644 index 000000000..ae72fb2f6 --- /dev/null +++ b/man/build_home.Rd @@ -0,0 +1,36 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/build_home.R +\name{build_home} +\alias{build_home} +\title{Build a home page for a lesson} +\usage{ +build_home(pkg, quiet, next_page = NULL) +} +\arguments{ +\item{pkg}{a list generated from \code{\link[pkgdown:as_pkgdown]{pkgdown::as_pkgdown()}} from the \verb{site/} +folder of a lesson.} + +\item{quiet}{a boolean passed to \code{\link[=build_html]{build_html()}}. if \code{TRUE}, this will have +pkgdown report what files are being built} + +\item{next_page}{the next page file name. This will allow the navigation +element to be set up correctly on the navigation bar} +} +\value{ +nothing. This is used for its side-effect +} +\description{ +Build a home page for a lesson +} +\details{ +The index page of the lesson is a combination of two pages: +\enumerate{ +\item index.md (or README if the index does not exist) +\item learners/setup.md +} + +This function uses \code{\link[=render_html]{render_html()}} to convert the page into HTML, which gets +passed on to the "syllabus" or "overview" templates in {varnish} (via the +\code{\link[=build_html]{build_html()}} function as the \code{{{{ readme }}}} and \code{{{{ setup }}}} keys. +} +\keyword{internal} diff --git a/man/build_html.Rd b/man/build_html.Rd new file mode 100644 index 000000000..002f11f86 --- /dev/null +++ b/man/build_html.Rd @@ -0,0 +1,67 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/build_html.R +\name{build_html} +\alias{build_html} +\title{Build instructor and learner HTML page} +\usage{ +build_html( + template = "chapter", + pkg, + nodes, + global_data, + path_md, + quiet = TRUE +) +} +\arguments{ +\item{template}{the name of the {varnish} template to use. Defaults to +"chapter"} + +\item{pkg}{an object created from \code{\link[pkgdown:as_pkgdown]{pkgdown::as_pkgdown()}}} + +\item{nodes}{an \code{xml_document} object. In the case of using this for the +index page (from \code{\link[=build_home]{build_home()}}), \code{nodes} will be a list of two +\code{xml_documents}; one for instructors and one for learners so that the +instructors have the schedule available to them (however, this might be a +relic, Zhian needs to inspect it).} + +\item{global_data}{a list store object that contains copies of the global +variables for the page, including metadata, navigation, and variables for +the {varnish} templates.} + +\item{path_md}{the path (absolute, relative, or filename) the current +markdown file being processed.} + +\item{quiet}{This parameter is passed to \code{\link[pkgdown:render_page]{pkgdown::render_page()}} and will +print the progress if \code{TRUE} (default).} +} +\value{ +\code{TRUE} if the page was built and \code{NULL} if it did not need to be +rebuilt +} +\description{ +Build instructor and learner HTML page +} +\details{ +This function is a central workhorse that connects the global +lesson metadata and the global variables for each page to the rendering +engine: {pkgdown}. It will perform the global operations that includes +setting up the navigation, adding metadata, and building both the instructor +and learner versions of the page. + +In the Workbench, there are three types of pages: +\enumerate{ +\item primary content pages: these are primary content with a 1:1 relationship +between the source and the output. These are episodes along with custom +learner and instructor content +\item aggregate content pages: pages that are aggregated from other pages such +as key points, all-in-one, images +\item concatenated content pages: concatenations of source files and potentially +aggregate data. Examples are index, learner profiles, and the instructor +notes pages. +} + +Each of these types of pages have their own process for setting up content, +which gets processed before its passed here. +} +\keyword{internal} From 6598e16be22138da2ffacd84ceb11d9bf2130c60 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 22 Aug 2023 12:03:37 -0700 Subject: [PATCH 27/39] add fix for weird home file badness --- R/build_home.R | 3 ++- tests/testthat/test-overview.R | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/R/build_home.R b/R/build_home.R index 244dfc938..2db9d6a5d 100644 --- a/R/build_home.R +++ b/R/build_home.R @@ -41,7 +41,8 @@ build_home <- function(pkg, quiet, next_page = NULL) { setup <- xml2::read_html(setup) fix_nodes(setup) - nav <- get_nav_data(idx_file, page_forward = next_page) + idx_src <- fs::path(path, fs::path_file(idx_file)) + nav <- get_nav_data(idx_file, idx_src, page_forward = next_page) needs_title <- nav$pagetitle == "" if (needs_title) { diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index ee9131960..edfaf7729 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -81,6 +81,13 @@ test_that("Lessons without episodes can be built", { expect_true(fs::dir_exists(fs::path(lsn, "site", "built"))) expect_true(fs::dir_exists(fs::path(lsn, "site", "docs"))) + # read in index and make sure the destinations are correct + idx_file <- fs::path(lsn, "site", "docs", "index.html") + expect_true(fs::file_exists(idx_file)) + idx <- xml2::read_html(idx_file) + edit_link <- xml2::xml_find_first(idx, ".//a[text()='Edit on GitHub']") + expect_match(xml2::xml_attr(edit_link, "href"), "edit/main/index.md") + }) From 429e9d54353170fed396ab11b1cc866f75f44b6b Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 22 Aug 2023 15:55:44 -0700 Subject: [PATCH 28/39] set_config to actually use logical values --- R/set_dropdown.R | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/R/set_dropdown.R b/R/set_dropdown.R index 481bf2158..d3ac5eb72 100644 --- a/R/set_dropdown.R +++ b/R/set_dropdown.R @@ -173,7 +173,13 @@ set_config <- function(pairs = NULL, create = FALSE, path = ".", write = FALSE) # creates a character vector for the number of keys we need line <- character(length(keys)) for (i in seq(keys)) { - line[i] <- glue::glue("{keys[[i]]}: {siQuote(values[[i]])}") + vali <- values[[i]] + if (is.logical(vali)) { + vali <- if (vali) "true" else "false" + } else { + vali <- siQuote(vali) + } + line[i] <- glue::glue("{keys[[i]]}: {vali}") } if (create) { appends <- what == -9L From 40ff1c33de7ba08ad314376104da4ba796f95e2d Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 22 Aug 2023 15:56:47 -0700 Subject: [PATCH 29/39] use official config setter in tests --- tests/testthat/test-overview.R | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index edfaf7729..28cece5b9 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -1,6 +1,6 @@ -lsn <- restore_fixture() test_that("We can switch between overview and regular lesson metadata", { + lsn <- restore_fixture() # CONTEXT --------------------------------------------------- # I discovered that if I had built a _regular_ lesson after an overview lesson # then the regular lesson accidentally inherited some metadata from the @@ -31,7 +31,10 @@ test_that("We can switch between overview and regular lesson metadata", { # remove first episode reset_episodes(tmp) # add overview to config - cat("\noverview: true\nurl: https://example.com/\n", file = fs::path(tmp, "config.yaml"), append = TRUE) + suppressMessages({ + set_config(list(overview = TRUE, url = "https://example.com/"), + path = tmp, create = TRUE, write = TRUE) + }) # delete episodes folder fs::dir_delete(fs::path(tmp, "episodes")) @@ -62,11 +65,15 @@ test_that("We can switch between overview and regular lesson metadata", { }) test_that("Lessons without episodes can be built", { + lsn <- restore_fixture() # remove first episode sandpaper::reset_episodes(lsn) # add overview to config - cat("\noverview: true\n", file = fs::path(lsn, "config.yaml"), append = TRUE) + suppressMessages({ + set_config(list(overview = TRUE, url = "https://example.com/"), + path = tmp, create = TRUE, write = TRUE) + }) # delete episodes folder fs::dir_delete(fs::path(lsn, "episodes")) @@ -93,6 +100,23 @@ test_that("Lessons without episodes can be built", { test_that("top level fig, files, and data directories are copied over", { + lsn <- restore_fixture() + + # remove first episode + sandpaper::reset_episodes(lsn) + # add overview to config + suppressMessages({ + set_config(list(overview = TRUE, url = "https://example.com/"), + path = tmp, create = TRUE, write = TRUE) + }) + # delete episodes folder + fs::dir_delete(fs::path(lsn, "episodes")) + + expect_false(fs::dir_exists(fs::path(lsn, "episodes"))) + expect_false(fs::dir_exists(fs::path(lsn, "site", "docs"))) + expect_true(get_config(lsn)$overview) + + fs::dir_create(fs::path(lsn, c("fig", "files", "data"))) fs::file_touch(fs::path(lsn, c("fig", "files", "data"), "hello.png")) From 0babf1e25040c5d7e6e7c67de1c711c45a187184 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 22 Aug 2023 15:58:22 -0700 Subject: [PATCH 30/39] update set_config docs --- NEWS.md | 1 + R/set_dropdown.R | 23 ++++++++++++++++++----- man/set_config.Rd | 20 ++++++++++++++++---- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index 82ddf7b32..def8903cc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,6 +11,7 @@ - Internal function `root_path()` will no longer fail if the `episodes/` folder does not exist as long as one of the other four folders (`site/`, `learners/`, `instructors/`, `profiles/`) exists. +- `set_config()` can now properly process logical values into `true` and `false` # sandpaper 0.12.5 (unreleased) diff --git a/R/set_dropdown.R b/R/set_dropdown.R index d3ac5eb72..47e8237b3 100644 --- a/R/set_dropdown.R +++ b/R/set_dropdown.R @@ -65,8 +65,8 @@ set_dropdown <- function(path = ".", order = NULL, write = FALSE, folder) { #' Set individual keys in a configuration file #' -#' @param pairs a named character vector with keys as the names and the new -#' values as the contents +#' @param pairs a named list or character vector with keys as the names and the +#' new values as the contents #' @param create if `TRUE`, any new values in `pairs` will be created and #' appended; defaults to `FALSE`, which prevents typos from sneaking in. #' single key-pair values currently supported. @@ -87,7 +87,8 @@ set_dropdown <- function(path = ".", order = NULL, write = FALSE, folder) { #' - **title** `[character]` the lesson title (e.g. `'Introduction to R for #' Plant Pathologists'` #' - **created** `[character]` Date in ISO 8601 format (e.g. `'2021-02-09'`) -#' - **keywords** `[character]` comma-separated list (e.g `'static site, R, tidyverse'`) +#' - **keywords** `[character]` comma-separated list (e.g `'static site, R, +#' tidyverse'`) #' - **life_cycle** `[character]` one of pre-alpha, alpha, beta, stable #' - **license** `[character]` a license for the lesson (e.g. `'CC-BY 4.0'`) #' - **source** `[character]` the source repository URL @@ -104,8 +105,11 @@ set_dropdown <- function(path = ".", order = NULL, write = FALSE, folder) { #' - **fail_on_error** `[boolean]` for R Markdown lessons; fail the build if any #' chunks produce an error. Use `#| error: true` in chunk options to allow the #' error to be displayed -#' - **workbench-beta** yes `[boolean]` if truthy, this displays a banner on the +#' - **workbench-beta** `[boolean]` if truthy, this displays a banner on the #' site that indicates the site is in the workbench beta phase. +#' - **overview** `[boolean]` All lessons must have episodes with the exception +#' of overview lessons. To indicate that your lesson serves as an overview for +#' other lessons, use `overview: true` #' #' As the workbench becomes more developed, some of these optional keys may #' disappear. @@ -121,6 +125,8 @@ set_dropdown <- function(path = ".", order = NULL, write = FALSE, folder) { #' version to use #' - **varnish** `[character]` github string or version number of varnish #' version to use +#' - **pegboard** `[character]` github string or version number of pegboard +#' version to use #' #' For example, if you had forked your own version of varnish to modify the #' colourscheme, you could use: @@ -144,11 +150,18 @@ set_dropdown <- function(path = ".", order = NULL, write = FALSE, folder) { #' if (FALSE) { #' tmp <- tempfile() #' create_lesson(tmp, "test lesson", open = FALSE, rmd = FALSE) -#' # Change the title and License +#' # Change the title and License (default vars) #' set_config(c(title = "Absolutely Free Lesson", license = "CC0"), #' path = tmp, #' write = TRUE #' ) +#' +#' # add the URL and workbench-beta indicator +#' set_config(list("workbench-beta" = TRUE, url = "https://example.com/"), +#' path = tmp, +#' create = TRUE, +#' write = TRUE +#' ) #' } set_config <- function(pairs = NULL, create = FALSE, path = ".", write = FALSE) { keys <- names(pairs) diff --git a/man/set_config.Rd b/man/set_config.Rd index da501cc05..4a1b066da 100644 --- a/man/set_config.Rd +++ b/man/set_config.Rd @@ -7,8 +7,8 @@ set_config(pairs = NULL, create = FALSE, path = ".", write = FALSE) } \arguments{ -\item{pairs}{a named character vector with keys as the names and the new -values as the contents} +\item{pairs}{a named list or character vector with keys as the names and the +new values as the contents} \item{create}{if \code{TRUE}, any new values in \code{pairs} will be created and appended; defaults to \code{FALSE}, which prevents typos from sneaking in. @@ -53,8 +53,11 @@ the default github pages io domain. \item \strong{fail_on_error} \verb{[boolean]} for R Markdown lessons; fail the build if any chunks produce an error. Use \verb{#| error: true} in chunk options to allow the error to be displayed -\item \strong{workbench-beta} yes \verb{[boolean]} if truthy, this displays a banner on the +\item \strong{workbench-beta} \verb{[boolean]} if truthy, this displays a banner on the site that indicates the site is in the workbench beta phase. +\item \strong{overview} \verb{[boolean]} All lessons must have episodes with the exception +of overview lessons. To indicate that your lesson serves as an overview for +other lessons, use \code{overview: true} } As the workbench becomes more developed, some of these optional keys may @@ -70,6 +73,8 @@ but to provision these versions on GitHub, you can provision these in the version to use \item \strong{varnish} \verb{[character]} github string or version number of varnish version to use +\item \strong{pegboard} \verb{[character]} github string or version number of pegboard +version to use } For example, if you had forked your own version of varnish to modify the @@ -93,10 +98,17 @@ varnish: carpentries/varnish@BRANCH-name if (FALSE) { tmp <- tempfile() create_lesson(tmp, "test lesson", open = FALSE, rmd = FALSE) -# Change the title and License +# Change the title and License (default vars) set_config(c(title = "Absolutely Free Lesson", license = "CC0"), path = tmp, write = TRUE ) + +# add the URL and workbench-beta indicator +set_config(list("workbench-beta" = TRUE, url = "https://example.com/"), + path = tmp, + create = TRUE, + write = TRUE +) } } From 2f57438839a864217dd4ba0f92808eca3f58238d Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 22 Aug 2023 16:37:48 -0700 Subject: [PATCH 31/39] add shortcut for parse_file_matches and update doc --- R/utils-paths-source.R | 43 ++++++++++++++++++++++++-- _pkgdown.yml | 1 + man/parse_file_matches.Rd | 55 ++++++++++++++++++++++++++++++++++ tests/testthat/test-overview.R | 4 +++ 4 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 man/parse_file_matches.Rd diff --git a/R/utils-paths-source.R b/R/utils-paths-source.R index a4828bfb5..c8b39104a 100644 --- a/R/utils-paths-source.R +++ b/R/utils-paths-source.R @@ -134,7 +134,8 @@ get_resource_list <- function(path, trim = FALSE, subfolder = NULL, warn = FALSE # These are the only four items that we need to consider order for. for (i in subfolder) { # If the configuration is not missing, then we have to rearrange the order. - res[[i]] <- parse_file_matches(res[[i]], cfg[[i]], warn = warn, i) + res[[i]] <- parse_file_matches(reality = res[[i]], hopes = cfg[[i]], + warn = warn, subfolder = i) } if (use_subfolder) res[[subfolder]] else res[names(res) != "site"] } @@ -153,8 +154,46 @@ get_source_artifacts <- function(path, subfolder = "episodes") { ) } +#' Subset file matches to the order they appear in the config file +#' +#' @param reality a list of paths that exist in the lesson +#' @param hopes a list of files in the order they should appear in the lesson +#' @param warn a boolean. If `TRUE` and the `sandpaper.show_draft` option is +#' set to TRUE, then the files that are not in `hopes` are shown to the +#' screen as drafts +#' @param subfolder a character. The folder where we should find the files in +#' `hopes`. This is only used for creating an error message. +#' @return a character vector of `reality` subset in the order of `hopes` +#' @keywords internal +#' @examples +#' # setup ---------------------------------------------------- +#' # +#' # NOTE: we need to define our namespace here because using `:::` +#' # in example calls is illegal. +#' snd <- asNamespace("sandpaper") +#' print(need <- c("a", "bunch", "of", "silly", "files")) +#' print(exists <- fs::path("path", "to", sample(need))) +#' +#' # Rearrange files ------------------------------------------ +#' snd$parse_file_matches(reality = exists, hopes = need, +#' subfolder = "episodes") +#' +#' # a subset of files ---------------------------------------- +#' snd$parse_file_matches(reality = exists, +#' hopes = need[4:5], subfolder = "episodes") +#' +#' # a subset of files with a warning ------------------------- +#' op <- getOption("sandpaper.show_draft") +#' options(sandpaper.show_draft = TRUE) +#' on.exit(options(sandpaper.show_draft = op)) +#' snd$parse_file_matches(reality = exists, +#' hopes = need[-(4:5)], warn = TRUE, subfolder = "episodes") +#' +#' # files that do not exist give an error -------------------- +#' try(snd$parse_file_matches(reality = exists, +#' hopes = c("these", need[4:5]), subfolder = "episodes")) parse_file_matches <- function(reality, hopes = NULL, warn = FALSE, subfolder) { - if (is.null(hopes)) { + if (is.null(hopes) || is.null(reality)) { return(reality) } real_files <- fs::path_file(reality) diff --git a/_pkgdown.yml b/_pkgdown.yml index beb2380d9..cd7f1f476 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -117,6 +117,7 @@ reference: - get_built_db - get_hash - get_resource_list + - parse_file_matches - template_episode - yaml_list - title: "[Developer] Lesson Test Fixture" diff --git a/man/parse_file_matches.Rd b/man/parse_file_matches.Rd new file mode 100644 index 000000000..c5330b3eb --- /dev/null +++ b/man/parse_file_matches.Rd @@ -0,0 +1,55 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils-paths-source.R +\name{parse_file_matches} +\alias{parse_file_matches} +\title{Subset file matches to the order they appear in the config file} +\usage{ +parse_file_matches(reality, hopes = NULL, warn = FALSE, subfolder) +} +\arguments{ +\item{reality}{a list of paths that exist in the lesson} + +\item{hopes}{a list of files in the order they should appear in the lesson} + +\item{warn}{a boolean. If \code{TRUE} and the \code{sandpaper.show_draft} option is +set to TRUE, then the files that are not in \code{hopes} are shown to the +screen as drafts} + +\item{subfolder}{a character. The folder where we should find the files in +\code{hopes}. This is only used for creating an error message.} +} +\value{ +a character vector of \code{reality} subset in the order of \code{hopes} +} +\description{ +Subset file matches to the order they appear in the config file +} +\examples{ +# setup ---------------------------------------------------- +# +# NOTE: we need to define our namespace here because using `:::` +# in example calls is illegal. +snd <- asNamespace("sandpaper") +print(need <- c("a", "bunch", "of", "silly", "files")) +print(exists <- fs::path("path", "to", sample(need))) + +# Rearrange files ------------------------------------------ +snd$parse_file_matches(reality = exists, hopes = need, + subfolder = "episodes") + +# a subset of files ---------------------------------------- +snd$parse_file_matches(reality = exists, + hopes = need[4:5], subfolder = "episodes") + +# a subset of files with a warning ------------------------- +op <- getOption("sandpaper.show_draft") +options(sandpaper.show_draft = TRUE) +on.exit(options(sandpaper.show_draft = op)) +snd$parse_file_matches(reality = exists, + hopes = need[-(4:5)], warn = TRUE, subfolder = "episodes") + +# files that do not exist give an error -------------------- +try(snd$parse_file_matches(reality = exists, + hopes = c("these", need[4:5]), subfolder = "episodes")) +} +\keyword{internal} diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index 28cece5b9..71ef39a8e 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -12,6 +12,10 @@ test_that("We can switch between overview and regular lesson metadata", { fs::dir_copy(lsn, tmp) this_metadata$clear() clear_this_lesson() + withr::defer({ + this_metadata$clear() + clear_this_lesson() + }) # check lesson defaults and norms --------------------------- lcfg <- get_config(lsn) From a81e943080c381e9f52ff45770b9be1fb9e346c0 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 23 Aug 2023 16:37:06 -0700 Subject: [PATCH 32/39] set overview status on outset --- R/build_home.R | 2 -- R/utils-store.R | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/build_home.R b/R/build_home.R index 2db9d6a5d..2b5ff0f98 100644 --- a/R/build_home.R +++ b/R/build_home.R @@ -66,8 +66,6 @@ build_home <- function(pkg, quiet, next_page = NULL) { page_globals$metadata$update(nav) is_overview <- identical(page_globals$metadata$get()$overview, TRUE) if (is_overview) { - page_globals$instructor$set("overview", is_overview) - page_globals$learner$set("overview", is_overview) template <- "overview" } else { template <- "syllabus" diff --git a/R/utils-store.R b/R/utils-store.R index 25c0e0565..4a74d7723 100644 --- a/R/utils-store.R +++ b/R/utils-store.R @@ -200,6 +200,8 @@ clear_resource_list <- function(path) { instructor_globals$set("syllabus", create_syllabus(.resources$get()[["episodes"]], .this_lesson, path) ) + learner_globals$set("overview", .this_lesson$overview) + instructor_globals$set("overview", .this_lesson$overview) invisible(.this_lesson) }, clear = function() { From e1589c54503df17719881e8ab03eed261784bbbd Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Wed, 23 Aug 2023 16:37:20 -0700 Subject: [PATCH 33/39] revamp test --- tests/testthat/test-overview.R | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index 71ef39a8e..f541cc653 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -126,14 +126,15 @@ test_that("top level fig, files, and data directories are copied over", { withr::local_options(list("sandpaper.use_renv" = FALSE)) sandpaper::build_lesson(lsn, quiet = TRUE, preview = FALSE) + sitepath <- fs::path(lsn, "site", "docs") - expect_true(fs::dir_exists(fs::path(lsn, "site", "docs", "fig"))) - expect_true(fs::dir_exists(fs::path(lsn, "site", "docs", "files"))) - expect_true(fs::dir_exists(fs::path(lsn, "site", "docs", "data"))) + expect_true(fs::dir_exists(fs::path(sitepath, "fig"))) + expect_true(fs::dir_exists(fs::path(sitepath, "files"))) + expect_true(fs::dir_exists(fs::path(sitepath, "data"))) - expect_true(fs::file_exists(fs::path(lsn, "site", "docs", "fig", "hello.png"))) - expect_true(fs::file_exists(fs::path(lsn, "site", "docs", "files", "hello.png"))) - expect_true(fs::file_exists(fs::path(lsn, "site", "docs", "data", "hello.png"))) + expect_true(fs::file_exists(fs::path(sitepath, "fig", "hello.png"))) + expect_true(fs::file_exists(fs::path(sitepath, "files", "hello.png"))) + expect_true(fs::file_exists(fs::path(sitepath, "data", "hello.png"))) }) From 90aa8daccee7eb2a4838ea62fe370845d10e7f8b Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Thu, 24 Aug 2023 11:28:05 -0700 Subject: [PATCH 34/39] bump versions of varnish and pegboard --- DESCRIPTION | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index d8c604498..db21f2ccc 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -31,7 +31,7 @@ Description: We provide tools to build a Carpentries-themed lesson repository License: MIT + file LICENSE Imports: pkgdown (>= 1.6.0), - pegboard (>= 0.5.1), + pegboard (>= 0.6.0), cli (>= 3.4.0), commonmark, fs (>= 1.5.0), @@ -64,7 +64,7 @@ Suggests: jsonlite, sessioninfo, mockr, - varnish (>= 0.2.1) + varnish (>= 0.3.0) Additional_repositories: https://carpentries.r-universe.dev/ Remotes: ropensci/tinkr, From e46437854ce50d8b0f6f3390f297e6f8c967c7f2 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Thu, 24 Aug 2023 11:28:23 -0700 Subject: [PATCH 35/39] add content tests for varnish home and setup links --- tests/testthat/test-overview.R | 36 ++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/testthat/test-overview.R b/tests/testthat/test-overview.R index f541cc653..fd5097ca9 100644 --- a/tests/testthat/test-overview.R +++ b/tests/testthat/test-overview.R @@ -92,13 +92,49 @@ test_that("Lessons without episodes can be built", { expect_true(fs::dir_exists(fs::path(lsn, "site", "built"))) expect_true(fs::dir_exists(fs::path(lsn, "site", "docs"))) + # CONTENT TESTING ----------------------------------------------------- # read in index and make sure the destinations are correct idx_file <- fs::path(lsn, "site", "docs", "index.html") expect_true(fs::file_exists(idx_file)) idx <- xml2::read_html(idx_file) + + # we should have an edit link in the main page edit_link <- xml2::xml_find_first(idx, ".//a[text()='Edit on GitHub']") expect_match(xml2::xml_attr(edit_link, "href"), "edit/main/index.md") + # links to home and setup should appear in the navigation + xpath_home_link_mobi <- ".//nav//div[starts-with(@class,'accordion ')]/a" + xpath_setup_link_desk <- ".//nav//a[@class='nav-link']" + xpath_setup_link_mobi <- ".//nav//div[@class='accordion-body']/ul/li/a" + + # the home link exists on mobile view + home_link_mobi <- xml2::xml_find_first(idx, xpath_home_link_mobi) + + # the setup link should be prominent in both desktop and mobile view + setup_link_desk <- xml2::xml_find_first(idx, xpath_setup_link_desk) + setup_link_mobi <- xml2::xml_find_first(idx, xpath_setup_link_desk) + + expect_match(xml2::xml_attr(home_link_mobi, "href"), "index.html") + expect_match(xml2::xml_attr(setup_link_desk, "href"), "index.html#setup") + expect_match(xml2::xml_attr(setup_link_mobi, "href"), "index.html#setup") + + + # Instructor notes should have above properties ----------------------------- + ins_file <- fs::path(lsn, "site", "docs", "instructor", "instructor-notes.html") + expect_true(fs::file_exists(ins_file)) + ins <- xml2::read_html(ins_file) + + # the home link exists on mobile view + home_link_mobi <- xml2::xml_find_first(ins, xpath_home_link_mobi) + + # the setup link should be prominent in both desktop and mobile view + setup_link_desk <- xml2::xml_find_first(ins, xpath_setup_link_desk) + setup_link_mobi <- xml2::xml_find_first(ins, xpath_setup_link_mobi) + + expect_match(xml2::xml_attr(home_link_mobi, "href"), "index.html") + expect_match(xml2::xml_attr(setup_link_desk, "href"), "index.html#setup") + expect_match(xml2::xml_attr(setup_link_mobi, "href"), "index.html#setup") + }) From f60feedc1fa6fd3055378f3e91ddc669622dd984 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 29 Aug 2023 07:21:48 -0700 Subject: [PATCH 36/39] use released pegboard --- DESCRIPTION | 1 - 1 file changed, 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index db21f2ccc..d1aaf09d0 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -68,7 +68,6 @@ Suggests: Additional_repositories: https://carpentries.r-universe.dev/ Remotes: ropensci/tinkr, - carpentries/pegboard@allow-overview-pages, carpentries/varnish@allow-overview-pages SystemRequirements: pandoc (>= 2.11.4) - https://pandoc.org Encoding: UTF-8 From 43c6735e23e0b154ad58665aa2db2c029cea032a Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Tue, 29 Aug 2023 08:27:42 -0700 Subject: [PATCH 37/39] add dev pegboard --- DESCRIPTION | 1 + 1 file changed, 1 insertion(+) diff --git a/DESCRIPTION b/DESCRIPTION index d1aaf09d0..c1f50b968 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -68,6 +68,7 @@ Suggests: Additional_repositories: https://carpentries.r-universe.dev/ Remotes: ropensci/tinkr, + carpentries/pegboard, carpentries/varnish@allow-overview-pages SystemRequirements: pandoc (>= 2.11.4) - https://pandoc.org Encoding: UTF-8 From 16e270b82d5ffd26822db83dcf26b67188a9d9d1 Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 1 Sep 2023 11:28:41 -0700 Subject: [PATCH 38/39] use varnish default branch --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index e2828a816..43ed648de 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -73,7 +73,7 @@ Additional_repositories: https://carpentries.r-universe.dev/ Remotes: ropensci/tinkr, carpentries/pegboard, - carpentries/varnish@allow-overview-pages + carpentries/varnish SystemRequirements: pandoc (>= 2.11.4) - https://pandoc.org Encoding: UTF-8 LazyData: true From 7af7beddc25755d88a9e64e4abc1685d11de932f Mon Sep 17 00:00:00 2001 From: "Zhian N. Kamvar" Date: Fri, 1 Sep 2023 11:28:48 -0700 Subject: [PATCH 39/39] clean up news a bit --- NEWS.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 48a59153e..9aeb084c2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,7 +4,9 @@ * Overview style lessons that do not have episodic content can now be processed, analysed, and built by {sandpaper}. To make your lesson an overview lesson, - you can add `overview: true` to your `config.yaml` + you can add `overview: true` to your `config.yaml` (reported: @zkamvar, + https://github.com/carpentries/workbench/issues/65; + implemented: @zkamvar, #496) * The new `spoiler` class of fenced div will allow authors to specify an expandable section of content that is collapsed by default. This replaces the former paradigm of using "floating solution" blocks to present options for @@ -14,7 +16,7 @@ * Internal function `root_path()` will no longer fail if the `episodes/` folder does not exist as long as one of the other four folders (`site/`, `learners/`, - `instructors/`, `profiles/`) exists. + `instructors/`, `profiles/`) exists (fixed: @zkamvar, #496) * `set_config()` can now properly process logical values into `true` and `false` * R Markdown documents with modificiations to child documents will now take into account changes to the child documents (reported @jcolomb, #497; fixed