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

Feature request: differentiate between links and images in validation reports #143

Closed
joelnitta opened this issue Nov 10, 2023 · 7 comments · Fixed by #144
Closed

Feature request: differentiate between links and images in validation reports #143

joelnitta opened this issue Nov 10, 2023 · 7 comments · Fixed by #144

Comments

@joelnitta
Copy link

When lesson validation runs (via build_lesson()), I get a message about broken links, but no indication of the actual URL, so it is difficult to act on this information.

── Validating Internal Links and Images 
! There were errors in 3/80 links

sandpaper v 0.14.1

@zkamvar
Copy link
Contributor

zkamvar commented Nov 10, 2023

Hi Joel! It should be displayed in a list below these warnings as one of the first things that sandpaper does is call validate_lesson(), which will display the results of the validation failures underneath the notification:

sandpaper::validate_lesson()                                                                
── Validating Fenced Divs ────────────────────────────────────────────────────────────────────
── Validating Internal Links and Images ──────────────────────────────────────────────────────
! There were errors in 1/166 linksSome linked internal files do not exist <https://carpentries.github.io/sandpaper/articles/include-child-documents.html#workspace-consideration>

episodes/episodes.Rmd:893 [missing file]: [Example of Wrapped Alt Text (with apologies to William Carlos Williams)](fig/freezer.png)

If you are looking on GitHub, these can be found in the summary of each build:

For example, here is the summary for the most recent build of sandpaper docs showing the file that is missing to demonstrate the value of alt text:

GitHub Annotation showing one missing file warning

That being said, do you think it would be helpful to populate the source marker pane in RStudio (akin to the way {lintr} displays errors)?

@zkamvar
Copy link
Contributor

zkamvar commented Nov 10, 2023

Also, note: everything regarding the validation is within the realm of {pegboard}, so I'm moving the issue over there.

@zkamvar zkamvar transferred this issue from carpentries/sandpaper Nov 10, 2023
@joelnitta
Copy link
Author

Hm... I just updated to the most recent pegboard (0.7.1) and sandpaper (0.14.1), and I'm still not seeing the URLs of the reported links:

> build_lesson(rebuild=TRUE)
── Validating Fenced Divs ────────────────────────────────────────────
── Validating Internal Links and Images ──────────────────────────────
! There were errors in 3/80 links
◌ Images need alt-text <https://webaim.org/techniques/hypertext/link_text#alt_link>

episodes/introduction.Rmd:87 [image missing alt-text]: https://allisonhorst.github.io/palmerpenguins/reference/figures/lter_penguins.png
episodes/basic-targets.Rmd:170 [image missing alt-text]: https://allisonhorst.github.io/palmerpenguins/reference/figures/culmen_depth.png
episodes/quarto.Rmd:73 [image missing alt-text]: https://ucsbcarpentry.github.io/Reproducible-Publications-with-RStudio-Quarto/fig/03-qmd-workflow.png
ℹ Consent to use package cache provided

Furthermore, the images that have been reported as missing alt-text actually do have alt-text, but I think that should be a separate issue.

FYI the lesson I am trying to build is here: https://github.com/carpentries-incubator/targets-workshop

@joelnitta
Copy link
Author

do you think it would be helpful to populate the source marker pane in RStudio (akin to the way {lintr} displays errors)?

Sure, although I think if the checks were reported correctly during build_lesson(), that would probably be sufficient.

@zkamvar
Copy link
Contributor

zkamvar commented Nov 16, 2023

Ah! I think I see what the problem is! It's none other than our old friend, fluid representations!

In terms of {pegboard}, it processes images and links the same way because an image is a special class of link, and I never ended up writing text to distinguish them -_-

For the breadcrumbs, when you call sandpaper::validate_lesson(), it will call the $validate_links() method for each Markdown file, which is stored as an Episode object:

pegboard/R/Episode.R

Lines 629 to 635 in e7e5c45

validate_links = function(warn = TRUE) {
res <- validate_links(self)
if (warn) {
throw_link_warnings(res)
}
invisible(res)
}

This will call the (poorly documented) throw_link_warnings() function, which will collect the information from the validation table and present it in a friendly fashion, passing it to the issue_warnings() function:

issue_warning(what = "links",
cli = has_cli(),
n = nrow(err),
N = nrow(VAL),
infos = link_info[failed],
reports = reports)

issue_warnings() generates the summary that you see printed:

pegboard/R/utils-cli.R

Lines 36 to 39 in e7e5c45

what <- if (is.null(what)) "elements" else what
n <- if (is.null(n)) "?" else n
N <- if (is.null(N)) "?" else N
msg <- "There were errors in {n}/{N} {what}"

What I could do is to modify the throw_link_warnings() function to switch between "links", "images", and "links and images" depending on the errors so that it's more clear.

@zkamvar
Copy link
Contributor

zkamvar commented Nov 16, 2023

In terms of the alt text reporting, the reports are correct.

The markdown syntax for The Workbench comes from pandoc, which uses the square braces for captions, not alt text (see https://carpentries.github.io/sandpaper-docs/style.html#figures).

I would suggest something like this:

![The three species of penguins in the `palmerpenguins` dataset. Artwork by @allison_horst.
](https://allisonhorst.github.io/palmerpenguins/reference/figures/lter_penguins.png){
alt="Illustration of three penguins with colourful backgrounds.
Labels above their heads read: 'Chinstrap!', 'Gentoo!', and 'Adélie'"
}

@zkamvar zkamvar changed the title Feature request: specify URLs of missing links Feature request: differentiate between links and images in validation reports Nov 16, 2023
zkamvar added a commit that referenced this issue Nov 16, 2023
@zkamvar
Copy link
Contributor

zkamvar commented Nov 17, 2023

I have made the release and it will be generally available within the hour.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants