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

Add --no-update flag to spell-check #511

Merged
merged 13 commits into from
Feb 14, 2024
23 changes: 15 additions & 8 deletions R/testing.R
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@ run_test <- function(hook_name,
#' @param expect_success Whether or not an exit code 0 is expected. This can
#' be derived from `std_err`, but sometimes, non-empty stderr does not mean
#' error, but just a message.
#' @param read_only If `TRUE` and `artifacts` are not `NULL`, then assert that hook
#' did not modify the artifacts.
#' @param read_only If `TRUE`, then assert that no new files were created.
#' Additionally, if `artifacts` are not `NULL`, then assert that hook did not
#' modify the artifacts.
#' @keywords internal
run_test_impl <- function(path_executable,
path_candidate,
Expand All @@ -112,6 +113,7 @@ run_test_impl <- function(path_executable,
)
path_stderr <- tempfile()
path_stdout <- tempfile()
files_before_hook <- fs::dir_ls(tempdir, all = TRUE, recurse = TRUE)
exit_status <- hook_state_create(
tempdir,
path_candidate_temp,
Expand All @@ -133,12 +135,17 @@ run_test_impl <- function(path_executable,
std_out,
exit_status
)
if (isTRUE(read_only) && !is.null(artifacts)) {
purrr::iwalk(artifacts, function(reference_path, temp_path) {
artifact_before_hook <- readLines(testthat::test_path(reference_path))
artifact_after_hook <- readLines(fs::path_join(c(tempdir, temp_path)))
testthat::expect_equal(artifact_before_hook, artifact_after_hook)
})
if (isTRUE(read_only)) {
files_after_hook <- fs::dir_ls(tempdir, all = TRUE, recurse = TRUE)
testthat::expect_equal(files_before_hook, files_after_hook)

if (!is.null(artifacts)) {
purrr::iwalk(artifacts, function(reference_path, temp_path) {
artifact_before_hook <- readLines(reference_path)
artifact_after_hook <- readLines(fs::path_join(c(tempdir, temp_path)))
testthat::expect_equal(artifact_before_hook, artifact_after_hook)
})
}
}
}

Expand Down
51 changes: 28 additions & 23 deletions inst/hooks/exported/spell-check.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

"Spell check for files
Usage:
spell-check [--lang=<language>] <files>...
spell-check [--lang=<language>] [--read-only] <files>...

Options:
--lang=<language> Passed to `spelling::spell_check_files()` [default: en_US]
--read-only Don't update inst/WORDLIST with errors (new words)

" -> doc

Expand All @@ -16,10 +17,12 @@ if (file.exists(path_wordlist)) {
ignore <- readLines(path_wordlist, encoding = "UTF-8")
action <- "update"
} else {
if (!dir.exists(dirname(path_wordlist))) {
dir.create(dirname(path_wordlist))
if (isFALSE(arguments$read_only)) {
if (!dir.exists(dirname(path_wordlist))) {
dir.create(dirname(path_wordlist))
}
file.create(path_wordlist)
}
file.create(path_wordlist)
ignore <- character()
action <- "create"
}
Expand All @@ -34,24 +37,26 @@ spelling_errors <- spelling::spell_check_files(
if (nrow(spelling_errors) > 0) {
cat("The following spelling errors were found:\n")
print(spelling_errors)
ignore_df <- data.frame(
original = unique(c(ignore, spelling_errors$word))
)
ignore_df$lower <- tolower(ignore_df$original)
ignore_df <- ignore_df[order(ignore_df$lower, method = "radix"), ]
ignore <- ignore_df$original[ignore_df$lower != ""] # drop blanks if any
writeLines(ignore, path_wordlist)
cat(
"All spelling errors found were copied to inst/WORDLIST assuming they were",
"not spelling errors and will be ignored in the future. Please ",
"review the above list and for each word that is an actual typo:\n",
"- fix it in the source code.\n",
"- remove it again manually from inst/WORDLIST to make sure it's not\n",
" ignored in the future.\n",
"Then, try committing again.\n"
)
if (isTRUE(arguments$read_only)) {
cat("Hint: you can enable an automatic updating of WORDLIST with errors (new words) by removing the --read-only flag.\n")
} else {
ignore_df <- data.frame(
original = unique(c(ignore, spelling_errors$word))
)
ignore_df$lower <- tolower(ignore_df$original)
ignore_df <- ignore_df[order(ignore_df$lower, method = "radix"), ]
ignore <- ignore_df$original[ignore_df$lower != ""] # drop blanks if any
writeLines(ignore, path_wordlist)
cat(
"All spelling errors found were copied to inst/WORDLIST assuming they were",
"not spelling errors and will be ignored in the future. Please",
"review the above list and for each word that is an actual typo:\n",
"- fix it in the source code.\n",
"- remove it again manually from inst/WORDLIST to make sure it's not\n",
" ignored in the future.\n",
"Then, try committing again.\n",
"Hint: you can disable this behavior by providing a --read-only flag.\n"
)
}
stop("Spell check failed", call. = FALSE)
} else {
ignore <- ignore[ignore != ""] # drop blanks if any
writeLines(ignore, path_wordlist)
}
5 changes: 3 additions & 2 deletions man/run_test.Rd

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

5 changes: 3 additions & 2 deletions man/run_test_impl.Rd

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

3 changes: 3 additions & 0 deletions tests/testthat/in/spell-check-fail-2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# a ttle

This is a fsssile.
20 changes: 18 additions & 2 deletions tests/testthat/test-hooks.R
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,24 @@ run_test(
# success
run_test("spell-check", suffix = "-success.md", std_err = NULL)

# basic failure
run_test("spell-check", suffix = "-fail.md", std_err = "Spell check failed")
# failure with --read-only does not create WORDLIST
run_test(
"spell-check",
suffix = "-fail.md",
std_err = "Spell check failed",
cmd_args = "--read-only",
read_only = TRUE
)

# failure with --read-only does not update WORDLIST
run_test(
"spell-check",
suffix = "-fail-2.md",
std_err = "Spell check failed",
cmd_args = "--read-only",
artifacts = c("inst/WORDLIST" = test_path("in/WORDLIST")),
read_only = TRUE
)

# success with wordlist
run_test("spell-check",
Expand Down
10 changes: 10 additions & 0 deletions vignettes/available-hooks.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,14 @@ The `lang` arg will be passed to `spelling::spell_check_files()`.
id: spell-check
args: [--lang=<language>]

**read only**

The `--read-only` flag will be passed to spell check. This flag makes
this hook idempotent.

id: spell-check
args: [--read-only]

This hook does not modify input files. It will add all words not found
in the dictionary to `inst/WORDLIST`, assuming they were spelled
correctly but were not in the dictionary. An example might be "RStudio".
Expand All @@ -205,6 +213,8 @@ them and remove them from `inst/WORDLIST`. If there were not typos, or
you fixed all, stage `inst/WORDLIST` and this time, the commit should
pass.

To opt out of updating `inst/WORDLIST` provide the `--read-only` flag.

## `roxygenize`

A hook to run `roxygen2::roxygenize()`. Makes sure you commit your `.Rd`
Expand Down
2 changes: 1 addition & 1 deletion vignettes/hook-order.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Read only hooks should generally run only after write hooks.
- roxygenize - caches
- codemeta-description-updated - must be before use-tidy-description
- use-tidy-description
- spell-check - updates `inst/WORDLIST`; should run after roxygenize
- spell-check - updates `inst/WORDLIST` (unless `--read-only` arg is specified); should run after roxygenize

### Read only

Expand Down
Loading