http://api.drupal.org/api/function/t says: "HTML markup within translation strings is allowed, but should be avoided if possible. The exception are embedded links; link titles add a context for translators, so should be kept in the main string."

But using the code form the example:

$output = t('Go to the <a href="@contact-page">contact page</a>.', array('@contact-page' => url('contact')));

Returns in this error:

Line 20: Use the matching Drupal theme functions, not raw HTML.

	  $output = t('Go to the <a href="@contact-page">contact page</a>.', array('@contact-page' => url('contact')));

Cheers

Comments

morbus iff’s picture

morbus iff
they/them
commented

The answer for this one is: it depends. Some people prefer the above. Some people prefer:

t('Go to the @contact-page.', array('@contact-page' => l(t('contact page'), 'contact'))

You'd be fine if you labeled it a false positive and moved on.

morbus iff’s picture

morbus iff
they/them
commented
Status: Active » Closed (won't fix)
bartezz’s picture

bartezz commented
Status: Closed (won't fix) » Active

Well erm... if this module is about applying finely aged and obsessively anal wisdom from years of Drupal development and persnickety quality control and thus sticking to the docs then it's not about what people prefer IMHO but about what's in the docs... :)

http://api.drupal.org/api/function/t

Here is an example of incorrect usage of t():
$output .= t('<p>Go to the @contact-page.</p>', array('@contact-page' => l(t('contact page'), 'contact')));

Not trying to be a smartass but just giving ya some tough love :p

Cheers

morbus iff’s picture

morbus iff
they/them
commented
Status: Active » Closed (won't fix)

I won't be fixing this; it's too infrequent for my tastes and slightly prohibitive (turning a simple regexp comparison into a full-blown callback that can know when embedded links are used within a t(), and yelling at anything else). To throw the same documentation in your face as you did to me: "You should expect some false positives with Coder Tough Love, but I'd still want you to report them as bugs so I can continue to tighten the analysis." You've done so, thanks. My prerogative here is to /keep/ it as a known false positive, one that I've known of for quite some time.

Thanks for the report.

morbus iff’s picture

morbus iff
they/them
commented
Version: 6.x-1.0 » 6.x-1.x-dev
Status: Closed (won't fix) » Active

Hrm. Should be plausible by moving A into its own regexp with a "t(" lookahead.

bartezz’s picture

bartezz commented

Cool! Though throwing docs into faces sounds a bit harsh... like i said, showing you some tough love :)

I wish I could help ya by supplying a patch, yet regular expressions aren't my thing.
Somehow can't get my head around those...

Cheers

danepowell’s picture

danepowell commented

Also seeing this false positive. I know it seems like a trivial point, but if that's what's in the docs then I think Coder Tough Love should abide by it... that's kind of the whole point, right? ;)

hass’s picture

hass commented
Priority: Normal » Major

Code style in #1 is not allowed per drupal code style rules. The reason is that the link name cannot translated context sensitive. It's not people personal preference it's an i18n bug.

The complaining rule is incorrect and need to be fixed. Also have this issue, see #1390036: Use the matching Drupal theme functions, not raw HTML.

hass’s picture

hass commented
Title: False positive? Using link in string? » D7 false detection: HTML is allowed in translatable strings

Like #1390592: D7: statements should probably be wrapped in db_rewrite_sql() this issue also requires to know that D7+ is tested. We have the context sensitive translation strings since D7 and the current rule is for D6 and may only acceptable for D6, however bad it is.

hass’s picture

hass commented
Priority: Major » Critical

Still the most false detections in review on d.o.

xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Critical » Major
hefox’s picture

hefox commented

Ran coder_tough_love.module through coder review, and this issue is preventing it from passing coder tough love's review (*teehee*).

bartezz’s picture

bartezz commented

Hehehe!

Jorrit’s picture

Jorrit commented

#1 is not going to work as @contact-page will escape the HTML tag generated by l(). If the @ is replaced by an !, the "Potential problem: form_set_error() and form_error() only accept filtered text..." or "Potential problem: drupal_set_message() only accepts filtered text" error may be triggered when using these functions.

hass’s picture

hass commented

@jorrit: the string in #1 is not correct and can be seen as a bug, not only because of "!". Using l() is a bug.