Context sensitive translation issues

t('Not sure what to do? Try the "!getting-started" page.', array('!getting-started' => $getting_started));
Could become very difficult...

Why? That's a perfectly valid string.

What single character to use as a decimal point.
What single character to use as the thousands separator.
Text to put before the number, such as currency symbol.
Text to put after the number, such as currency symbol.

When I saw this strings I wonder much... this is something that *will* dynamically change per language view... and we have a language switcher in D6 - so this strings are not wrong, but the logic behind sounds really wrong.

I use the PHP tools I am given. Those strings are should at least be run through t(). Not ideal, I know, but that's what we got.

Aside it would be good to change the many "query" to be more explainable like "database query" when it is a db query. This makes translation also easier.

No.

This view is being edited by user !user, and is therefore locked from editing by others. This lock is !age old. Click here to !break.

Look, this is the Drupal standard for embedding links into text. I'm sorry if it's hard to translate, but making it easy to translate means making really ugly strings. Cope with it.

Are you continuing to work on this patch?

Here is my review of what is currently here:

-        vpr(t('set_display called with invalid display id @display', array('@display' => $display_id)));
+        vpr(t('Set_display called with invalid display id @display.', array('@display' => $display_id)));

The original is correct; the function is set_display() not Set_display(). Since we're using a function name, the case matters. A feasible correction would be:

+        vpr(t('set_display() called with invalid display id @display.', array('@display' => $display_id)));

@@ -907,7 +907,7 @@
     if (!empty($this->definition['many to one'])) {
       $form['add_table'] = array(
         '#type' => 'checkbox',
-        '#title' => t('Allow multiple arguments to work together.'),
+        '#title' => t('Allow multiple arguments to work together'),
         '#description' => t('If selected, multiple instances of this argument can work together, as though multiple terms were supplied to the same argument. This setting is not compatible with the "Reduce duplicates" setting.'),
         '#default_value' => !empty($this->options['add_table']),
       );
@@ -1047,7 +1047,7 @@
 
     $form['not'] = array(
       '#type' => 'checkbox',
-      '#title' => t('Exclude the argument'),
+      '#title' => t('Exclude the argument.'),
       '#description' => t('If selected, the numbers entered in the argument will be excluded rather than limiting the view.'),
       '#default_value' => !empty($this->options['not']),
     );

One of the above is incorrect; I think it's probably the second one. Both are checkboxes and we don't ordinarily put periods on checkbox titles.

-    $getting_started = t('Install the advanced help module for the getting started');
+    $getting_started = t('Install the advanced help module for the getting started.');
   }
 
   $vars['help'] = t('Not sure what to do? Try the "!getting-started" page.', array('!getting-started' => $getting_started));

This change results in this sentence:

Not sure what to do? Try the "Install the advanced help module for the getting started." page.

That period doesn't belong, IMO.

