Problem/Motivation

While working on the t() docs in #2570431: Document that certain (non-"href") attribute values in t() and SafeMarkup::format() are not supported and may be insecure,
noticed this:

 * The t() function serves two purposes. First, at run-time it translates
 * user-visible text into the appropriate language. Second, various mechanisms
 * that figure out what text needs to be translated work off t() -- the text
 * inside t() calls is added to the database of strings to be translated.
 * These strings are expected to be in English, so the first argument should
 * always be in English. To enable a fully-translatable site, it is important
 * that all human-readable text that will be displayed on the site or sent to
 * a user is passed through the t() function, or a related function. See the
 * @link https://www.drupal.org/node/322729 Localization API @endlink pages for
 * more information, including recommendations on how to break up or not
 * break up strings for translation.

in bootstrap.inc

The part:
"To enable a fully-translatable site"

might be misleading.

Proposed resolution

Change to
"To allow the site to be localized"
or
....

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Update the patch to incorporate feedback from reviews (include an interdiff) Instructions
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards Instructions

User interface changes

None.

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YesCT created an issue. See original summary.

joyceg’s picture

Assigned: Unassigned » joyceg
joyceg’s picture

joyceg’s picture

Status: Active » Needs review
YesCT’s picture

Issue summary: View changes
Status: Needs review » Needs work

@joyceg thanks for making the patch.

The wording looks good to me.

since that shortened the line, that line and the next lines should be re-wrapped to be as close to 80 characters as possible. (dont fix unrelated things, just the lines we have to change)

https://www.drupal.org/node/1354#drupal

"Lines containing comments (including docblocks) must wrap as close to 80 characters as possible without going over,..."

joyceg’s picture

FileSize
1.18 KB
joyceg’s picture

I am adding the interdiff and the patch here.

joyceg’s picture

Status: Needs work » Needs review
YesCT’s picture

Status: Needs review » Needs work
+++ b/core/includes/bootstrap.inc
@@ -271,10 +271,10 @@ function drupal_get_path($type, $name) {
+ * https://www.drupal.org/node/322729 Localization API @endlink pages for
  * more information, including recommendations on how to break up or not
  * break up strings for translation.

I think these lines need to be wrapped to 80 chars also.

---------

and the patch file should not be in the patch file... :)

adding .patch to your gitignore is a good idea.
here is a sample of mine if that helps https://gist.github.com/YesCT/d968f8209fde43c52090

The last submitted patch, 7: 2576431-7.patch, failed testing.

joyceg’s picture

Okay.I will work on it.

joyceg’s picture

joyceg’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: clarification-2576431-12-D8.patch, failed testing.

The last submitted patch, 12: clarification-2576431-12-D8.patch, failed testing.

YesCT’s picture

+++ b/core/includes/bootstrap.inc
@@ -271,12 +271,12 @@ function drupal_get_path($type, $name) {
+ * is passed through the t() function, or a related function. See the @link
+ * https://www.drupal.org/node/322729 Localization API @endlink pages for more

so I looked up @link:
https://www.drupal.org/node/1354#link

The entire @link ... @endlink must be on the same line

YesCT’s picture

Status: Needs work » Needs review
FileSize
1.1 KB
1.1 KB

doing that. interdiff to patch in 3.

reviewing with
git diff --color-words
will probably be helpful.

Status: Needs review » Needs work

The last submitted patch, 17: 2576431.17.patch, failed testing.

The last submitted patch, 17: 2576431.17.patch, failed testing.

YesCT’s picture

Status: Needs work » Needs review
FileSize
1.1 KB

ha! now the actual patch.

j2r’s picture

Status: Needs review » Reviewed & tested by the community

Applied the patch and docs updated as required.

Previous text - To enable a fully-translatable site,
Updated text - To allow the site to be localized,

dani3lr0se’s picture

The new wording looks good. Nice job.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: +rc eligible

Committed 10f1025 and pushed to 8.0.x. Thanks!

  • alexpott committed 10f1025 on 8.0.x
    Issue #2576431 by joyceg, YesCT: Improve t() docs "fully-translatable...

Status: Fixed » Closed (fixed)

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