Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
t() function is used to translate strings without page context. Some unnecessary white-spaces removed, and other cosmetic white-spaces for different sentence concatenation moved outside the t() function.
Comment | File | Size | Author |
---|---|---|---|
#8 | t-new.patch | 5.32 KB | TR |
t-function-spaces.patch | 6.1 KB | oriol_e9g | |
Comments
Comment #1
plachThis should be an easy one to review.
Comment #3
oriol_e9gt-function-spaces.patch queued for re-testing.
Comment #4
aspilicious CreditAttribution: aspilicious commentedLol at the tag
Comment #5
oriol_e9gOps! O.o WTF!
Comment #6
skottler CreditAttribution: skottler commentedt-function-spaces.patch queued for re-testing.
Comment #8
TR CreditAttribution: TR commentedPatch failed to apply because commit 1ce52d92fa19def removed the t() call from field.form.inc.
Attached is a re-rolled patch which excludes the change to field.form.inc.
Comment #9
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedMaybe we should include this one:
./modules/book/book-navigation.tpl.php: <a href="<?php print $next_url; ?>" class="page-next" title="<?php print t('Go to next page'); ?>"><?php print $next_title . t('
›'); ?></a>
That space should better be outside the t(), shouldn't it?
Comment #10
TR CreditAttribution: TR commented@ Niklas Fiekas: That use of t() in the book-navigation.tpl.php seems wrong to me. ">" is not a character that needs translating, with or without whitespace included. Yes, I understand that it's desirable to allow users to customize this "arrow" symbol, much as breadcrumb separators may be customized, but that's not something that should be done via t(). Thus I feel that the issue you raise is a theming issue more appropriately addressed in a separate bug report filed against the book module.
Changing back to needs review. Patch still applies cleanly in D8 and D7.
Comment #11
TR CreditAttribution: TR commentedActually, the use of t() in book-navigation.tpl.php is already being addressed in #736604: book module now without clearfix & better markup + accessibility love, so there's no need to deal with it here.
Comment #12
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedThanks TR, I just grepped for more whitespace in t's and didn't think about that. Since it was the only thing I criticized: RTBC.
Comment #13
Gábor HojtsyAgreed, looks good.
Comment #14
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks!
Should we backport this to 7.x?
Comment #15
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedYes, I guess.
Comment #16
webchickOverall this looks *pretty* safe. The string changes are all in admin pages or tests, so no unwitting end users should stumble across them. And these are legitimate bugs with the strings, so I think they're ok for D7.
It's interesting that file.test has a hunk like this:
...but yet there's no corresponding change in file.module. I'm wondering how that could possibly not be failing tests right now. Hm.
At any rate, committed and pushed to 7.x. Thanks!