docs: Use rlang::check_installed() internally to install missing suggested packages on the fly

[olivroy]

Close #2039 . (possibly enough for #1735 and #1348 imo)
follow-up to #2036

Use only loop in testing.

Also, for dm_gui(), handle suggested at once will improve the UX significantly.

# Now
> dm_gui(dm = dm)
ℹ The packages "colourpicker", "shinyAce", and "shinydashboard" are required to use `dm_gui()`.
✖ Would you like to install them?

1: Yes
2: No

I also return TRUE early if all packages are installed. Is that correct?

[krlmlr]

Thank you for your help. Can you please split this PR into two?

https://code-review.tidyverse.org/author/focused.html

[krlmlr]

Locally:

git checkout main
git pull
git checkout -b b-fix-ci
git cherry-pick suggests-follow

This cherry-picks the last commit on top of the main branch, this is the one I'd like to merge first. Then, open a new pull request.

After that:

git checkout suggests-follow
git reset --hard HEAD^

This erases the last commit.

For more complex splitting, interactive rebase is helpful.

[olivroy]

From my testing, I verified #1348. Indeed, check_suggested() can work with multiple packages.

[krlmlr]

Thanks for following up! I think we might be able to get rid of the for loops entirely.

[olivroy]

There may be some code duplication, but this seems to work as intended now. getting rid of version, and message argument.

I removed the install.packages() bit because some version may be attached to it, which would create invalid code.

I just added a test.

[krlmlr]

Thanks, cool!

b6f5382 adds a lot of noise here, can you please send this as a separate PR?

[krlmlr]

Suggested change

	installed <- map_lgl(packages, function(pkg) {
	is_installed(pkg)
	})
	installed <- map_lgl(packages, is_installed)

Can this move even higher, outside if (!use) ? We could return immediately if all packages are installed, in all cases.

[krlmlr]

Suggested change

	installed <- map_lgl(packages, function(pkg) {
	is_installed(pkg)
	})

Redundant now?

[krlmlr]

Suggested change

	# Return early if all packages are installed.
	if (rlang::is_installed(packages)) {
	return(TRUE)
	}

Not needed if we do an early return?

[olivroy]

I tried playing with the order without chaning test results, but it didn't work.

[olivroy]

Possibly in a follow-up because this PR aims not to change the default behavior. (except gor removing the version and message argument from check_suggested

[olivroy]

Reverted b6f5382 + added cli pluralization. Not sure about changing the order as it may result in different results, depending on which packages are installed on the system.

[krlmlr]

I force-pushed, if needed you can use

git fetch
git reset --hard origin/suggests-follow

to sync locally.

Thanks for recognizing a real shortcoming in our approach to handling dependencies and for pushing through!

[olivroy]

Glad you like it and that you validated what I did. Always enriching. Tests still pass locally for me after changes you made.

[krlmlr]

Looking into ?check_installed, I wonder if we could make do with one single try_catch(check_installed(...), ...) call. I suspect this won't make the code easier to understand, to debug, or to enhance -- I suggest calling it a pull request and see how this works in practice before changing further.

[olivroy]

Looking into ?check_installed, I wonder if we could make do with one single try_catch(check_installed(...), ...) call. I suspect this won't make the code easier to understand, to debug, or to enhance -- I suggest calling it a pull request and see how this works in practice before changing further.

check_suggest <- function(pkg, call = rlang::caller_env()) {
  rlang::local_interactive(FALSE) # To avoid the prompt
  
  rlang::try_fetch(
    rlang::check_installed(pkg, reason = "to have an enhanced functionality.", call = call),
    error = function(e) {rlang::inform("The function is enhanced by the following package, ", parent = e, call = call)})
  invisible()
}
g <- function(x) {
  check_suggest(c("ggplot2 (>= 3.5.0)", "rsvg", "sss"))
  x**2
}
g(2)
#> The function is enhanced by the following package, 
#> Caused by error in `g()`:
#> ! The packages "ggplot2" (>= 3.5.0) and "sss" are required to have an enhanced functionality.
[1] 4

An example I shared earlier that could be what you were thinking about? But as I pointed out, the message shared by check_installed() is not 100% correct for the suggested enhancements.

[krlmlr]

Thanks for the example, I missed that. The condition object e has elements that contain the packages and versions that were missing, so in theory we could put together the same message, but I don't think it's worth the effort at this time.

[krlmlr]

Thanks!