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

Cran compliance #14

Merged
merged 9 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
1 change: 1 addition & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
^\.github$
^\.lintr$
.log
^cran-comments\.md$
2 changes: 1 addition & 1 deletion R/box_lsp.R
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ process_module <- function(sym_name, signature, action) {
#' @returns Used for side-effects provided by the `action` list of functions.
#'
#' @examples
#' \dontrun{
#' \donttest{
#' action <- list(
#' assign = function(symbol, value) {
#' cat(paste("ASSIGN: ", symbol, value, "\n"))
Expand Down
40 changes: 38 additions & 2 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#' @returns Writes configuration lines to `file_path`.
#'
#' @examples
#' \dontrun{
#' if (interactive()) {
#' use_box_lsp()
#' }
#'
Expand All @@ -15,11 +15,47 @@ use_box_lsp <- function(file_path = ".Rprofile") {

tryCatch({
if (file.exists(file_path)) {
response <- readline(
cli::cli(
cli::cli_alert(
c(
"`{file_path}` exists. Lines will be added to the end of this file. ",
"Would you like to continue (yes/No)"
),
wrap = TRUE
)
)
)
response <- substr(response, 1, 1)
if (!(response == "Y" || response == "y")) {
cli::cli_inform("`{file_path}` file append cancelled.")
return(invisible(NULL))
}

cli::cli_alert_info(
"{file_path} exists. Will append {{box.lsp}} configuration lines to the file."
"Appending {{box.lsp}} configuration lines to the `{file_path}`."
)
write("", file = file_path, append = TRUE)
} else {
response <- readline(
cli::cli(
cli::cli_alert(
"Would you like to create `{file_path}` with {{box_lsp}} configuration? (yes/No)",
wrap = TRUE
)
)
)
response <- substr(response, 1, 1)
if (!(response == "Y" || response == "y")) {
cli::cli_inform("`{file_path}` file creation cancelled.")
return(invisible(NULL))
}

cli::cli_alert_info(
"Creating `{file_path}` with {{box.lsp}} configuration."
)
}

write(to_write, file = file_path, append = TRUE)
cli::cli_alert_success(
c(
Expand Down
33 changes: 33 additions & 0 deletions cran-comments.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# box.lsp 0.1.0 Resubmission

> If there are references describing the methods in your package, please add these in the description field of your DESCRIPTION file

There are no references to include regarding this package.

> \dontrun{} should only be used if the example really cannot be executed
(e.g. because of missing additional software, missing API keys, ...) by
the user. That's why wrapping examples in \dontrun{} adds the comment
("# Not run:") as a warning for the user. Does not seem necessary.
Please replace \dontrun with \donttest.
Functions which are supposed to only run interactively (e.g. shiny)
should be wrapped in if(interactive()). Please replace /dontrun{} with
if(interactive()){} if possible, then users can see that the functions
are not intended for use in scripts / functions that are supposed to run
non interactively.

`\dontrun` replaced with `if(interactive)` in `box.lsp::use_box_lsp` and `\donttest` in `box.lsp::box_use_parser`.

> Please ensure that your functions do not write by default or in your
examples/vignettes/tests in the user's home filespace (including the
package directory and getwd()). This is not allowed by CRAN policies.
Please omit any default path in writing functions. In your
examples/vignettes/tests you can write to tempdir().
-> R/utils.R and your tests

All examples that could write in the user's filespace are blocked by using `if(interactive())`. Tests are using `withr::temp*` functions to prevent that. `box.lsp::use_box_lsp` still has the default path set, but to prevent from writing anything by accident it now requires a confirmation from the user (with default set to No).

## R CMD check results

0 errors | 0 warnings | 1 note
Copy link
Collaborator Author

@radbasa radbasa Aug 28, 2024

Choose a reason for hiding this comment

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

Would 1 note be an issue?


* This is a new release.
2 changes: 1 addition & 1 deletion man/box_use_parser.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/use_box_lsp.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tests/testthat/test-lsp_aaa_regression.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
test_that("Completion of attached package functions works", {
skip_on_cran()
testthat::skip_on_cran()
client <- language_client()

temp_file <- withr::local_tempfile(fileext = ".R")
Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-lsp_completion_package_attach_list.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test_that("completion of package attached function list works", {
testthat::skip_on_cran()
client <- language_client()

temp_file <- withr::local_tempfile(fileext = ".R")
Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-lsp_completion_whole_package.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test_that("completion of whole package attached works", {
testthat::skip_on_cran()
skip("SKIP. Not implemented.")
client <- language_client()

Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-signature.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test_that("signature works with box-attached functions", {
testthat::skip_on_cran()
skip("SKIP. Erratic, inconsistent.")
client <- language_client()

Expand Down