Use the coder module to save embarrassment!
On my last blog post, I threw out a one-line patch for a usability enhancement to the Bio module. Here's the line in question:
<?php
drupal_set_message(t('You can edit Bio settings at ') . l('admin/user/bio','admin/user/bio'));
?>How many errors are there in that line? Many more than I expected, as dww very patiently pointed out to me:
- no tabs -- we use 2 spaces.
- that's not the right way to handle t() and a link, since then it's backwards in RTL languages.
- we don't generally print raw URL paths like that in help messages
- help messages like this should end with a period.
- need to use a space after the comma between function parameters
Eeek!
I really appreciate dww pointing out the mistakes rather than just dismissing it by saying "go look at the coding standards and come back when you have a clue!". Because that's probably what many people would be tempted to do.
I've since read the above guidelines, and downloaded the Coder module, which is a pretty awesome way of saving a bit of embarrassment next time you want to submit a patch. Here's how it works:
1) Copy to sites/all/modules and enable the usual way. Now you have a new link in Navigation:
2) Click that link, then select exactly what you want the Coder module to do on this page:
3) Select what you'd like to review, there are many different options:
4) Submit, then view the results:
And voila! Sure it doesn't get everything, but it's still far better to have a module remind you of your elementary mistakes, than to waste a core developer's time! ;)
The reason of course for my different coding style is that 99.9% of modules and code I've written is unreleased. So sometime in the near future, I promise to get my modules up to scratch, get a CVS account and unleash them apon the world! Stay tuned!
Proper Syntax?
Hey CB,
I started adding the snippet to my modules a week ago (following your guidance).
Can you post an example of the code using the cleaned up syntax? I don't quite "get" dww's comment about rtl languages and the use of t().
Thanks!
Kevin
Re: Proper Syntax
It should probably be something like this, based on the recommendations I'm seeing:
<?phpdrupal_set_message(t('You can edit Bio settings at !url.', array('!url' => . l(t('its settings page'), 'admin/user/bio'))));
?>
Of course, I'm not a core developer and it only addresses the code standards mentioned specifically in this blog posting.
Oops, missed one thing
Forgot to take out the concatenation operator while editing the original.
That should be:
<?phpdrupal_set_message(t('You can edit Bio settings at !url.', array('!url' => l(t('its settings page'), 'admin/user/bio'))));
?>
Hi Kevin, This, I believe,
Hi Kevin,
This, I believe, is the correct syntax:
<?phpdrupal_set_message(t('You can configure the bio module on the <a href="@url">bio settings page</a>.', array('@url' => url('admin/user/bio'))));
?>
This is taken from t() function page over at api.drupal.org, where it gives an almost identical example.
http://api.drupal.org/api/function/t/5
t() and RTL
I think what he means is not really so much about RTL as it is about the grammar of various languages. When you write
drupal_set_message(t('something on some ') . $url);, you are assuming that in any language the sentence should look like "something on /foo".But in some hypothetical language, says "z", it could very well need to appear as "
zomezing /foo-zlocazed", which can never be translated by the original construct, since it forces the relative positions of sentence and url.OTOH, "
t('something on !url.', array('!url' => $url))" can be translated easily: the translators will receive this string to translate: "something on !url" and can then provide as a translation "zomething !url-zlocazed.", and your code will now work even in that languaze, er... language.Great explanation. It holds
Great explanation. It holds for RTL languages too. The translator will look at:
"You can change bio settings at" and maybe translate it to a RTL language:
"at settings bio change can you"
(obviously in another language, just illustrating a point).
Then the link will be appended, giving the following:
"at settings bio change can you" . "/admin/user/bio"
Read that RTL and the link is clearly in the wrong place.
RTL does not work that way
Characters in RTL strings are merely rendered from right to left when displayed, but in memory, the strings follow the same logical progression order as LTR languages.
Yes, I know... that's not
Yes, I know... that's not what I was saying. I was just trying to illustrate that dww is right about the link being displayed backwards.
I ran a quick test in my sandbox, here's the output:
This link should be on the left of the sentence, which is clearly not the case.
Still no conclusion
I have been searching for the proper format for a link in drupal_set_message for over an hour.. I give up, everyone has a different answer. There should be a page on d.o with examples that are acceptable for real life.
Post new comment