Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch jump links to use identifiers #1497

Closed
JordanPedersen opened this issue Feb 23, 2023 · 9 comments · Fixed by #1519
Closed

switch jump links to use identifiers #1497

JordanPedersen opened this issue Feb 23, 2023 · 9 comments · Fixed by #1519

Comments

@JordanPedersen
Copy link

Note: https://preview.carpentries.org/instructor-training/instructor/checkout.html#id_1-lesson-contributions Instructor teaching demo link is broken.

@brownsarahm
Copy link
Contributor

@zkamvar thislooks like within page jump links are not being processed the same as they did before.

@zkamvar
Copy link
Contributor

zkamvar commented Mar 6, 2023

Oh that's an interesting feature/bug! It looks like pandoc's auto_identifiers extension strips the leading number from numbered sections (emphasis mine):

The default algorithm used to derive the identifier from the heading text is:

  • Remove all formatting, links, etc.
  • Remove all footnotes.
  • Remove all non-alphanumeric characters, except underscores, hyphens, and periods.
  • Replace all spaces and newlines with hyphens.
  • Convert all alphabetic characters to lowercase.
  • Remove everything up to the first letter (identifiers may not begin with a number or punctuation mark).
  • If nothing is left after this, use the identifier section.

I believe the solution is for me to update {sandpaper} to use the gfm_auto_identifiers extension instead, which should be updated here: https://github.com/carpentries/sandpaper/blob/3bb22eff927a1fbe1017000f3ba8c2d91244b89a/R/render_html.R#L67

As for why #1-introduction is being transformed to #id_1-introduction, that's a bit of a mystery that I have to dig further into.

@zkamvar
Copy link
Contributor

zkamvar commented Mar 6, 2023

Another solution before I fix carpentries/sandpaper#403 is to add identifiers to these sections:

1. [Make a small contribution to a lesson or glossary](#contributions).
2. [Take part in an online community discussion session](#discussion).
3. [Take part in an online teaching demonstration session](#demonstration).

## 1\. Lesson Contributions {#contributions}

...


## 2\. Community Discussion {#discussion}

...


## 3\. Teaching Demonstration {#demonstration}

@brownsarahm
Copy link
Contributor

I personally like the identifiers because it makes the links more explicit and decouples the linking from the displayed title (eg we can make an update to the section headers without having to update the links) but I am interested in what other @carpentries/instructor-training-maintainers think on this, especially @karenword because in practice she does the most writing in the curriculum.

@ndporter
Copy link
Contributor

ndporter commented Mar 7, 2023

I'd second the preference to decouple links from titles/headers as a way to make our code more robust and reduce the need to adjust multiple things at once when changes/maintenance happen.

@karenword
Copy link
Contributor

Using identifiers makes sense to me. I was only using headers because that's what I knew how to do. :) Happy to learn a better way!

@brownsarahm
Copy link
Contributor

brownsarahm commented Mar 8, 2023

I'm going to update the title of this issue to match the required action, of switching to identifiers.

We will need to fix this at least in this section, but should probably also search through the curriculum for similar cases, but I am not exactly sure how to search for this issue...

@brownsarahm brownsarahm changed the title Broken url in Checkout - Instructor teaching demo switch jump links to use identifiers Mar 8, 2023
@zkamvar
Copy link
Contributor

zkamvar commented Mar 9, 2023

I'm going to update the title of this issue to match the required action, of switching to identifiers.

We will need to fix this at least in this section, but should probably also search through the curriculum for similar cases, but I am not exactly sure how to search for this issue...

I've gone ahead and searched for you (which also highlights some work I need to do with the validators), and there are no other links like this across the lesson that need to be fixed.

library("pegboard")
library("purrr") # for map_dfr
it <- get_lesson("carpentries/instructor-training", jekyll = FALSE)
#> Loading required namespace: gert
extra_links <- purrr::map_dfr(it$extra, function(x) x$validate_links())
#
# [lots of output here revealing http links that Zhian missed during conversions]
# 
extra_links[grepl("^[0-9]", extra_links$fragment), c("orig", "text", "sourcepos", "filepath")]
#>                          orig
#> 190   #1-lesson-contributions
#> 191   #2-community-discussion
#> 192 #3-teaching-demonstration
#>                                                      text sourcepos
#> 190     Make a small contribution to a lesson or glossary        14
#> 191   Take part in an online community discussion session        15
#> 192 Take part in an online teaching demonstration session        16
#>                 filepath
#> 190 learners/checkout.md
#> 191 learners/checkout.md
#> 192 learners/checkout.md
links <- it$validate_links()
links[grepl("^[0-9]", links$fragment), c("orig", "text", "sourcepos", "filepath")]
#> [1] orig      text      sourcepos filepath 
#> <0 rows> (or 0-length row.names)

Created on 2023-03-09 with reprex v2.0.2

@zkamvar
Copy link
Contributor

zkamvar commented Mar 14, 2023

One little update on this, while I was fixing the instructor view, I found that there are empty ID anchors that can be switched to use the curly anchor syntax in demo_lessons:

### <a id="dc-ecology"></a> Data Carpentry: Ecology

The empty anchors were a bit of a workaround to allow creation of a table of contents: a30a340

zkamvar added a commit that referenced this issue Mar 31, 2023
During my processing of this lesson, I had not enabled link validation
of the auxilary markdown files (that is, non-episode files), so a lot of
them had https links and sneaky ways of creating anchor links that were
not accessible
(see
#1497 (comment))

As of the update to {pegboard} 0.5.0, these external files are now
processed.
zkamvar added a commit that referenced this issue Mar 31, 2023
I have updated the anchor links in instructor notes and checkout so that
they will be consistent

This will fix #1497
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants