Replace deprecated sprintf rules in core translations

[chillu]

We've deprecated the use of sprintf() style placeholders (e.g. %s) in _t() calls with #6558 - but core still uses it. There's around two dozen strings in core. Ideally we also replace this in the existing translations as well, so the translation strings don't get invalidated (we either have to fix them or remove them, otherwise will see stray %s in the UI)

[robbieaverill]

@chillu there's a few cases in the tests, especially i18nTest that still use either sprintf(_t(...)) or unnamed placeholders. Some of the tests in that class are also specifically testing the legacy behaviour. Do you want these cases updated/removed?

[chillu]

Correct, everything referring to %s in i18nTest.php should be rewritten to test the new, associative array format. Since the functionality is deprecated, I'd say we should remove the tests at the same time as the feature itself, unless it's hard to separate out the existing tests

[robbieaverill]

@chillu I've removed sprintf and %s use in the i18n related tests. There was one change in i18nTestModuleInclude.ss re: SPRINTFINCLUDENONAMESPACE which didn't seem to "magically" inherit the template name as a prefix which is defined in the test class. @tractorcow had a look at this with me and we weren't sure why/how it works currently, but I figure that it's an ambiguous mistake that works for some reason so I've added the template prefix to it to be more explicit.

I've also removed the "legacy" assertions which were explicitly testing a bunch of %s placeholders and were marked as deprecated.

[tractorcow]

The issue was with legacy <% _t() %> template behaviour.

    function OldTPart_QuotedString(&$res, $sub)
    {
        $entity = $sub['String']['text'];
        if (strpos($entity, '.') === false) {
            $res['php'] .= "\$scope->XML_val('I18NNamespace').'.$entity'";
        } else {
            $res['php'] .= "'$entity'";
        }
    }

I suggest removing this, as it's been deprecated for quite some time. Templates should use <%t from now on.

[robbieaverill]

@tractorcow do you want me to whip this out as part of the PR?

[tractorcow]

It's up to you; It can be another PR or just a part of this if you like.

[robbieaverill]

@tractorcow @chillu are we ready for this PR now re: Transifex? If so I'll rebase #6598 tomorrow.

[tractorcow]

yes please

[robbieaverill]

Pulls

• FIX Replace deprecated %s placeholders in translations with named placeholders #6598
• FIX Remove legacy sprintf style translations in favour of placeholders silverstripe-versioned#12
• FIX Remove legacy sprintf style translations in favour of placeholders silverstripe-cms#1826
• FIX Remove legacy translation behaviour in template silverstripe-siteconfig#58
• FIX Remove legacy translation behaviour in template silverstripe-admin#78

[robbieaverill]

FYI @tractorcow I haven't removed the legacy template parser functions in #6598 - they're marked as deprecated against 5.0, didn't want to remove them in this PR.

[tractorcow]

Thanks @robbieaverill that's good. :D I'll review your work next week.

[dhensby]

bump @tractorcow

[tractorcow]

cms and versioned need rebasing, @robbieaverill