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

Web API dependent HTML validation #1495

Merged
merged 9 commits into from
Apr 4, 2025
Merged

Conversation

katrinabrock
Copy link
Contributor

@katrinabrock katrinabrock commented Apr 3, 2025

Closes #1494

@katrinabrock
Copy link
Contributor Author

This code is actually working. (CI is just complaining because I didn't add curl as a dependency, but it's already a transitive dependency with xfun.)

I marked it as a draft only because I don't think it should be merged without further discussion. In particular, sourcing a helper from test-rmd.R seems like it's against the spirit of test-rmd.R's simplicity.

@katrinabrock katrinabrock marked this pull request as ready for review April 4, 2025 11:44
@katrinabrock
Copy link
Contributor Author

katrinabrock commented Apr 4, 2025

@yihui This is fully working from my perspective. I implemented Option 2 form the options described in #1494.

You can see the tests are passing with no changes to the actual bookdown code. To show they're working, I merged the changes in this PR into the branch demoing the silent failure.

One-line diff between that branch and this branch
git diff html-validation silent-breakage-demo 
diff --git a/R/html.R b/R/html.R
index dad883f8..6959364c 100644
--- a/R/html.R
+++ b/R/html.R
@@ -279,7 +279,7 @@ split_chapters = function(
   # Need to take care of the div tags here before restore_part_html and
   # restore_appendix_html erase the section ids of the hidden PART or APPENDIX
   # sections.
-  if (split_level > 1) {
+  if (FALSE) {
     body = x[(i5 + 1):(i6 - 1)]
     h1 = grep('^<div (id="[^"]+" )?class="section level1("| )', body) + i5
     h2 = grep('^<div (id="[^"]+" )?class="section level2("| )', body) + i5

You can see the CI job is now failing with

 Error in eval(quote({ : 
  HTML issues detected in section-1.html, section-1.html, section-2.html, section-2.html, section-3.html, section-3.html, sub2.html, sub2.html, sub2.html, sub2.html, subsection-1.html, subsection-1.html, subsection-1.html, subsection-1.html, subsection-3.html, subsection-3.html, subsection-3.html, subsection-3.html, subsection-footnotes-2.html, subsection-footnotes-2.html, subsection-footnotes-2.html, subsection-footnotes-2.html, test-footnote.html, test-footnote.html
Calls: local -> eval.parent -> eval -> eval -> eval -> eval
Execution halted

except for the Mac and Windows jobs on which I skip this test as they don't have docker installed.

The passing of mac and windows jobs demonstrates that with no API and W3C_MARKUP_VALIDATOR_BASEURL unset, the tests pass without running the HTML validation so this will be opt-in for bookdown developer's local workflow. It will not impact end users at all because bookdown code itself isn't changing, only tests and CI.

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is amazing. Thank you!

@katrinabrock
Copy link
Contributor Author

katrinabrock commented Apr 4, 2025

Glad you like it!

btw - here is what the errors look like with the status quo code when you don't make any exceptions:

Error in eval(quote({ : 13 HTML issues detected:
  custom-environments.html: CSS: “border-top”: “solid\9” is not a “color” value.
  custom-environments.html: CSS: “border-bottom”: “solid\9” is not a “color” value.
  custom-environments.html: Attribute “number” not allowed on element “div” at this point.
  section-1.html: Attribute “number” not allowed on element “div” at this point.
  section-2.html: Attribute “number” not allowed on element “div” at this point.
  section-3.html: Attribute “number” not allowed on element “div” at this point.
  sub2.html: Attribute “number” not allowed on element “div” at this point.
  subsection-1.html: Attribute “number” not allowed on element “div” at this point.
  subsection-22.html: Attribute “number” not allowed on element “div” at this point.
  subsection-3.html: Attribute “number” not allowed on element “div” at this point.
  subsection-footnotes-1.html: Attribute “number

Probably easy enough to resolve. Might make a good first issue for someone.

@yihui yihui merged commit f7b9271 into rstudio:main Apr 4, 2025
14 checks passed
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.

[Testing] CI ignores changes that break HTML
2 participants