Split off from http://drupal.org/node/329998#comment-1099076.

In Szeged, we had a big long discussion about t() around the schema descriptions and it was the consensus of everyone at the table (who included Dries) that t()s should be removed from these descriptions. They mess up things because t() isn't available that early, and people discussed that no one is going to take the time to translate technical descriptions of stuff, and it doesn't really make sense since we also don't translate code comments, for example.

Hopefully someone can script this up so it's not too horrible. :)

Files: 
CommentFileSizeAuthor
#34 hook_schema_remove_t_docs.patch1.88 KBquicksketch
#27 t_remove_d6.patch158.3 KBandypost
#26 t6_remove.patch158.2 KBandypost
#14 issue-33213-14.patch8.21 KBlilou
Passed: 7483 passes, 0 fails, 0 exceptions
[ View ]
#10 htm-schema-translation.txt1022 byteslilou
#8 issue-332123-8-D7.patch162.29 KBlilou
Failed: Failed to apply patch.
[ View ]
#5 issue-332123-5-D7.patch162.29 KBlilou
#2 issue-332123-D7.patch165.57 KBlilou
Failed: Invalid PHP syntax.
[ View ]
#1 issue-332123-D7.patch165.12 KBlilou
Failed: Invalid PHP syntax.
[ View ]

Comments

lilou’s picture

Status:Active» Needs review
StatusFileSize
new165.12 KB
Failed: Invalid PHP syntax.
[ View ]

It should be clean.

lilou’s picture

StatusFileSize
new165.57 KB
Failed: Invalid PHP syntax.
[ View ]

replace also multiline (in simpletest.install) :

        'description' => t('Primary Key: Unique simpletest ID used to group test results together. Each time a set of tests
                            are run a new test ID is used.'),
swentel’s picture

Errors in color.install - where t() shouldn't be removed , this is for the requirements, nothing todo with schema iirc

lilou’s picture

undo color.install.

lilou’s picture

StatusFileSize
new162.29 KB

Undo two wrong replacements in book.install and forum.install.

swentel’s picture

while we're at it, php install and color install use t() but they should rather use $t = get_t() like other modules do I think. Can someone else confirm that ?

Anonymous’s picture

Status:Needs review» Needs work

The last submitted patch failed testing.

lilou’s picture

Status:Needs work» Needs review
StatusFileSize
new162.29 KB
Failed: Failed to apply patch.
[ View ]

Reroll.

Dries’s picture

Status:Needs review» Needs work

Looks good. Committed to CVS HEAD. We should document this on the module upgrade page. After it has been documented, please mark this 'fixed'. Thanks!

lilou’s picture

StatusFileSize
new1022 bytes

I haven't permission for edit page http://drupal.org/node/224333.

  1. Schema descriptions are no longer translated

Schema descriptions are no longer translated

(Issue) To reduce the strings to translate, descriptions of the schema database (tables and fields) in module.install no longer need to be translated.

6.x:

<?php
function forum_schema() {
 
$schema['forum'] = array(
   
'description' => t('Stores the relationship of nodes to forum terms.'),
   
'fields' => array(
     
'nid' => array(
       
'description' => t('The {node}.nid of the node.'),
      ),
    ),
  );
  return
$schema;
}
?>

7.x:

<?php
function forum_schema() {
 
$schema['forum'] = array(
   
'description' => 'Stores the relationship of nodes to forum terms.',
   
'fields' => array(
     
'nid' => array(
       
'description' => 'The {node}.nid of the node.',
      ),
    ),
  );
  return
$schema;
}
?>
webchick’s picture

@lilou: that horrible bug has now been fixed. Welcome to the documentation team. ;) Please feel free to add in your edits, which look great.

lilou’s picture

Status:Needs work» Fixed

Thx.

swentel’s picture

Status:Fixed» Needs work

block.install still needs some love - Let's look at others too. Noticed this thanks to #337794: PostgreSQL surge #1: make simpletest works again

lilou’s picture

Status:Needs work» Needs review
StatusFileSize
new8.21 KB
Passed: 7483 passes, 0 fails, 0 exceptions
[ View ]

You're right.

webchick’s picture

Status:Needs review» Fixed

Thanks; committed!

Pasqualle’s picture

Status:Fixed» Postponed (maintainer needs more info)

Actually the schema descriptions are all translated for D6, and also translated for contributed modules (which are translated already).

I agree, that translating these strings is meaningless. My question (1) is can I file issues for D6 contributed modules to remove t() from schema descriptions? That would make module translation much more easier, and if we translate D6 modules, then removing this option in D7 just does not seem right.

And also (2) can I file an issue for potx module, to not extract scheme descriptions from D6 and D6 modules?

Pasqualle’s picture

Pasqualle’s picture

so, it seems like we will have to backport this patch to D6 (read the potx issue)

