Skip to content

Simplify find config #2205

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 21, 2023
Merged

Simplify find config #2205

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3c2207d
simplify finding local settings
MichaelChirico Sep 21, 2023
f0aa186
missed conflict
MichaelChirico Sep 21, 2023
0960758
shared logic in helper
MichaelChirico Sep 21, 2023
1689bce
Merge branch 'main' into simplify-find-config
AshesITR Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 28 additions & 26 deletions R/settings_utils.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,21 @@ is_root <- function(path) {
identical(path, dirname(path))
}

is_directory <- function(filename) {
is_dir <- file.info(filename)$isdir

!is.na(is_dir) && is_dir
}

has_config <- function(path, config) {
file.exists(file.path(path, config))
is_directory <- function(filename) isTRUE(file.info(filename)$isdir)

#' Return the first of a vector of files that exists.
#'
#' Avoid running 'expensive' [file.exists()] for the full vector,
#' since typically the first entries will lead to early exit.
#' TODO(#2204): check if the implementation should be simpler
#' @noRd
first_exists <- function(files) {
for (file in files) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be avoided because file.exists() is vectorized.
Not sure if faster, we should check:

file_exists <- file.exists(files)
if (any(file_exists)) {
  files[which.max(file_exists)]
} else {
  NULL
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's the #2204 TODO -- I wanted to punt on that for now :)

if (file.exists(file)) {
return(file)
}
}
NULL
}

find_config <- function(filename) {
Expand All @@ -61,44 +68,39 @@ find_config <- function(filename) {
dirname(filename)
}

path <- normalizePath(path, mustWork = FALSE)

# NB: This vector specifies a priority order for where to find the configs,
# i.e. the first location where a config exists is chosen and configs which
# may exist in subsequent directories are ignored
file_locations <- c(
# Local (incl. parent) directories
find_config2(path),
find_local_config(path, basename(linter_file)),
# User directory
# cf: rstudio@bc9b6a5 SessionRSConnect.R#L32
file.path(Sys.getenv("HOME", unset = "~"), linter_file),
# Next check for a global config file
file.path(R_user_dir("lintr", which = "config"), "config")
)

# Search through locations, return first valid result
for (loc in file_locations) {
if (file.exists(loc)) {
return(loc)
}
}

NULL
first_exists(file_locations)
}

find_config2 <- function(path) {
config <- basename(getOption("lintr.linter_file"))
path <- normalizePath(path, mustWork = FALSE)

while (!has_config(path, config)) {
gh <- file.path(path, ".github", "linters")
if (has_config(gh, config)) {
return(file.path(gh, config))
find_local_config <- function(path, config_file) {
repeat {
guesses_in_dir <- c(
file.path(path, config_file),
file.path(path, ".github", "linters", config_file)
)
found <- first_exists(guesses_in_dir)
if (!is.null(found)) {
return(found)
}
path <- dirname(path)
if (is_root(path)) {
return(character())
}
}
return(file.path(path, config))
}

pkg_name <- function(path = find_package()) {
Expand Down