documentation only loaded for packages named in a library() call, not with p_load()

[D3SL]

This might be the languageserver package itself, but I only get on-hover documentation for commands from packages loaded using library() rather than pacman::p_load().

[renkun-ken]

Currently, we only capture explicit calls of library() and require() as attached packages for languageserver to load to provide information.

There could be many ways to not use library() or require() to attach a package such as

load_pkgs <- function() {
  library(pkg1)
  library(pkg2)
}

we don't handle such cases at the moment.

[renkun-ken]

It looks like pacman::p_load() calls pacman::p_load_single() for each package, which ultimately calls suppressMessages(require(package, character.only = TRUE)).

In fact, there's a good use case of nested library() or require() calls such as

suppressPackageStartupMessages({
  library(data.table)
  library(xml2)
})

I'll think about this.

[chunyunma]

For what it's worth, here is my use case.
I also don't use library() in my script.
Instead, I primarily use the import package and
reference package explicitly either by abbreviations like this:

dp <- import::from(dplyr, .all=TRUE, .into={new.env()})
dp$select(head(cars),dist)

Or, if I only need to use a function once or twice in a project,
I just write package::function().

With the first scenario, I cannot get any auto-complete or on-hover documentation.
Despite this disadvantage,
I still prefer the import approach in my teaching
because students who are new to R can get easily confused as to which function belong to which package.
If I reference them explicitly, even with an abbreviation,
it helps reinforce learning.

I know it is a lot to ask,
but would be great to have lsp support for dp$select(...) type of usage.

[D3SL]

@chunyunma I've actually seen some posts by the RStudio team about this and library() is still the canonical way to do things because package::function() will still have all the loading time of library(), but now you don't have the explicit up front notice that your code is using that particular package.

Also as someone who's taught a lot of students who were very non-tech people R I would say the import method is in and of itself far more confusing than simply teaching students to read the line in the console that says "Function X from package Y has been masked by loading package Z". If just reading that line is too confusing for them then I can't see piling environments on top of everything helping. Especially since it's really just an overcomplicated way of doing package::function().

If you really want continue to teach something that anti-canonical I'd recommend at least using something like the Box package which is intended to work this way, rather than a homemade kludge. It's a more comprehensive solution and you'll be less likely to run into any headaches down the road because it was designed to work that way from the ground up.

[chunyunma]

@D3SL Thank you for introducing me to the Box package.
I could be wrong, but this package does not seem fundamentally different from import.
In fact, the dp <- import::from(...) example I cited previously was a direct quote from the official documentation of import package, rather than "a homemade kludge".
This thread might not be the best place to discuss this topic.
I'd be happy to continue the conversation over a private channel, say emails.

[randy3k]

There are actually a few related issues

1. doing library in a conditional statement

if (CONDITION) {
    library(PACKAGE)
}

Above loads the package because we are only search for the library keyword statically. It may not make sense if the condition is always FALSE. But however I argue that we shall not load the package even if CONDITION turns out to be true in runtime.

2. library with character.only = FALSE doesn't work

foo <- "PACKAGE"
library(foo, character.only = TRUE)

This will import the package foo instead.

[randy3k]

I guess one solution is to check some kind of annotations in the file. Something like [[Rcpp::export...]].

# [[languageserver::import(pkg1, pkg2)]]
pacman:p_load(pkg1)

or even use Roxygen tags directly

#' @importFrom R6 R6Class
#' @import xml2

[renkun-ken]

I file a PR #452 to support customizable unscoped functions (e.g. suppressPackageStartupMessages) and library functions (e.g. pacman::p_load). I think it is sufficient to meet most demand on this topic.

@D3SL @wdiao-zju Would you like to have try via

remotes::install_github("REditorSupport/languageserver#452")

[wdiao-zju]

It is awesome. but It only work when load single package, however, it can not work when load multiple packages using p_load. e.g. pacman::p_load(readr, ggplot2)

[renkun-ken]

It is awesome. but It only work when load single package, however, it can not work when load multiple packages using p_load. e.g. pacman::p_load(readr, ggplot2)

Didn't know it could do this. Let me find a way to make it work.

[TTTPOB]

great, this patch works for me when using suppressPackageStartUpMessages

[renkun-ken]

The specification of the unscoped functions and library functions are more flexible via 73f79d5.

An unscoped function is associated with a character vector of arguments to capture so that each argument is regarded to be evaluated in the current scope. An example is tryCatch = c("expr", "finally") where both expressions are evaluated in the current scope.

A library function specification now has the full flexibility to handle how to extract the package names given the call in the document. For example,

"pacman::p_load" = function(call) {
        fun <- if (requireNamespace("pacman")) pacman::p_load else
            function(..., char, install = TRUE,
                     update = getOption("pac_update"),
                     character.only = FALSE) NULL
        call <- match.call(fun, call, expand.dots = FALSE)
        if (!isTRUE(call$character.only)) {
            vapply(call[["..."]], as.character, character(1L))
        }
    }

Now it supports the following:

suppressPackageStartUpMessages({
  tryCatch(pacman::p_load(readr, ggplot2), error = function(e) stop("Error"))
})

[renkun-ken]

A bug that exists for quite a while in parse_expr prevents suppressPackageStartUpMessages(library(reader)) from working. Let me fix this.

[renkun-ken]

The bug is fixed and please test against the latest PR and let me know if there is anything not working properly.

[wdiao-zju]

Great work. I found a minor bug and it doesn't matter. pacman::p_load(p1, p2, p3) is good. But it not work when I first library(pacman) then p_load(p1, p2, p3)

[renkun-ken]

Great work. I found a minor bug and it doesn't matter. pacman::p_load(p1, p2, p3) is good. But it not work when I first library(pacman) then p_load(p1, p2, p3)

If you really need p_load() rather than pacman::p_load(), then you may write it in your ~/.Rprofile:

options(languageserver.library_functions = list(
  p_load = function(call) {
        fun <- if (requireNamespace("pacman")) pacman::p_load else
            function(..., char, install = TRUE,
                     update = getOption("pac_update"),
                     character.only = FALSE) NULL
        call <- match.call(fun, call, expand.dots = FALSE)
        if (!isTRUE(call$character.only)) {
            vapply(call[["..."]], as.character, character(1L))
        }
    }
)

[D3SL]

I wanted to test this out some but unfortunately I'm still having the issue with enormous floods of processes being opened and it makes VSCode unusable for me. Trying to do anything results in gigabytes of memory being eaten by dozens of spurious processes. It's forced me to go back to RStudio, which is unfortunate because I'm starting to really think their de facto monopoly on the entire R ecosystem is getting destructive as more and more packages become "opinionated" about doing things only according to their exact manual of style with their proprietary products.