carpentries / varnish

Template for pkgdown site
https://carpentries.github.io/varnish/
Other
7 stars 29 forks source link

Bugfix: Avoid double html escaping of lesson title. #95

Closed Robadob closed 1 year ago

Robadob commented 1 year ago

There is currently a bug whereby title (from config.yaml), if containing &, will be double encoded to converting them to & in 4 locations of the templates. This presumably applies to other HTML encoded characters too.

See: https://github.com/r-lib/pkgdown/pull/298, where they addressed the same issue, albeit with a different fix (which caused NULL to be rendered when I attempted with the carpentries template).

The same fix already appears present on this and the following lines: https://github.com/carpentries/varnish/blob/8b930523e56f4003b381353d6f945228664584a4/inst/pkgdown/templates/header.html#L42

I'm not particularly familiar with pkgdown, mustache (where I found this alt-fix) or R, so it's possible there's a better resolution or I've missed something. I did however check all files in inst/pkgdown/templates for {{title}}.

I discovered this whilst developing a forked varnish with internal branding for a new course I'll likely be developing soon (we may submit to incubator when it's more than a plan). You can find that WIP here with the issue resolved.

I also noticed that this lesson from the incubator list, had simply switch to and in order to presumably work around this issue.

zkamvar commented 1 year ago

Thank you for the fix! This was definitely an oversight on my part. I've tested it with https://zkamvar.github.io/test-varnish-95/ and have confirmed that it works!

I would like to add you as a contributor, would you want to update the DESCRIPTION file with your information?

Once that happens, we will bump the version and release the patch.

Robadob commented 1 year ago

I would like to add you as a contributor, would you want to update the DESCRIPTION file with your information?

Have done, I presume you'll update the changelog within NEWS.md?

Thanks

zkamvar commented 1 year ago

That indeed I will, thank you!