hass’s picture

I can only guess how many hours I've spend on the German translation of schema translation in D& and now all this translations are going to be lost.

Gábor Hojtsy’s picture

Version:7.x-dev» 6.x-dev
Status:Postponed (maintainer needs more info)» Patch (to be ported)

I've translated most of the Hungarian schema strings just to see that the translation is 100%, not to say that it will be ever read, or that it would not go against storing the descriptions in the database as table comments. I am in support of a backport.

Gábor Hojtsy’s picture

Summarizing reasons to remove t() for hass, since he does not yet get it (see http://drupal.org/node/338409#comment-1131476):

- the functionality behind t() is not available when the schema hooks are invoked in the installer, up until locale module is enabled, so the t() calls always return the same string in installation time
- this text is documenting the code, and therefore is similar to code comments, which are not translated either
- this is technical text, and many of the translation teams such as the Hungarian team translates it just to see 100% completed translations, not because it will ever show on the interface
- if t() is not used on these strings, Drupal can actually add the descriptions to the tables and fields themselves in the database (for those backends where this is supported, for example MySQL - http://dev.mysql.com/doc/refman/5.1/en/create-table.html), so more powerful database tools can operate with those, show and help database admins to work with the tables

hass’s picture

You missed to say that Schema module shows the translated strings in UI.

Gábor Hojtsy’s picture

It does not need to.

andypost’s picture

Schema field descriptions possible used in views, isn't it?
If no, suppose, t() for schema in 6x is useless.

andypost’s picture

Another opinions to remove t()
- less runtime memory usage
- faster serialize|unserialize for locale cache

because most of descriptions less then 75 chars so they stored in locale cache but useless in production env

andypost’s picture

Status:Patch (to be ported)» Needs review
StatusFileSize
new158.2 KB

Every *.install in modules checked and cleared hook_schema t()

andypost’s picture

StatusFileSize
new158.3 KB

Chasing 6-dev

hass’s picture

Status:Needs review» Reviewed & tested by the community

Code applies cleanly, Codewise also good and tested on dev machine with all core modules enabled.

Gábor Hojtsy’s picture

Status:Reviewed & tested by the community» Fixed

Committed to Drupal 6, thanks!

Also updated docs at http://drupal.org/node/322731 to say:

One notable case in installer code is hook_schema(), which runs at install time, but Drupal core (up until Drupal 6.9) and contributed modules alike used t() for descriptions of tables and fields. The reason for that is that the Drupal installer itself does not use these strings, so it is not bothered with it not being translated while used in the installer. The contributed schema module uses these strings to help you understand the database structure. Drupal 7 changed this practice to not use t() at all in schema descriptions for various reasons (see http://drupal.org/node/332123 for more information). Therefore to save translators from pouring work into something they will entirely loose later (and help website performance as well as other good reasons), Drupal 6.9 does not use t() on these strings anymore. You should include schema strings as verbatim strings without wrapping them in localization code.

<?php
/**
 * Implementation of hook_schema() to demonstrate that t() is not used.
 */
function example_schema() {
 
$schema['example_table'] = array(
   
'description' => 'This table stores example stuff.',
   
'fields' => array(
     
'eid' => array(
       
'description' => 'Example identifier.',
       
'type' => 'serial',
       
'not null' => TRUE,
      ),
     
'value' => array(
       
'description' => 'Value of the example. Can be a string or a serialized array.',
       
'type' => 'text',
       
'not null' => TRUE
     
)
    ),
   
'primary key' => array('eid'),
  );
  return
$schema;
}
?>
kkaefer’s picture

Yay! This was really frustrating to translate.

Status:Fixed» Closed (fixed)

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

quicksketch’s picture

Status:Closed (fixed)» Needs work

We need to update more documentation on this. All the examples of hook_schema still include this code.

http://api.drupal.org/api/function/hook_schema
http://drupal.org/node/146843

John Morahan’s picture

I changed http://drupal.org/node/146862 - didn't find any other examples of descriptions in that section of the handbook.
http://api.drupal.org/api/function/hook_schema is still wrong, including the D7 version

quicksketch’s picture

Status:Needs work» Needs review
Issue tags:+Quick fix, +Needs Documentation
StatusFileSize
new1.88 KB

Could we get this committed to HEAD? Then we can do the same to the D6 version (which is still in contrib, so I can get it).

webchick’s picture

Status:Needs review» Fixed

Committed. Thanks!

webchick’s picture

Status:Fixed» Patch (to be ported)

Or, well.

quicksketch’s picture

Status:Patch (to be ported)» Fixed

Fixed in D6 http://drupal.org/cvs?commit=214280

Thanks webchick!

Status:Fixed» Closed (fixed)
Issue tags:-Quick fix, -Needs Documentation

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