Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use {cli} progress bar #2641

Merged
merged 12 commits into from
Dec 3, 2024
3 changes: 0 additions & 3 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
35 changes: 16 additions & 19 deletions R/lint.R
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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"
Expand Down
3 changes: 1 addition & 2 deletions R/lintr-package.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions tests/testthat/test-lint_dir.R
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down
Loading