Merge pull request #86 from carpentries/update-link_processing

update fix links to process headings
carpentries · May 11, 2022 · 307dffa · 307dffa
2 parents 7c5bace + bf2a288
commit 307dffa
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 6 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,6 +1,6 @@
 Package: pegboard
 Title: Explore and Manipulate Markdown Curricula from the Carpentries
-Version: 0.2.5
+Version: 0.2.6
 Authors@R:c(
     person(given = "Zhian N.",
            family = "Kamvar",

diff --git a/NEWS.md b/NEWS.md
@@ -1,3 +1,12 @@
+# pegboard 0.2.6
+
+## BUG FIX
+
+ - `fix_links()` now processes links in headers and links with unescaped ampersands
+    - internal function `text_to_links()` now processes unescaped ampersands
+    - internal function `find_lesson_links()` no longer expects links to be 
+      strictly in paragraph elements.
+
 # pegboard 0.2.5
 
 ## MISC

diff --git a/R/fix_links.R b/R/fix_links.R
@@ -53,7 +53,7 @@ fix_link_type <- function(type, body) {
 find_lesson_links <- function(body, type = "rel_link") {
   ns <- NS(body)
   xml2::xml_find_all(body,
-    glue::glue(".//{ns}paragraph/{ns}text[{LINKS[[type]]}][not(@klink)]")
+    glue::glue(".//{ns}text[{LINKS[[type]]}][not(@klink)]")
   )
 }
 
@@ -139,6 +139,9 @@ text_to_links <- function(txt, ns = NULL, type, sourcepos = NULL) {
 
   texts <- strsplit(txt, rgx, perl = TRUE)[[1]]
   texts <- texts[texts != ""]
+  # escape ampersands that are not valid code points, though this will still
+  # allow invalid code points, but it's better than nothing
+  texts <- gsub("[&](?![#]?[A-Za-z0-9]+?[;])", "&amp;", texts, perl = TRUE)
   are_links <- grepl(lnk, texts, perl = TRUE)
   texts[are_links]  <- purrr::map_chr(texts[are_links], make_link, pattern = lnk, type = type)
   texts[!are_links] <- glue::glue("<text>{texts[!are_links]}</text>")

diff --git a/tests/testthat/test-conversion.R b/tests/testthat/test-conversion.R
@@ -13,9 +13,13 @@ test_that("Episodes with commonmark-violating liquid relative links can be read"
   lnsp <- test_path("examples", "link-split.md")
   withr::defer(rm("lnsp", "tmp"))
   # Not fixing liquid will make the parser sad
-  expect_error(Episode$new(lnsp))
+  bad <- Episode$new(lnsp)
+  # One real link and one anchor: no bueno :(
+  expect_length(bad$links, 2)
+
   # fixing liquid will resucue us!
   tmp <- Episode$new(lnsp, fix_liquid = TRUE)
+  expect_length(tmp$links, 4)
 
   expect_equal(basename(tmp$path), "link-split.md")
   expect_snapshot(cat(tmp$show(), sep = "\n"))

diff --git a/tests/testthat/test-make_pandoc_alt.R b/tests/testthat/test-make_pandoc_alt.R
@@ -1,14 +1,14 @@
 
 test_that("make_pandoc_alt() converts alt text good", {
 
-  skip_if(R.version$major < 4)
-  f <- textConnection(paste(c("![has alt text](img1.png)", 
+  txt <- paste(c("![has alt text](img1.png)", 
       "",
    "![](needs-alt.png)", 
       "",
    "![ ](decorative.png)",
       "",
-   "![has alt text](img2.png){: .class}", ""), collapse = "\n"))
+   "![has alt text](img2.png){: .class}", ""), collapse = "\n")
+  f <- textConnection(txt)
   body <- tinkr::to_xml(f)$body
   ns <- tinkr::md_ns()
   images <- xml2::xml_find_all(body, ".//md:image", ns = ns)