pharmaR / riskmetric

Metrics to evaluate the risk of R packages
https://pharmar.github.io/riskmetric/
Other
159 stars 30 forks source link

has_news metric does not find "NEWS" files #349

Closed bertcarnell closed 1 day ago

bertcarnell commented 1 month ago

The riskmetric:::news_from_dir function does not find a NEWS file without an extension.

Reproducible example:

install.packages(c("riskmetric", "lhs"))
# works
list.files(file.path(.libPaths()[1], "riskmetric"), pattern = "^NEWS\\.", full.names=TRUE)
# fails
list.files(file.path(.libPaths()[1], "lhs"), pattern = "^NEWS\\.", full.names=TRUE)

# works for both
list.files(file.path(.libPaths()[1], "riskmetric"), pattern = "^NEWS[.]*", full.names=TRUE)
list.files(file.path(.libPaths()[1], "lhs"), pattern = "^NEWS[.]*", full.names=TRUE)

The reason that lhs failes and riskmetric works is that riskmetric includes a "NEWS.md" while lhs only includes a "NEWS".

news_from_dir <- function(path) {
  files <- list.files(path, pattern = "^NEWS\\.", full.names = TRUE)
  # should be changed to this to handle NEWS.Rd, NEWS.md, and NEWS
  files <- list.files(path, pattern = "^NEWS[.]*", full.names = TRUE)
  ...
}
csgillespie commented 1 month ago

I would probably go a little more conservative with

list.files(path, pattern = "^NEWS($|\\.)", full.names = TRUE)

Also, the code below will need changed. Something like (note the change in the else if

    f = files[[i]]
    tryCatch(
      {
        ext = tolower(tools::file_ext(f))
        if (ext == "rd") {
          content[[i]] = .tools$.news_reader_default(f)
        } else if (ext == "md" || nchar(ext) == 0L) {
          # NOTE: should we do validation of markdown format?
          content[[i]] = readLines(f, warn = FALSE)
        }
        valid[[i]] = TRUE
      },
      error = function(e) {
        valid[[i]] = FALSE
      }
    )
  }