Fix Resurrect Achievement Items#2938
Fix Resurrect Achievement Items#2938dlglin wants to merge 4 commits intoopenwebwork:WeBWorK-2.21from
Conversation
| if ($set->enable_reduced_scoring && ($set->reduced_scoring_date != $set->due_date)) { | ||
| return $c->maketext( | ||
| 'This assignment has been reopened and is due on [_1]. After that date any work ' | ||
| . 'completed will count for [_2]% of its value until [_3]. ', | ||
| $c->formatDateTime($set->reduced_scoring_date, $c->ce->{studentDateDisplayFormat}), | ||
| $c->ce->{pg}{ansEvalDefaults}{reducedScoringValue} * 100, | ||
| $c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat}) | ||
| ) . $rerandomizeMessage; | ||
| } else { | ||
| return $c->maketext( | ||
| 'This assignment has been reopened and will now close on [_1]. ', | ||
| $c->formatDateTime($set->due_date, $c->ce->{studentDateDisplayFormat}) | ||
| ) . $rerandomizeMessage; | ||
| } | ||
| } |
There was a problem hiding this comment.
$rerandomizeMessage should also be a positional argument inside of the maketext call.
There was a problem hiding this comment.
Unsure if that gets tricky or there should be a conditional to check if it should be added or not so the translated strings don't have extra space in them in the case rerandomizeMessage is empty.
There was a problem hiding this comment.
$rerandomizeMessage is the result of a maketext call, so that string will already be translated. I don't see how having it as a positional argument is necessary.
Is it a problem to have an extra space at the end of a string to be translated?
There was a problem hiding this comment.
My understanding is not all languages are right to left, so just appending the string to the end may not be appropriate for some translations.
In terms of the html, an extra space won't matter, but what happens if a translation doesn't understand that space is important and leaves it off. Putting the translated string as a positional argument inside the maketext call makes the use of the space clearer and allows positioning the statement in the string appropriately for the translation.
There are various places thought out the code that a string is translated and then added as a positional argument in future translations to join things together. I think this should probably do the same. I think the space thing is very minor and probably won't matter if the positional argument is empty.
There was a problem hiding this comment.
Others may have a better knowledge, what you did may be okay, I just feel it isn't. I think a positional argument is better than string concatenation. I'm unsure if there is a better approach to avoid having to write lots of cases like you have been doing in most of this.
There was a problem hiding this comment.
It is always best to give as much as possible together for translations. However, generally concatenations of two sentences can be done. Usually the bidirectional algorithm will make it work even for right to left languages. However, the space should be inserted between the translations and not inside of one of them. The translator may not add that space in the translation, and even if they do may not put it in the correct place, not knowing that it is going to be concatenated with another translation.
Perhaps making it a positional argument inside another translation is better though.
3 participants
This fixes a couple of issues with the resurrect achievement items in the case where reduced scoring is enabled:
This fixes #2907.