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

fix aggregate functions to not fail with special episode names #512

Merged
merged 7 commits into from
Sep 18, 2023

Conversation

zkamvar
Copy link
Contributor

@zkamvar zkamvar commented Sep 13, 2023

In short, there was a problem if someone named an episode starting with images-, the build would fail (see #511). In addition, even if it did succeed, the link in the page would fail because it would have the images- slug trimmed off.

I have done the following actions to fix this:

  1. added tests in tests/testthat/test-utils-aggregate.R
  2. removed slug trimming in the make_[thing]_section family of functions
  3. added a prefix = TRUE catch in the main aggregate function

I also styled these files using styler

Testing

To test this PR, you can install it locally with remotes::install_github() or pak::pkg_install() and then run the example below.

pak::pkg_install("carpentries/sandpaper#512")

Example

This PR will fix that issue and a working reprex is demonstrated below:

tmp <- tempfile()
sandpaper::create_lesson(tmp, rmd = FALSE, open = FALSE)
#> → Creating Lesson in '/tmp/Rtmp2CZhpv/file17fbe74349550'...
#> ℹ No schedule set, using Rmd files in 'episodes/' directory.
#> → Creating Lesson in '/tmp/Rtmp2CZhpv/file17fbe74349550'...→ To remove this message, define your schedule in 'config.yaml' or use `set_episodes()` to generate it.
#> → Creating Lesson in '/tmp/Rtmp2CZhpv/file17fbe74349550'...────────────────────────────────────────────────────────────────────────────────
#> → Creating Lesson in '/tmp/Rtmp2CZhpv/file17fbe74349550'...ℹ To save this configuration, use
#> 
#> set_episodes(path = path, order = ep, write = TRUE)
#> → Creating Lesson in '/tmp/Rtmp2CZhpv/file17fbe74349550'...✔ First episode created in '/tmp/Rtmp2CZhpv/file17fbe74349550/episodes/introduction.md'
#> → Creating Lesson in '/tmp/Rtmp2CZhpv/file17fbe74349550'...ℹ Workflows up-to-date!
#> → Creating Lesson in '/tmp/Rtmp2CZhpv/file17fbe74349550'...✔ Lesson successfully created in '/tmp/Rtmp2CZhpv/file17fbe74349550'
#> → Creating Lesson in '/tmp/Rtmp2CZhpv/file17fbe74349550'...
#> /tmp/Rtmp2CZhpv/file17fbe74349550
sandpaper::create_episode_md("images and pixels", path = tmp, add = TRUE)
sandpaper::build_lesson(tmp, quiet = TRUE, preview = FALSE)
fs::dir_tree(tmp, recurse = 2)
#> /tmp/Rtmp2CZhpv/file17fbe74349550
#> ├── CODE_OF_CONDUCT.md
#> ├── CONTRIBUTING.md
#> ├── LICENSE.md
#> ├── README.md
#> ├── config.yaml
#> ├── episodes
#> │   ├── data
#> │   ├── fig
#> │   ├── files
#> │   ├── images-and-pixels.md
#> │   └── introduction.md
#> ├── index.md
#> ├── instructors
#> │   └── instructor-notes.md
#> ├── learners
#> │   ├── reference.md
#> │   └── setup.md
#> ├── links.md
#> ├── profiles
#> │   └── learner-profiles.md
#> └── site
#>     ├── DESCRIPTION
#>     ├── README.md
#>     ├── _pkgdown.yaml
#>     ├── built
#>     │   ├── CODE_OF_CONDUCT.md
#>     │   ├── LICENSE.md
#>     │   ├── config.yaml
#>     │   ├── data
#>     │   ├── fig
#>     │   ├── files
#>     │   ├── images-and-pixels.md
#>     │   ├── index.md
#>     │   ├── instructor-notes.md
#>     │   ├── introduction.md
#>     │   ├── learner-profiles.md
#>     │   ├── links.md
#>     │   ├── md5sum.txt
#>     │   ├── reference.md
#>     │   └── setup.md
#>     └── docs
#>         ├── 404.html
#>         ├── CODE_OF_CONDUCT.html
#>         ├── LICENSE.html
#>         ├── aio.html
#>         ├── android-chrome-192x192.png
#>         ├── android-chrome-512x512.png
#>         ├── apple-touch-icon.png
#>         ├── assets
#>         ├── bootstrap-toc.css
#>         ├── bootstrap-toc.js
#>         ├── config.yaml
#>         ├── data
#>         ├── docsearch.css
#>         ├── docsearch.js
#>         ├── favicon-16x16.png
#>         ├── favicon-32x32.png
#>         ├── favicons
#>         ├── fig
#>         ├── files
#>         ├── images-and-pixels.html
#>         ├── images.html
#>         ├── index.html
#>         ├── instructor
#>         ├── instructor-notes.html
#>         ├── introduction.html
#>         ├── key-points.html
#>         ├── link.svg
#>         ├── md5sum.txt
#>         ├── mstile-150x150.png
#>         ├── pkgdown.css
#>         ├── pkgdown.js
#>         ├── pkgdown.yml
#>         ├── profiles.html
#>         ├── reference.html
#>         ├── safari-pinned-tab.svg
#>         ├── site.webmanifest
#>         └── sitemap.xml

Created on 2023-09-13 with reprex v2.0.2
This will fix #511

This addresses #511 by testing that `build_images()` does not clobber an
episode that starts with `images-`, `build_instructor_notes()` does not
clobber an episode that starts with `instructor-notes-` and so on...
@zkamvar
Copy link
Contributor Author

zkamvar commented Sep 18, 2023

I am going to self-merge this because I would like to make sure the changes are incorporated into #513

@zkamvar zkamvar merged commit 73d5204 into main Sep 18, 2023
12 checks passed
@zkamvar zkamvar deleted the fix-images-511 branch September 18, 2023 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lesson build fails when an episode name starts with the word 'images'
1 participant