Use {cli} progress bar (#2641)

* Use cli progress bar: first draft * update NEWS * Update NEWS.md * tweak * note experimental status in a comment * respect `show_progress`'s authoritah * fix warning * nolint --------- Co-authored-by: Michael Chirico <[email protected]>
r-lib · Dec 3, 2024 · 13c7682 · 13c7682
1 parent 0103bad
commit 13c7682
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 30 deletions.
diff --git a/NAMESPACE b/NAMESPACE
@@ -181,13 +181,10 @@ importFrom(stats,na.omit)
 importFrom(tools,R_user_dir)
 importFrom(utils,capture.output)
 importFrom(utils,getParseData)
-importFrom(utils,getTxtProgressBar)
 importFrom(utils,globalVariables)
 importFrom(utils,head)
 importFrom(utils,relist)
-importFrom(utils,setTxtProgressBar)
 importFrom(utils,tail)
-importFrom(utils,txtProgressBar)
 importFrom(xml2,as_list)
 importFrom(xml2,xml_attr)
 importFrom(xml2,xml_children)

diff --git a/NEWS.md b/NEWS.md
@@ -91,6 +91,7 @@
 
 ## Notes
 
+* All user-facing messages (including progress bars) are now prepared using the `{cli}` package (#2418 and #2641, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent.
 * All user-facing messages are now prepared using the `{cli}` package (#2418, @IndrajeetPatil). All messages have been reviewed and updated to be more informative and consistent.
 * File locations in lints and error messages contain clickable hyperlinks to improve code navigation (#2645, #2588, @olivroy).
 * {lintr} now depends on R version 4.0.0. It already does so implicitly due to recursive upstream dependencies requiring this version; we've simply made that dependency explicit and up-front (#2569, @MichaelChirico).

diff --git a/R/lint.R b/R/lint.R
@@ -182,20 +182,24 @@ lint_dir <- function(path = ".", ...,
     return(lints)
   }
 
-  pb <- if (isTRUE(show_progress)) {
-    txtProgressBar(max = length(files), style = 3L)
+  if (isTRUE(show_progress)) {
+    lints <- lapply(
+      # NB: This cli API is experimental (https://github.com/r-lib/cli/issues/709)
+      cli::cli_progress_along(files, name = "Running linters"),
+      function(idx) {
+        lint(files[idx], ..., parse_settings = FALSE, exclusions = exclusions)
+      }
+    )
+  } else {
+    lints <- lapply(
+      files,
+      function(file) { # nolint: unnecessary_lambda_linter.
+        lint(file, ..., parse_settings = FALSE, exclusions = exclusions)
+      }
+    )
   }
 
-  lints <- flatten_lints(lapply(
-    files,
-    function(file) {
-      maybe_report_progress(pb)
-      lint(file, ..., parse_settings = FALSE, exclusions = exclusions)
-    }
-  ))
-
-  if (!is.null(pb)) close(pb)
-
+  lints <- flatten_lints(lints)
   lints <- reorder_lints(lints)
 
   if (relative_path) {
@@ -688,13 +692,6 @@ has_positional_logical <- function(dots) {
     !nzchar(names2(dots)[1L])
 }
 
-maybe_report_progress <- function(pb) {
-  if (is.null(pb)) {
-    return(invisible())
-  }
-  setTxtProgressBar(pb, getTxtProgressBar(pb) + 1L)
-}
-
 maybe_append_error_lint <- function(lints, error, lint_cache, filename) {
   if (is_lint(error)) {
     error$linter <- "error"

diff --git a/R/lintr-package.R b/R/lintr-package.R
@@ -13,8 +13,7 @@
 #' @importFrom rex rex regex re_matches re_substitutes character_class
 #' @importFrom stats complete.cases na.omit
 #' @importFrom tools R_user_dir
-#' @importFrom utils capture.output getParseData getTxtProgressBar globalVariables head relist
-#'   setTxtProgressBar tail txtProgressBar
+#' @importFrom utils capture.output getParseData  globalVariables head relist tail
 #' @importFrom xml2 as_list
 #'   xml_attr xml_children xml_find_all xml_find_chr xml_find_lgl xml_find_num xml_find_first xml_name xml_text
 ## lintr namespace: end

diff --git a/tests/testthat/test-lint_dir.R b/tests/testthat/test-lint_dir.R
@@ -10,12 +10,6 @@ test_that("lint all files in a directory", {
 
   expect_s3_class(lints, "lints")
   expect_identical(sort(linted_files), sort(files))
-
-  expect_output(
-    lint_dir(the_dir, parse_settings = FALSE, show_progress = TRUE),
-    "======",
-    fixed = TRUE
-  )
 })
 
 test_that("lint all relevant directories in a package", {