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.

Files: 
CommentFileSizeAuthor
#8 t-new.patch5.32 KBTR
PASSED: [[SimpleTest]]: [MySQL] 32,988 pass(es).
[ View ]
t-function-spaces.patch6.1 KBoriol_e9g
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch t-function-spaces.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Comments

Component:language system» locale.module
Issue tags:+Novice

This should be an easy one to review.

Issue tags:-Novice

t-function-spaces.patch queued for re-testing.

Lol at the tag

Issue tags:+Novice

Ops! O.o WTF!

Issue tags:-Novice

t-function-spaces.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Novice

The last submitted patch, t-function-spaces.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new5.32 KB
PASSED: [[SimpleTest]]: [MySQL] 32,988 pass(es).
[ View ]

Patch 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.

Status:Needs review» Needs work

Maybe 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?

Assigned:oriol_e9g» Unassigned
Status:Needs work» Needs review

@ 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.

Actually, 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.

Status:Needs review» Reviewed & tested by the community

Thanks 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.

Agreed, looks good.

Version:8.x-dev» 7.x-dev

Committed to 8.x. Thanks!

Should we backport this to 7.x?

Issue tags:+needs backport to D7

Yes, I guess.

Status:Reviewed & tested by the community» Fixed

Overall 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:

-    $message = t('Only files with the following extensions are allowed: ') . '<em class="placeholder">' . $extensions . '</em>';
+    $message = t('Only files with the following extensions are allowed:') . ' <em class="placeholder">' . $extensions . '</em>';

...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!

Automatically closed -- issue fixed for 2 weeks with no activity.