Skip to content

Commit

Permalink
Allow feedback on unparsable unicode characters to render without `i1…
Browse files Browse the repository at this point in the history
…8next` (#765)

* fix(unparsable_unicode_message): pre-interpolate message so feedback is comprehensible when `i18next` is unavailable

* docs: update NEWS

* docs: update documentation

* refactor(unparsable_unicode_message): use `sub()` rather than `glue()` to handle interpolation
  • Loading branch information
rossellhayes authored Mar 9, 2023
1 parent 86b53df commit 7648a1f
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 49 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,5 @@ Config/Needs/coverage: covr
Config/Needs/website: pkgdown, tidyverse/tidytemplate
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.1
RoxygenNote: 7.2.3
SystemRequirements: pandoc (>= 1.14) - http://pandoc.org
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

- The embedded Ace editor used in learnr exercises now defaults to a tab width of 2, aligning with the Tidyverse style guide (#761).

- Add a fallback to generate a comprehensible English feedback message for code that fails to parse because it contains non-ASCII characters. Previously, if i18next was unavailable, the feedback would contain uninterpolated i18next markup. Now the feedback is pre-interpolated so students will always see a comprehensible message (#765).

# learnr 0.11.2

- Fixed an issue that prevented htmlwidgets from working in exercise code unless similar widgets were added to the tutorial prose (thanks @munoztd0 #744, #745).
Expand Down
22 changes: 17 additions & 5 deletions R/exercise.R
Original file line number Diff line number Diff line change
Expand Up @@ -1039,20 +1039,32 @@ exercise_check_unparsable_unicode <- function(exercise, error_message) {
)
}

unparsable_unicode_message <- function(i18n_key, code, line, pattern, replacement_pattern = NULL) {
unparsable_unicode_message <- function(
i18n_key, code, line, pattern, replacement_pattern = NULL
) {
code <- unlist(strsplit(code, "\n"))[[line]]

character <- str_extract(code, pattern)
highlighted_code <- exercise_highlight_unparsable_unicode(code, pattern, line)

suggestion <- NULL
if (!is.null(replacement_pattern)) {
suggestion <- html_code_block(str_replace_all(code, replacement_pattern))
suggestion <- if (!is.null(replacement_pattern)) {
html_code_block(str_replace_all(code, replacement_pattern))
} else {
NULL
}

code <- exercise_highlight_unparsable_unicode(code, pattern, line)

text <- i18n_translations()$en$translation$text[[i18n_key]]
text <- sub("{{character}}", character, text, fixed = TRUE)
text <- sub("{{code}}", highlighted_code, text, fixed = TRUE)
if (!is.null(suggestion)) {
text <- sub("{{suggestion}}", suggestion, text, fixed = TRUE)
}

i18n_div(
paste0("text.", i18n_key),
HTML(i18n_translations()$en$translation$text[[i18n_key]]),
HTML(text),
opts = list(
character = character,
code = highlighted_code,
Expand Down
12 changes: 6 additions & 6 deletions man/learnr-package.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/tutorial.Rd

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

62 changes: 26 additions & 36 deletions tests/testthat/test-exercise.R
Original file line number Diff line number Diff line change
Expand Up @@ -1171,52 +1171,42 @@ test_that("Errors with global setup code result in an internal error", {
test_that("evaluate_exercise() returns message for unparsable non-ASCII code", {
skip_if_not_pandoc("1.14")

expect_unparsable_message <- function(user_code, problem, key) {
ex <- mock_exercise(user_code = user_code)
feedback <- evaluate_exercise(ex, new.env())$feedback
expect_equal(feedback, exercise_check_code_is_parsable(ex)$feedback)
expect_match(feedback$message, regexp = key, fixed = TRUE)
for (character in problem) {
expect_match(feedback$message, regexp = character, fixed = TRUE)
}
}

# Curly double quotes
ex <- mock_exercise(
user_code = "str_detect(\u201ctest\u201d, \u201ct.+t\u201d)"
)
result <- evaluate_exercise(ex, new.env())
expect_equal(result$feedback, exercise_check_code_is_parsable(ex)$feedback)
expect_match(result$feedback$message, "text.unparsablequotes")
expect_match(
result$feedback$message,
i18n_translations()$en$translation$text$unparsablequotes,
fixed = TRUE
expect_unparsable_message(
"str_detect(\u201ctest\u201d, \u201ct.+t\u201d)",
problem = c("\u201c", "\u201d"),
key = "text.unparsablequotes"
)

# Curly single quotes
ex <- mock_exercise(
user_code = "str_detect(\u2018test\u2019, \u2018t.+t\u2019)"
)
result <- evaluate_exercise(ex, new.env())
expect_equal(result$feedback, exercise_check_code_is_parsable(ex)$feedback)
expect_match(result$feedback$message, "text.unparsablequotes")
expect_match(
result$feedback$message,
i18n_translations()$en$translation$text$unparsablequotes,
fixed = TRUE
expect_unparsable_message(
"str_detect(\u2018test\u2019, \u2018t.+t\u2019)",
problem = c("\u2018", "\u2019"),
key = "text.unparsablequotes"
)

# En dash
ex <- mock_exercise(user_code = "63 \u2013 21")
result <- evaluate_exercise(ex, new.env())
expect_equal(result$feedback, exercise_check_code_is_parsable(ex)$feedback)
expect_match(result$feedback$message, "text.unparsableunicodesuggestion")
expect_match(
result$feedback$message,
i18n_translations()$en$translation$text$unparsableunicodesuggestion,
fixed = TRUE
expect_unparsable_message(
"63 \u2013 21",
problem = "\u2013",
key = "text.unparsableunicodesuggestion"
)

# Plus-minus sign
ex <- mock_exercise(user_code = "63 \u00b1 21")
result <- evaluate_exercise(ex, new.env())
expect_equal(result$feedback, exercise_check_code_is_parsable(ex)$feedback)
expect_match(result$feedback$message, "text.unparsableunicode")
expect_match(
result$feedback$message,
i18n_translations()$en$translation$text$unparsableunicode,
fixed = TRUE
expect_unparsable_message(
"63 \u00b1 21",
problem = "\u00b1",
key = "text.unparsableunicode"
)
})

Expand Down

0 comments on commit 7648a1f

Please sign in to comment.