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

FIX Finish removing deprecated i18n functionality. #11528

Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 4 additions & 24 deletions src/i18n/i18n.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use SilverStripe\i18n\Messages\MessageProvider;
use SilverStripe\View\TemplateGlobalProvider;
use InvalidArgumentException;
use SilverStripe\Core\ArrayLib;

/**
* Base-class for storage and retrieval of translated entities.
Expand Down Expand Up @@ -170,20 +171,9 @@ public static function _t($entity, $arg = null)
user_error("Missing default for localisation key $entity", E_USER_WARNING);
}

// Deprecate legacy injection format (`string %s, %d`)
// inject the variables from injectionArray (if present)
$sprintfArgs = [];
if ($default && !preg_match('/\{[\w\d]*\}/i', $default ?? '') && preg_match('/%[s,d]/', $default ?? '')) {
$sprintfArgs = array_values($injection ?? []);
$injection = [];
}

// If injection isn't associative, assume legacy injection format
$failUnlessSprintf = false;
if ($injection && array_values($injection ?? []) === $injection) {
$failUnlessSprintf = true; // Note: Will trigger either a deprecation error or exception below
$sprintfArgs = array_values($injection ?? []);
$injection = [];
// Injection must be associative
if (!empty($injection) && !ArrayLib::is_associative($injection)) {
throw new InvalidArgumentException('Injection must be an associative array');
}
Comment on lines +174 to 177
Copy link
Member Author

Choose a reason for hiding this comment

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

Uses a more declarative condition than the previous one, but the result is the same.

Error moved here from below, as per the comment (see line 209)


// Detect plurals: Has a {count} argument as well as a `|` pipe delimited string (if provided)
Expand All @@ -201,16 +191,6 @@ public static function _t($entity, $arg = null)
$result = static::getMessageProvider()->translate($entity, $default, $injection);
}

if (!$default && !preg_match('/\{[\w\d]*\}/i', $result ?? '') && preg_match('/%[s,d]/', $result ?? '')) {
throw new Exception('sprintf style localisation cannot be used in translations - detected in $result');
}

if ($failUnlessSprintf) {
// Note: After removing deprecated code, you can move this error up into the is-associative check
// Neither default nor translated strings were %s substituted, and our array isn't associative
throw new InvalidArgumentException('Injection must be an associative array');
}

return $result;
}

Expand Down
2 changes: 1 addition & 1 deletion src/includes/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,6 @@ function project()
*/
function _t($entity, $arg = null)
{
// Pass args directly to handle deprecation
// Pass args directly as we allow variable args.
Copy link
Member Author

Choose a reason for hiding this comment

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

We still allow variable args because you can technically add a comment which should be but isn't used by the i18n text collector. Changing that is well outside the scope of this PR so leaving as is.

return call_user_func_array([i18n::class, '_t'], func_get_args());
}
Loading