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.

CommentFileSizeAuthor
#1 fontyourface-Improve_readability_of_debugging_info-1845672-1.patch15.27 KBDrave Robber

Comments

Drave Robber’s picture

Drave Robber commented
Status: Active » Needs review
StatusFileSize
new fontyourface-Improve_readability_of_debugging_info-1845672-1.patch15.27 KB

Patch attached.

sreynen’s picture

sreynen commented
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

Drave Robber commented

<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

neslee canil pinto
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs work » Closed (outdated)