Skip to content

Commit

Permalink
Merge pull request #409 from carpentries/fix-instructor-view-links-404
Browse files Browse the repository at this point in the history
fix instructor view asset links
  • Loading branch information
zkamvar authored Mar 14, 2023
2 parents a11a6a4 + 2465756 commit 40f250c
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 6 deletions.
6 changes: 6 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# sandpaper 0.11.9 (in development)

BUG FIX
-------

* Links to assets in instructor view no longer render a 404. (reported:
@brownsarahm, #404; fixed: @zkamvar, #408)

CONTINUOUS INTEGRATION
----------------------

Expand Down
24 changes: 18 additions & 6 deletions R/utils-xml.R
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ fix_codeblocks <- function(nodes = NULL) {
add_code_heading <- function(codes = NULL, labels = "OUTPUT") {
if (length(codes) == 0) return(codes)
xml2::xml_set_attr(codes, "tabindex", "0")
heads <- xml2::xml_add_sibling(codes, "h3", labels, class = "code-label",
heads <- xml2::xml_add_sibling(codes, "h3", labels, class = "code-label",
.where = "before")
for (head in heads) {
xml2::xml_add_child(head, "i",
xml2::xml_add_child(head, "i",
"aria-hidden" = "true", "data-feather" = "chevron-left")
xml2::xml_add_child(head, "i",
xml2::xml_add_child(head, "i",
"aria-hidden" = "true", "data-feather" = "chevron-right")
}
invisible(codes)
Expand Down Expand Up @@ -97,7 +97,7 @@ fix_setup_link <- function(nodes = NULL) {
if (length(nodes) == 0) return(nodes)
links <- xml2::xml_find_all(nodes, ".//a")
hrefs <- xml2::url_parse(xml2::xml_attr(links, "href"))
setup_links <- hrefs$scheme == "" &
setup_links <- hrefs$scheme == "" &
hrefs$server == "" &
hrefs$path == "setup.html"
xml2::xml_set_attr(links[setup_links], "href", "index.html#setup")
Expand All @@ -115,9 +115,21 @@ use_learner <- function(nodes = NULL) {
use_instructor <- function(nodes = NULL) {
if (length(nodes) == 0) return(nodes)
copy <- xml2::read_html(as.character(nodes))
# lnk <- xml2::xml_find_all(copy, ".//a[not(starts-with(@href, 'http'))]")
# find all local links and transform non-html and nested links ---------
lnk <- xml2::xml_find_all(copy,
".//a[@href][not(contains(@href, '://')) and not(starts-with(@href, '#'))]"
)
lnk_hrefs <- xml2::xml_attr(lnk, "href")
lnk_paths <- xml2::url_parse(lnk_hrefs)$path
# links without HTML extension
not_html <- !fs::path_ext(lnk_paths) %in% c("html", "")
# links that are not in the root directory (e.g. files/a.html, but not ./a.html)
is_nested <- lengths(strsplit(sub("^[.][/]", "", lnk_paths), "/")) > 1
is_above <- not_html | is_nested
lnk_hrefs[is_above] <- fs::path("../", lnk_hrefs[is_above])
xml2::xml_set_attr(lnk, "href", lnk_hrefs)
# find all images and refer back to source
img <- xml2::xml_find_all(copy, ".//img[not(starts-with(@src, 'http'))]")
# xml2::xml_set_attr(lnk, "href", fs::path("instructor/", xml2::xml_attr(lnk, "href")))
xml2::xml_set_attr(img, "src", fs::path("../", xml2::xml_attr(img, "src")))
as.character(copy)
}
34 changes: 34 additions & 0 deletions tests/testthat/_snaps/utils-xml.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# paths in instructor view that are nested or not HTML get diverted

Code
xml2::xml_find_all(html_test, ".//a[@href]")
Output
{xml_nodeset (10)}
[1] <a href="index.html">a</a>
[2] <a href="./index.html">b</a>
[3] <a href="fig/thing.png">c</a>
[4] <a href="./fig/thang.jpg">d</a>
[5] <a href="data/thing.csv">e</a>
[6] <a href="files/papers/thing.pdf">f</a>
[7] <a href="files/confirmation.html">g</a>
[8] <a href="#what-the">h</a>
[9] <a href="other-page.html#section">i</a>
[10] <a href="other-page">j</a>

---

Code
xml2::xml_find_all(res, ".//a[@href]")
Output
{xml_nodeset (10)}
[1] <a href="index.html">a</a>
[2] <a href="./index.html">b</a>
[3] <a href="../fig/thing.png">c</a>
[4] <a href=".././fig/thang.jpg">d</a>
[5] <a href="../data/thing.csv">e</a>
[6] <a href="../files/papers/thing.pdf">f</a>
[7] <a href="../files/confirmation.html">g</a>
[8] <a href="#what-the">h</a>
[9] <a href="other-page.html#section">i</a>
[10] <a href="other-page">j</a>

26 changes: 26 additions & 0 deletions tests/testthat/test-utils-xml.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,30 @@


test_that("paths in instructor view that are nested or not HTML get diverted", {
html_test <- xml2::read_html(commonmark::markdown_html(c(
"<a id='what-the'></a><h2>h</h2>\n",
"[a](index.html)",
"[b](./index.html)",
"[c](fig/thing.png)", # asset
"[d](./fig/thang.jpg)", # asset
"[e](data/thing.csv)", # asset
"[f](files/papers/thing.pdf)", # asset
"[g](files/confirmation.html)", # asset
"[h](#what-the)",
"[i](other-page.html#section)",
"[j](other-page)"
)))
res <- xml2::read_html(use_instructor(html_test))
# refs are transformed according to our rules
refs <- xml2::xml_text(xml2::xml_find_all(res, ".//@href"))
expect_equal(startsWith(refs, "../"),
c(FALSE, FALSE, TRUE, TRUE, TRUE, TRUE, TRUE, FALSE, FALSE, FALSE))
expect_snapshot(xml2::xml_find_all(html_test, ".//a[@href]"))
expect_snapshot(xml2::xml_find_all(res, ".//a[@href]"))
})



test_that("empty args result in nothing happening", {
expect_null(fix_nodes())
expect_null(fix_setup_link())
Expand Down

0 comments on commit 40f250c

Please sign in to comment.