Do not force cli to refresh the display

[DavisVaughan]

Closes #167

Instead, rely on cli's own internal timing and trust that it knows when to refresh the display at regular intervals.

forceing a refresh results in WAY slower progress bars, because it forces cli to do a whole bunch of work at each iteration. In particular, it calls cli:::inline_transformer() via cli:::glue() WAY more often, which is what does the inline interpolation in cli. As shown in #167 (comment), this takes a huge amount of time.

Running the original example from #167

f <- function(R = 1000) {
  pb <- progressr::progressor(steps = R)
  onerun <- function(...) {
    pb()
    Sys.sleep(0.001)
  }
  system.time(lapply(seq_len(R), onerun))
}

progressr::handlers(global = TRUE)

progressr::handlers(progressr::handler_void)
f() |> print()
#>   user  system elapsed 
#>  0.155   0.011   1.427 

progressr::handlers(progressr::handler_progress)
f() |> print()
#>   user  system elapsed 
#>  0.401   0.019   1.672 

progressr::handlers(progressr::handler_cli)
f() |> print()
#>   user  system elapsed 
#>  0.234   0.010   1.498 

Cranked up to 10k iterations

f <- function(R = 10000) {
  pb <- progressr::progressor(steps = R)
  onerun <- function(...) {
    pb()
    Sys.sleep(0.001)
  }
  system.time(lapply(seq_len(R), onerun))
}

progressr::handlers(global = TRUE)

progressr::handlers(progressr::handler_void)
f() |> print()
#>   user  system elapsed 
#>  2.017   0.138  14.910 

progressr::handlers(progressr::handler_progress)
f() |> print()
#>   user  system elapsed 
#>  4.663   0.424  17.605 

progressr::handlers(progressr::handler_cli)
f() |> print()
#>   user  system elapsed 
#>  3.081   0.383  16.065 

[HenrikBengtsson]

Great catch. This was introduced in 01eb29a in order to support cli reporting progress via progressr, e.g.

library(cli)

options(cli.progress_handlers = "progressr")

clean <- function() {
  cli_progress_bar("Cleaning data", total = 100)
  for (i in 1:100) {
    Sys.sleep(5/100)
    cli_progress_update()
  }
  cli_progress_done()
}

clean()

I just checked, and it looks like it still works without force = TRUE. I don't recall why I added that in that commit - maybe a "just-in-case" addition.