-
Notifications
You must be signed in to change notification settings - Fork 334
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 Remove legacy sprintf style translations in favour of placeholders #1826
FIX Remove legacy sprintf style translations in favour of placeholders #1826
Conversation
a67863a
to
521f405
Compare
big conflicts |
@robbieaverill can we rebase this and I'll merge it in. |
521f405
to
92f995d
Compare
@tractorcow rebased, thanks =) |
@@ -1742,7 +1742,7 @@ public function delete($data, $form) | |||
|
|||
$this->getResponse()->addHeader( | |||
'X-Status', | |||
rawurlencode(sprintf(_t('SilverStripe\\CMS\\Controllers\\CMSMain.REMOVEDPAGEFROMDRAFT', "Removed '%s' from the draft site"), $record->Title)) | |||
rawurlencode(sprintf(_t(__CLASS__ . '.REMOVEDPAGEFROMDRAFT', "Removed '{title}' from the draft site"), $record->Title)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Darn, I just realised we need to remove these sprintf calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #1915
I've manually cherry picked this to 4 branch. |
Issue: silverstripe/silverstripe-framework#6582
Note: CMSBatchAction_* classes have still got
%d
placeholders in them, because they get passed around a bit before being replaced after the actual translation. I figured this isn't appropriate to replace at this point as it's logic change rather than sprintf -> named placeholder swapping.