New hook to check contents of _pkgdown.yaml contains all topics

[gravesti]

I'd like to propose a new hook that checks if _pkgdown.yaml contains all of the topics in man/. I'm not sure if this is a general enough issue but currently we are wasting a lot of time with commits failing the upstream checks when pkgdown tries to rebuild the site.
Errors like:
* Topics missing from index: my_new_function

My prototype hook simply reads in each of the .Rd files and checks that one of the \alias{} entries is included in a contents section of _pkgdown.yaml. It might be interesting to check the other direction too.

[lorenzwalthert]

Thanks @gravesti for raising this. I think indeed it could be useful. However, I'd not try to emulate what {pkgdown} does as a check, as this is less reliable than using the {pkgdown} functionality directly and may also change in the future. Steps involved to leverage existing infra:

• Find the relevant code in {pkgdown}. E.g. from the error / warning message you mention, one could do a full text search in the source code of {pkgdown} to find it.
• Create a hook with it.
• Ask the maintainers of {pkgdown} to export that functionality if not already the case.

Are you interested in contributing that?

[gravesti]

Hi @lorenzwalthert, yes I'm interested in looking at this further. I'm hoping to avoid having to run too much of the pkgdown process each commit though.

[gravesti]

It seems it would be enough to test if generating the indexes completes without error, ie to run

pkgdown::build_reference_index()
pkgdown::build_articles_index()

Unfortunately this brings a dependency on Pandoc as it does actually builds the html index pages. Is it possible within pre-commit checks? Otherwise I'll ask if the pkgdown devs can export some of the internal functions.

[lorenzwalthert]

Pandoc dependency sounds like trouble, in particular in pre-commit.ci settings. Did you find the internal functions in {pkgdown} that are responsible to trigger * Topics missing from index: my_new_function. It would be nice if they exported it, but we can possibly also rely on internal methods as a strategy of last resort.

[gravesti]

Yes, I could find the functions which generate the errors.

build_reference_index() calls data_reference_index() which has the logic we don't want to reimplement to process everything and it calls a fairly trivial check_missing_topics() which generates the actual warning.

For vignettes, there's just build_articles_index() which calls data_articles_index() which does the processing and gives the warning.

The build_*_index() functions have the call to render_page() which is where the Pandoc stuff comes in. The data_*_index() functions would be good to use I think but are not exported.

One idea I had is to define a dummy render_page(), but would allow us to use the build_*_index() functions as-is.
https://github.com/r-lib/pkgdown/blob/f85cfa24fbf0c809c4ca9dbd23a53742165367cb/R/build-reference.R#L243

[lorenzwalthert]

Do you need assistance in proceeding? I think your approach makes sense. We can also monky patch our dummy render_page() into build_*_index(), e.g. with {mockr} or {mockery}.

[gravesti]

Good idea. I submitted an issue and pull request to pkgdown but didn't get a response. I wasn't aware of those mock packages. I'll prepare something today.

[gravesti]

I'm wondering about the tests for this hook. I think it will need a minimal package folder structure.
Is the idea to put all the files in tests/test_that/reference and pass them to run_test artifacts?

[lorenzwalthert]

Should be added to the template for packages.

[lorenzwalthert]

Done.