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
.Rbuildignore can contain empty lines #570
Conversation
R/utils.R
Outdated
@@ -113,7 +113,7 @@ ignore_files <- function(rfiles, path) { | |||
rfiles_relative <- sub("^[/]*", "", rfiles_relative) | |||
|
|||
# Remove any files that match any perl-compatible regexp | |||
patterns <- readLines(rbuildignore, warn = FALSE) | |||
patterns <- scan(rbuildignore, what = "", sep = "\n", blank.lines.skip = TRUE, quiet = TRUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than switching to scan()
, which is extremely complex, could you please just filter ""
out of patterns?
tests/testthat/test-Rbuildignore.R
Outdated
@@ -7,6 +7,6 @@ test_that("roxygen ignores files with matching pattern in .Rbuildignore", { | |||
expect_equal(basename(package_files(test_pkg)), c("a.R", "ignore_me.R")) | |||
|
|||
#writeLines("^R/ignore_me.R$", file.path(test_pkg, ".Rbuildignore")) | |||
writeChar("^R/ignore_me.R$\n", file.path(test_pkg, ".Rbuildignore"), eos = NULL) | |||
writeChar("^R/ignore_me.R$\n\n.nonexistentfile", file.path(test_pkg, ".Rbuildignore"), eos = NULL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make this a separate test with a clear name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A separate test file or is a separate test_that enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test = test_that block
Looks good - can you please add a bullet to NEWS? |
Done. |
NEWS.md
Outdated
@@ -2,6 +2,7 @@ | |||
|
|||
* Automatically generating a usage section for an infix function containing "<-" | |||
no longer removes "<-" from the function name (#554). | |||
* Allowing empty lines in .Rbuildignore. Previously, empty lines caused all files being ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a blank line before and add your github username. And mention issue #572 which this PR fixes
Hopefully everything is fine now 😉 |
Thanks! |
Doesn't this bug warrant an urgent submission of roxygen2 6.0.1 to CRAN? |
Empty lines in the .Rbuildignore file caused roxygen2 6.0 to ignore all files because
""
matches everything.I also extended the test to cover that case.