The XSL strings should be translated
Dave Reid - June 12, 2009 - 01:53
| Project: | XML sitemap |
| Version: | 6.x-1.x-dev |
| Component: | xmlsitemap.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
| Issue tags: | release 6.x-1.1 |
Jump to:
Description
Instead of having an actual XSL file, we should have the XSL as a menu callback and generate it's output in xmlsitemap.pages.inc.
#1
The XSL style sheet has been introduced to help people on navigating through their sitemap; to use translated strings would be a help more for people whose first language is not English.
#2
Instead of generating the entire file contents in code, here's my inital solution in 6.x-2.x:
<?php
function xmlsitemap_output_xsl() {
module_load_include('inc', 'xmlsitemap');
drupal_set_header('Content-type: application/xml; charset=utf-8');
$module_path = drupal_get_path('module', 'xmlsitemap');
$replacements = array(
'Sitemap file' => t('Sitemap file'),
'Generated by the Drupal XML sitemap module.' => t('Generated by the <a href="@link-xmlsitemap">Drupal XML sitemap module</a> @version.', array('@link-xmlsitemap' => 'http://drupal.org/project/xmlsitemap', '@version' => _xmlsitemap_get_version())),
'Number of sitemaps in this index' => t('Number of sitemaps in this index'),
'Click on the table headers to change sorting.' => t('Click on the table headers to change sorting.'),
'Sitemap URL' => t('Sitemap URL'),
'Last modification date' => t('Last modification date'),
'Number of URLs in this sitemap' => t('Number of URLs in this sitemap'),
'URL location' => t('URL location'),
'Change frequency' => t('Change frequency'),
'Priority' => t('Priority'),
'[xsl-css]' => base_path() . $module_path . '/xsl/xmlsitemap.xsl.css',
'[xsl-js]' => base_path() . $module_path . '/xsl/xmlsitemap.xsl.js',
);
$xsl_content = file_get_contents($module_path . '/xsl/xmlsitemap.xsl');
$xsl_content = str_replace(array_keys($replacements), $replacements, $xsl_content);
echo $xsl_content;
}
?>
Replaces all the strings that need translation, so it works.
#3
It's a good compromise between using
printfor each line of the content, and not having translated strings.I have only a question; rather than using
str_replace(), which in the specific case forces you to usearray_keys(), would not it better to usestrtr()? The vantage is that, in the case the second parameter is an array, the function requires two arguments, not three as the other function.#4
I guess I don't have a preference. strtr makes more sense. Some comment from back in 2004 suggests that str_replace is faster (http://us3.php.net/manual/en/function.strtr.php#48095), but again, that's from 2004. Now that you've pointed it out, my neurotic side wants to benchmark compare them. Thanks Kiam. :)
#5
Using
ab -n 500 http://mysql.drupal6dev.local/sitemap.xslwith caching disabled.Using str_replace():
Requests per second: 8.16 [#/sec] (mean)
Time per request: 122.540 [ms] (mean)
Time per request: 122.540 [ms] (mean, across all concurrent requests)
Transfer rate: 38.42 [Kbytes/sec] received
Connection Times (ms)
min mean[+/-sd] median max
Connect: 0 0 0.0 0 0
Processing: 118 122 4.0 121 154
Waiting: 85 118 6.4 117 148
Total: 118 122 4.0 121 154
Using strtr():
Requests per second: 7.56 [#/sec] (mean)
Time per request: 132.313 [ms] (mean)
Time per request: 132.313 [ms] (mean, across all concurrent requests)
Transfer rate: 35.58 [Kbytes/sec] received
Connection Times (ms)
min mean[+/-sd] median max
Connect: 0 0 0.1 0 1
Processing: 127 132 5.5 131 193
Waiting: 111 128 6.9 126 193
Total: 127 132 5.5 131 193
str_replace wins by just under 10 ms. :)
#6
I can report that
t()usesstrtr(), in both Drupal 6, and 7. Before to see thatstrtr()was used fromt(), I always thought tostrtr()as a function to translate single characters present in a string, not to change a substring present in a string with a different substring.I think the pro of
strtr()is shown in the example found in the PHP site, which uses the following code:<?php$trans = array("hello" => "hi", "hi" => "hello");
echo strtr("hi all, I said hello", $trans);
?>
The code is said to correctly output "hello all, I said hi"; I think that
str_replace()cannot handle that case, and this can explain why it's faster (if it is even faster).#7
Yep, I know it's used in t(). I guess a 10ms difference isn't really reason enough to prefer str_replace over strtr considering that in most cases this file will be cached and then the speed difference won't even matter. The example you're referring to is str_replace will make replacements in things that have already been replaced, which won't and doesn't happen in our set of strings but could easily happen with the enormous amount of strings that get passed through t(), so that doesn't really matter either. I'll probably change this to strtr anyway, just for simplicity's sake. And it looks nicer.
#8
http://mysql.drupal6dev.local/sitemap.xslsuggests we are in presence of a Mac-fellow. I am glad to find other people who use Drupal and Mac at the same time. :-)I must say I am already missing my Mac, even if I would not return back home just to use it again (I can buy a new Mac here, and I am already thinking of doing that).
An apple a day...
#9
Suprisingly, I'm actually using Linux Mint/Ubuntu 8.10. I just like using the usual Mac .local domain so I have set up all my localhost domains with it in /etc/hosts. I'm tricky like that. :)
#10
Considering this has been fully implemented in 6.x-2.x, moving back down to 6.x-1.x for consideration.
#11
I guessed it was a particular case, but I thought it could explain the difference of speed between the two functions.
str_replace()is faster because it takes the assumption that the example case shown forstrtr()will not happen to the arguments passed to it.#12
Now I am fixing things back (I mean with the report metadata).
#13
A Penguin-fellow, then. I am not sure I will be able to see any of them (penguins, I mean) here in the Empire State, if I don't go to the Bronx Zoo.
Linux is welcome more than Windoze is, anyway. :-)
#14
I'll agree that this may be the correct way to do it but one could easily have a files/xmlsitemap/gss.xsl file that overrides the default one in the module directory. I.E. there is an easy method to translate the strings anyway. Should we allow for a hook_xmlsitemap_xsl so that other modules could provide their own version?
#15
That is something excessive because nobody will make a module just to change the strings used in the XSL stylesheet.
The other alternative reported (to let the users change the XSL file) doesn't sound good either; if I would need the support for two languages, I should keep to rename two files to be able to swap from one language to the other.
It is also true that, if I look at http://example.com/sitemap.xml , I will always see the sitemap with English strings; to see it in Italian, I should visit http://example.com/it/sitemap.xml .
#16
I'm thinking hook_xmlsitemap_xsl could be used to provide a custom XSL and not change the strings of the default XSL. Someone could want to provide a very different looking schema that has even different translatable strings. Or maybe rather than a hook we look for a xmlsitemap_custom_xsl function that can be used to override the default so that only one module can provide the override. We don't need more than one module providing an XSL.
So at the top of xmlsitemap_output_xsl we
<?phpif (function_exists('xmlsitemap_custom_xsl')) {
echo xmlsitemap_custom_xsl();
return;
}
?>
Or something like that.
#17
Ping, comments on #16?
#18
I think that another module would not be interested in changing the XSL style sheet used from XML sitemap. I find the approach followed by the branch 2.x the right way.
I guess Dave likes the way he implemented it, otherwise he would not have committed the change in his branch.
#19
If anything I don't think making the XSL interchangeable/alterable is a high priority. I am strongly against the approach in #16 because it reminds me of custom_url_rewrite_outbound which were bastardization functions when there should have been proper Drupal hooks. If down the road two other modules want to implement it, they're screwed because only one can 'have' the custom function. Nevermind the problems if both modules declare the function we get a PHP error. So if we want to do it, here's a couple of options I think should be used (at least in the 6.x-2.x branch).
1. If clean URLs are enabled, the user can put their own sitemap.xsl file in the Drupal root directory. This will override the hook_menu() path of 'sitemap.xsl' (think robotstxt module). This doesn't require us to do anything.
2. A module can override the hook_menu() path for 'sitemap.xsl' by using hook_menu_alter():
<?phpfunction mymodule_menu_alter() {
$items['sitemap.xsl']['page callback'] = 'mymodule_output_custom_xsl';
$items['sitemap.xsl']['file path'] = drupal_get_path('module', 'mymodule');
}
?>
This function also doesn't require us to do anything.
3. We make a new variable with the path to the XSL file to be read:
<?phpfunction xmlsitemap_output_xsl() {
$module_path = drupal_get_path('module', 'xmlsitemap');
$xsl_file = variable_get('xmlsitemap_xsl_file', $module_path . '/xsl/xmlsitemap.xsl');
$xsl_content = file_get_contents($xsl_file);
// Do the XSL processing and output
...
}
?>
4. Add an hook_xmlsitemap_xsl_alter() so that other modules can alter the XSL before strtr is run:
<?phpfunction xmlsitemap_output_xsl() {
$module_path = drupal_get_path('module', 'xmlsitemap');
$xsl_content = file_get_contents($module_path . '/xsl/xmlsitemap.xsl');
// Allow other modules to alter the XSL.
drupal_alter('xmlsitemap_xsl', $xsl_content);
// Do the XSL processing and output
...
}
?>
This option they can change certain strings, change the whole darn file, whatever they want.
In summary, I probably prefer #4 as it's the most Drupal thing to do, allows changes small and large. I'm still not decided yet though.
EDIT: I had combined #3 and #4 into #4. I corrected #4.
#20
Option #4 is the Drupal 6 way of doing that; the alternative is the #3, in the case #4 could not be implemented (or we don't want to implement it).
#21
#3 is similar to the way Darren did it for D5, the exception being that he didn't use a variable and just looked for gss.xsl in files/xmlsitemap for the alternative file.
#4 is pretty and I like it.
It would be good to hear from Darren Oh as to why he considered the alternative XSL file.
#22
#3 and #4 combined are even better; it seems the perfect King Solomon solution.
#23
I will consider this at a later time.
#24