Allow feedback on unparsable unicode characters to render without i18next
#765
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…is comprehensible when `i18next` is unavailable
rossellhayes
added
the
type: bug
Yeah that is elegant but let's use the same method we used in the other functions and not add the new dependency |
…` to handle interpolation
|
Got it, refactored to use |
gadenbuie
approved these changes
Mar 9, 2023
Comment on lines
+1174
to
+1182
| 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) | ||
| } | ||
| } |
There was a problem hiding this comment.
Nice! This cleans up the code quite a bit
| @@ -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). | |||
There was a problem hiding this comment.
I think we can trim this down a little bit
Suggested change
| - 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 now pre-renders (in English) the feedback message it provides when non-ASCII characters are included in submitted unparsable R code. This makes the feedback useful in places where learnr's in-browser translations aren't available (#765). |
Closed
gadenbuie
added a commit
that referenced
this pull request
Mar 10, 2023
Applies suggestion from #765
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pre-interpolate message generated by
unparsable_unicode_message()so feedback is comprehensible wheni18nextis unavailable.Previously, if
i18nextwas unavailable, the feedback would contain uninterpolated strings, like this:This PR adds a dependency on
glue, which provides an elegant way to pre-interpolate the string, but isn't strictly necessary. If we'd like to avoid adding a dependency, I can refactor to just use regex.PR task list:
devtools::document()