find_external_resources works with custom format using theme

[cderv]

This is fixing #2493 in a quite scoped manner.

This PR account for error when resolve_theme() is used in finding resources step.
Some custom theme could use the theme configuration in other context that bslib theming detection. However, resolve_theme() is an internal function to detect if this is a bslib theme, and css should be copied as resource for rendering in another folder.

We consider that an error means theme is not what rmarkdown's format expect it to be (either a vector of string, or a non built-in boostrap 3 theme), this is indeed not a bslib and we should not copy CSS.

@yihui just adding you as reviewer, if you happen to pass around here. I'll merge it otherwise as I know you are travelling

[gadenbuie]

This seems like a good fix to me! It's unfortunate that resolve_theme() is tightly coupled with the themes provided by html_document() (i.e. Bootstrap themes) (I'd even recommend renaming it resolve_bs_theme() or resolve_base_theme(), but understand if that's out of scope).

That said, this seems like a reasonable compromise to tryCatch(resolve_theme()) in discover_rmd_resources(), since that use of resolve_theme() part of what's pushing the function out of its original purpose.

[cderv]

It's unfortunate that resolve_theme() is tightly coupled with the themes provided by html_document() (i.e. Bootstrap themes) (I'd even recommend renaming it resolve_bs_theme() or resolve_base_theme(), but understand if that's out of scope).

I am open to make adaption to this. It was introduce specifically for bslib support, but if it can be useful to different purpose we can make adaptation. I am curious of what you have in mind (is this for your theme handling in cleanrmd ?)

It is an internal function for now, so quite open to change IMO.

That said, this seems like a reasonable compromise to tryCatch(resolve_theme()) in discover_rmd_resources(), since that use of resolve_theme() part of what's pushing the function out of its original purpose.

That is exactly what I was thinking. Other idea was to add an argument in resolve_theme(..., check_supported = TRUE) than when set to FALSE not do the match.arg check that we added for more use friendly error when providing a non supported Bootstrap theme.

[gadenbuie]

I like both ideas! After reading more about how these functions are used I think:

1. resolve_theme() should be named resolve_bs_theme()
2. themes() should be named bs_themes()
3. resolve_theme() should have the check_supported argument you proposed to avoid failing in non-critical code paths, like discover_rmd_resources()

Those updates wouldn't change the behavior, but I do think it would make the code easier to read and understand in the future 😃

[cderv]

I'll go with this fix for now, and think more about renaming the function. I feel tryCatch here is correctly align with what we expect when using resolve_theme() in discover_rmd_resources context.