Raw print_r() output is hard to read, wrapping it in <pre></pre> improves readability a lot.

(Realized while trying to hunt down a suspected minor issue with Google fonts import.)

Patch coming.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Drave Robber’s picture

Status: Active » Needs review
FileSize
15.27 KB

Patch attached.

sreynen’s picture

Status: Needs review » Needs work

I think the <pre></pre> needs to be added in the parameter, not the message. fontyourface_log() passes the message to watchdog(), which passes it to t(). http://drupal.org/node/322774 says block tags shouldn't be in messages, and <pre></pre> is a block tag.

Drave Robber’s picture

<blabla>While I believe 'block tags' might be a bit too formal criterion given how different use cases can be,</blabla>
this makes a lot of sense on entirely different grounds - not making existing translations obsolete if we can avoid that.

Now, putting <pre> in the parameter would also require changing the placeholder from @ to !; as we're mostly dumping external data, we should probably take some precautions. So

fontyourface_log('Response: @response', array('@response' => print_r($response, TRUE)));

would become

fontyourface_log('Response: !response', array('!response' => '<pre>' . filter_xss(print_r($response, TRUE)) . '</pre>'));

(for some obscure reason, theme_dblog_message() does filter_xss() only on overview, not on details page)
 
Thoughts?

Neslee Canil Pinto’s picture

Issue summary: View changes
Status: Needs work » Closed (outdated)