-            drupal_set_message(t('@type handler @table.@field is not available.', array(
+            drupal_set_message(t('@type handler @table. @field is not available.', array(

That period is a delimiter, not an end of sentence, as in x.y -- for example, node.title

-  $vars['base_table'] = !empty($table['table']['base']['title']) ?
-    $table['table']['base']['title'] : t('Unknown or missing table name');
+  $vars['base_table'] = !empty($table['table']['base']['title']) ? $table['table']['base']['title'] : t('Unknown or missing table name.');

I like to format my code to 80 columns when there's a convenient way to do so. That ? made it convenient, so I'd prefer to leave that cr+tab in.

       'title' => t('Export'),
-      'alt' => t("Export this view"),
+      'alt' => t('Export this view'),
       'href' => "admin/build/views/export/$view->name",
     ),
     array(
       'title' => t('Clone'),
-      'alt' => t("Create a copy of this view"),
+      'alt' => t('Create a copy of this view'),
       'href' => "admin/build/views/clone/$view->name",

Shouuldn't you have added periods to these while making those changes? Alt text seems like it should have periods too. (I wish I were less sloppy about that, but I seem to just make that error by habit.)

@@ -983,8 +982,8 @@
       $path = $display->handler->get_path();
       if (strpos($path, '%') === FALSE && !isset($paths[$path])) {
         $vars['quick_links_raw'][] = array(
-          'title' => t('View "!display"', array('!display' => $display->display_title)),
-          'alt' => t("Go to the real page for this display"),
+          'title' => t('View %display', array('%display' => $display->display_title)),
+          'alt' => t('Go to the real page for this display.'),
           'href' => $path,
         );
         // Displays can have the same path; no point in showing more than one link.
@@ -1247,7 +1246,7 @@

The above text is link text; using % instead of ! will result in a double check_plain on the display title, causing double encoding ickiness if someone puts special characters in the display name. Also, I'm not comfortable adding an em inside link text.

-    $ret[] = views_ui_analysis(t('This view has only a default display and therefore will not be placed anywhere on your site; perhaps you want to add a page or a block display.'), 'warning');
+    $ret[] = views_ui_analysis(t('This view has only a default display and therefore will not be placed anywhere on your site. Perhaps you want to add a page or a block display.'), 'warning');

Are you allergic to semi-colons? I realize I overuse them but you're removing perfectly valid ones.

-    '#description' => t('Enter an optional tag for this view; it is used only to help sort views on the administrative page.'),
+    '#description' => t('Enter an optional tag for this view, that is used only to help sort views on the administrative page.'),

This change creates an invalid clause. In your version the comma should not appear. My version breaks it into distinct thoughts "do this; why", which I personally find easier for a user to interpret, but that's a matter of opinion.

-      $options = array('' => t('<None>'));
+      $options = array('' => t('- None -'));

Why?

   function render($values) {
-    $text = !empty($this->options['text']) ? $this->options['text'] : t('view');
+    $text = !empty($this->options['text']) ? $this->options['text'] : t('View');
     $nid = $values->{$this->aliases['nid']};
     return l($text, "node/$nid");
   }

While there may be some inconsistency, generally within Drupal whenever there is an operations link (see admin/content/node-types and admin/content/node) the operations are not upper-cased. These should remain lowercase. Also see Drupal's node/XXX/revisions for the 'revert' example.

@@ -284,7 +284,7 @@
       'relationship' => 'none',
     ),
   ));
-  $handler->override_option('title', 'Today\'s popular content');
+  $handler->override_option('title', "Today's popular content");
   $handler->override_option('items_per_page', 5);
   $handler->override_option('link_display', 'page_1');
   $handler->override_option('style_plugin', 'list');
@@ -298,7 +298,7 @@

Don't bother changing these; these are generated from an export, and the export will never be smart enough to choose between ' and " intelligently. The next time the export is updated this will just be reverted.

@@ -12,6 +12,6 @@
 
 <div class="more-link">
   <a href="<?php print $more_url ?>">
-    <?php print t('more'); ?>
+    <?php print t('More'); ?>
   </a>
 </div>

About a year ago there was a long thread about the more link. The end result: try to keep it just like Drupal's. Leave it lowercase.

About the string "View" and/or "view": Is this view-as-verb or view-as-noun?

View-as-noun is problematic since "View" and "view" are used as verbs in core (like the tab on http://drupal.org/user). For some languages there's no word or expression which can cover both meanings.

In core D6, a similar problem ("May as long month name" = "May as short month name" in English, but not in all other languages) was solved by introducing the string "!long-month-name May", while keeping the string "May" for the short version.

Is it possible to use a "!noun View" for those cases where we're talking about "a view", and not "to view"?

here are some minimal fixes pulled from the last patch. Skipped most of the punctuation/capitalization suggested changes.

missed the change to the template file.

Ok, I committed pwolanin's stripped down version. I realize there are other issues remaining but some of them require more thought. It's best to do the ones that require thought separately and get the easy ones in.

Oh and let's separate ones that require thought into separate issues.

Is the "view as noun" vs "view as verb" thing in #20 one of those that require thought?

zirvap: Sorry, yes it may be. I think it may only matter if View is ever used as a standalone, though, which I'm not sure it is.

No problem, I'm just wondering whether I should start a new issue for this or not.

Searching for "view" in http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/views/trans... results in

#: modules/comment/views_handler_field_comment_link.inc:34 modules/node/views_handler_field_node_link.inc:35 modules/user/views_handler_field_user_link.inc:38
msgid "view"

I've no idea if that's a view-as-verb or view-as-noun context, though.

Those you list are all 'view' as verb -- they are links to objects (content, comment, user) with 'view' as the default link text, in the same manner that core uses it.

Aha, then there's no problem -- or at least, there's no problem with ambigious translations of the string "view" :-)