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

Error when re-building site when package man pages have aliases starting with [<- #2582

Closed
trevorld opened this issue May 24, 2024 · 8 comments · Fixed by #2583
Closed

Error when re-building site when package man pages have aliases starting with [<- #2582

trevorld opened this issue May 24, 2024 · 8 comments · Fixed by #2583

Comments

@trevorld
Copy link

  • This seems to be is a duplicate of Error when building vctrs #2505 with a minimal reproducible example.
  • I've created a minimal, reproducible example: https://github.com/trevorld/pkgdown.issue2505
  • This reproduces for me with {pkgdown} development and release
  • Seems to be an erorr when re-building site when the package has man pages that have aliases starting with [<-?
  • Error doesn't occur if you delete the site and call pkgdown::build_site() but it will occur if you call pkgdown::build_site() a second time without deleting the site beforehand:
unlink("docs", recursive = TRUE)
pkgdown::build_site()
pkgdown::build_site()
── Installing package pkgdown.issue2505 into temporary library ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
── Building pkgdown site for package pkgdown.issue2505 ─────────────────────────
Reading from: /home/trevorld/a/projects/R/pkgdown.issue2505
Writing to: /home/trevorld/a/projects/R/pkgdown.issue2505/docs
── Sitrep ──────────────────────────────────────────────────────────────────────
✔ URLs ok.
✔ Open graph metadata ok.
✔ Articles metadata ok.
✔ Reference metadata ok.
── Initialising site ───────────────────────────────────────────────────────────
── Building home ───────────────────────────────────────────────────────────────
Reading README.md
Writing 404.html
── Building function reference ─────────────────────────────────────────────────
Reading man/Extract.bm_matrix.Rd
── Building sitemap ────────────────────────────────────────────────────────────
Error: 
! in callr subprocess.
Caused by error in `.f(.x[[i]], ...)`:
! Failed to wrap URL "https://trevorldavis.com/R/pkgdown.issue2505/reference/[<-.bm_bitmap.html"
Caused by error in `read_xml.raw()`:
! StartTag: invalid element name [68]
ℹ See `$stderr` for standard error.
Type .Last.error to see the more details.

Enter a frame number, or 0 to exit   

1: pkgdown::build_site()
2: build_site_external(pkg = pkg, examples = examples, run_dont_run = run_dont
3: callr::r(function(..., cli_colors, hyperlinks, pkgdown_internet) {
    optio
4: get_result(output = out, options)
5: throw(callr_remote_error(remerr, output), parent = fix_msg(remerr[[3]]))
6: base::stop(cond)
hadley added a commit that referenced this issue May 24, 2024
* Only message if sitemap changed
* Simplify generation
* Escape file names. Fixes #2582.
@hadley
Copy link
Member

hadley commented May 24, 2024

Thanks for the reprex. So useful!

@hadley
Copy link
Member

hadley commented May 24, 2024

Hmmm, #2583 suggests that the problem is actually the file names, since windows can't even write them.

@trevorld
Copy link
Author

> xml2::url_escape("[<-.bm_bitmap.html")
[1] "%5B%3C-.bm_bitmap.html"

@trevorld
Copy link
Author

  • I guess if you escaped the file names upon creation you'd need to double escape the links?
  • Or maybe just skip creating redirects for any of the [<- aliases? Or skip if building on Windows?

@hadley
Copy link
Member

hadley commented May 25, 2024

Yeah maybe skipping creation makes sense?

Also makes me wonder how redirects should be correctly encoded in the sitemap.

@trevorld
Copy link
Author

Besides [<- there may be other theoretical operator aliases that may cause similar issues like > and &?

x <- numeric(1L)
class(x) <- "awkward"
g <- function(e1, e2) { "problematic alias?" }

`/.awkward` <- g
x / x

`+.awkward` <- g
x + x

`&.awkward` <- g
x & x

`<.awkward` <- g
x < x

`>.awkward` <- g
x > x

@hadley
Copy link
Member

hadley commented May 26, 2024

I'd suggest we just copy what roxygen2 does: https://github.com/r-lib/roxygen2/blob/main/R/utils.R#L12-L62

@hadley
Copy link
Member

hadley commented May 28, 2024

https://developers.google.com/search/docs/crawling-indexing/sitemaps/build-sitemap suggests that the sitemap should include canonical urls, so we shouldn't include redirects there at all.

hadley added a commit that referenced this issue May 29, 2024
* Only message if sitemap changed
* Simplify generation
* Exclude redirects from sitemap.
* Exclude invalid filenames from alias redirects

Fixes #2582
SebKrantz pushed a commit to SebKrantz/pkgdown that referenced this issue Jun 1, 2024
* Only message if sitemap changed
* Simplify generation
* Exclude redirects from sitemap.
* Exclude invalid filenames from alias redirects

Fixes r-lib#2582
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 a pull request may close this issue.

2 participants