#348907 follow-up: table building in privatemsg_list() needs documentation
Liam McDermott - April 10, 2009 - 03:42
| Project: | Privatemsg |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | duplicate |
| Issue tags: | 348907 |
Jump to:
Description
On line 402 of privatemsg.module (after this patch is applied) is the following code:
<?php
$threads = array();
$form['#data'] = array();
while ($row = db_fetch_array($result)) {
// Store the raw row data.
$form['#data'][$row['thread_id']] = $row;
// Store the themed row data.
$form['#rows'][$row['thread_id']] = _privatemsg_list_thread($row);
// store thread id for the checkboxes array
$threads[$row['thread_id']] = '';
}
if (empty($form['#data'])) {
// If no threads are displayed, use these default columns.
$keys = array('subject', 'author', 'last_updated');
}
else {
// Load the keys of the first row in data, we don't know the key
$keys = array_keys($form['#data'][key($form['#data'])]);
$form['actions'] = _privatemsg_action_form();
}
// Load the themed list headers based on the available data
$form['#headers'] = _privatemsg_list_headers(!empty($form['#data']), $keys);
?>Is this building a table? Because it's not immediately obvious, particularly with the $form variable being used. Documentation should be added, to tell developers what's going on.
Also, perhaps we should point out that the table is rendered into HTML by theme functions. Bonus points for adding an @see theme_privatemsg_list() and any other relevant theme functions.
#1
Table building has been moved into the theme function as part of #523064: Display tags in thread list so we don't have to add comments anymore :)