posit-dev / positron

Positron, a next-generation data science IDE
Other
2.56k stars 80 forks source link

R: Messages coming from a cli subprocess are red #2928

Open DavisVaughan opened 5 months ago

DavisVaughan commented 5 months ago

They should not be red if our global calling handler is working right

Screenshot 2024-04-29 at 4 56 22 PM
rlang::inform("hi")
callr::r(function() { rlang::inform("hi") }, show = TRUE)

Like pkgdown::build_site()

DavisVaughan commented 5 months ago
> rlang:::default_message_file
function () 
{
    opt <- peek_option("rlang:::message_file")
    if (!is_null(opt)) {
        return(opt)
    }
    if ((is_interactive() || is_rstudio()) && sink.number("output") == 
        0 && sink.number("message") == 2) {
        stdout()
    }
    else {
        stderr()
    }
}
> rlang:::is_rstudio
function () 
{
    Sys.getenv("RSTUDIO_SESSION_PID") %in% c(Sys.getpid(), getppid())
}

And indeed if you set Sys.setenv(RSTUDIO_SESSION_PID = Sys.getpid()) in the parent process, then calls to pkgdown::build_site() show less red coloring (notably Reading from is no longer red) due to callr inheriting that env var and it matching getppid() (the parent process id, i.e. only looking 1 process up).

callr captures that stdout/stderr (depending on if the envvar is set) and streams it back up to the parent process, but I don't think it is emitted as an R "message" because it doesn't go through our global calling handler (I tried a breakpoint in there). You can see the difference in output channels in Positron:

x <- callr::r_process$new(callr::r_process_options(func = function() rlang::inform("hi")))
x$read_error_lines()
x$read_output_lines()

Sys.setenv(RSTUDIO_SESSION_PID = Sys.getpid())

x <- callr::r_process$new(callr::r_process_options(func = function() rlang::inform("hi")))
x$read_error_lines()
x$read_output_lines()
Screenshot 2024-04-29 at 5 20 20 PM
DavisVaughan commented 5 months ago

There is some additional weirdness in both RStudio and Positron - here is RStudio

Screenshot 2024-04-29 at 5 28 48 PM

One thing of note in my image above is that Building pkgdown site is red in both RStudio and Positron - that goes through cli::cli_rule() and i imagine that does not use rlang::inform(), unlike Reading from, so it probably ultimately emits to stderr in the subprocess https://github.com/r-lib/pkgdown/blob/5f9606b54aedbb36ade5a1f9db4c2780ceaf4e56/R/build.R#L437-L438

The reason why the Installing package rule at the top is white in both rstudio and positron is because it runs in the main process, not the subprocess, so positron's global calling handler runs there and captures it, and idk what happens in rstudio, but i guess something intelligent (or maybe it has to do with the fact that cli messages don't inherit from the "message" class, see cli:::cli__message_create()) https://github.com/r-lib/pkgdown/blob/5f9606b54aedbb36ade5a1f9db4c2780ceaf4e56/R/build.R#L347

DavisVaughan commented 5 months ago

So two tractable issues are: