[drupal-devel] Re: [drupal:dries] /includes theme.inc
Op 18-okt-2005, om 16:45 heeft drupal-devel@drupal.org het volgende geschreven:
User: dries Branch: HEAD Date: Tue, 18 Oct 2005 14:45:09 +0000
Modified files: /includes theme.inc
Log message: - Patch #32573 by Moshe: added support for ordered lists to theme_item_list().
Links: http://cvs.drupal.org/diff.php?path=drupal/includes/ theme.inc&old=1.261&new=1.262
-- [ Drupal cvs list | http://list.drupal.org/ ]
I do not, and I repeat _not_ like this approach! A nicer solution would imo be: /** * Return a themed list of items. * * @param $items * An array of items to be displayed in the list. * @param $title * The (optional) title of the list. * @return * A string containing the list output. */ function theme_item_list($items = array(), $title = NULL, $type = 'unordered list') { $class = str_replace('-', ' ', $type); if (isset($items)) { switch($type) { case 'unordered list': $list .= '<ul>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ul>'; case 'ordered list': $list .= '<ol>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ol>'; case 'definition list': $list .= '<dl>'; foreach ($items as $item) { $list .= '<dt>'. $item .'</dt>'; } $list .= '</dl>'; } if (isset($title)) { $output .= '<h3>'. $title .'</h3>'; } $output .= '<div class="'. $class .'">'. $list .'</div>'; return $output; } Isn't something like this nicer? And, easier to use? Steef --- Stefan Nagtegaal Drupal-Devel@iStyledThis.nl Drupal Development Mailinglist
On 10/18/05, drupal-devel-owner@drupal.org <drupal-devel-owner@drupal.org> wrote:
Op 18-okt-2005, om 16:45 heeft drupal-devel@drupal.org het volgende geschreven: I do not, and I repeat _not_ like this approach!
A nicer solution would imo be: <snip> Isn't something like this nicer? And, easier to use?
I'd rather type 'ul' over 'unordered list' any day, so no. Plus, a definition list looks like this: <dl> <dt>term</dt> <dd>description</dd> </dl> Grtz, Breyten :)
Calm down Steef ... Perhaps you want to submit a patch.
I do not, and I repeat _not_ like this approach!
A nicer solution would imo be:
/** * Return a themed list of items. * * @param $items * An array of items to be displayed in the list. * @param $title * The (optional) title of the list. * @return * A string containing the list output. */ function theme_item_list($items = array(), $title = NULL, $type = 'unordered list') { $class = str_replace('-', ' ', $type);
if (isset($items)) { switch($type) { case 'unordered list': $list .= '<ul>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ul>';
case 'ordered list': $list .= '<ol>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ol>';
case 'definition list': $list .= '<dl>'; foreach ($items as $item) { $list .= '<dt>'. $item .'</dt>'; } $list .= '</dl>'; }
if (isset($title)) { $output .= '<h3>'. $title .'</h3>'; }
$output .= '<div class="'. $class .'">'. $list .'</div>';
return $output; }
Isn't something like this nicer? And, easier to use?
On Wo, 19 oktober, 2005 4:07 pm, Moshe Weitzman zei:
Calm down Steef ... Perhaps you want to submit a patch.
I do not, and I repeat _not_ like this approach!
A nicer solution would imo be:
/** * Return a themed list of items. * * @param $items * An array of items to be displayed in the list. * @param $title * The (optional) title of the list. * @return * A string containing the list output. */ function theme_item_list($items = array(), $title = NULL, $type = 'unordered list') { $class = str_replace('-', ' ', $type);
if (isset($items)) { switch($type) { case 'unordered list': $list .= '<ul>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ul>';
case 'ordered list': $list .= '<ol>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ol>';
case 'definition list': $list .= '<dl>'; foreach ($items as $item) { $list .= '<dt>'. $item .'</dt>'; } $list .= '</dl>'; }
if (isset($title)) { $output .= '<h3>'. $title .'</h3>'; }
$output .= '<div class="'. $class .'">'. $list .'</div>';
return $output; }
Isn't something like this nicer? And, easier to use?
Moshe, I'm sorry if this sounds rude to you. This was never my intention.. Probably the words i choose weren't the right ones to show I'm against this change.. Don't get me wrong, I admire your work and _really_ like the idea of making theme_item_list() a little more flexible, but not the choosen solution.. If you think my solution is better than yours, please let me know or give some feedback. After that I'll start making a patch.. Once again, my apoligize to you if this post express' anger, any form of frustration or made you upset.. Steef
May I suggest a little bit different? /** * Return a themed list of items. * * @param $items * An array of items to be displayed in the list. * @param $title * The (optional) title of the list. * @return * A string containing the list output. */ function theme_item_list($items = array(), $title = NULL, $type = 'unordered list') { $class = ''; $output = ''; if (isset($items)) { switch($type) { case 'unordered list': case 'ul': $list .= '<ul>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ul>'; $class = 'unordered-list'; break; case 'ordered list': case 'ol': $list .= '<ol>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ol>'; $class = 'ordered-list'; break; case 'definition list': case 'dl': $list .= '<dl>'; foreach ($items as $item) { $list .= '<dt>'. $item .'</dt>'; } $list .= '</dl>'; $class = 'definition-list'; break; } if (isset($title)) { $output .= '<h3>'. $title .'</h3>'; } $output .= '<div class="'. $class .'">'. $list .'</div>'; return $output; } Reasons: * E_ALL Compilant * break was missing * good for types 'ul'-like (like I prefer) and 'ordered list'-like (like you prefer) btw... shouldnt there be a way to changing the class name? just a tought... Regards, - Sergio On 10/18/05, drupal-devel-owner@drupal.org <drupal-devel-owner@drupal.org> wrote:
Op 18-okt-2005, om 16:45 heeft drupal-devel@drupal.org het volgende geschreven:
User: dries Branch: HEAD Date: Tue, 18 Oct 2005 14:45:09 +0000
Modified files: /includes theme.inc
Log message: - Patch #32573 by Moshe: added support for ordered lists to theme_item_list().
Links: http://cvs.drupal.org/diff.php?path=drupal/includes/ theme.inc&old=1.261&new=1.262
-- [ Drupal cvs list | http://list.drupal.org/ ]
I do not, and I repeat _not_ like this approach!
A nicer solution would imo be:
/** * Return a themed list of items. * * @param $items * An array of items to be displayed in the list. * @param $title * The (optional) title of the list. * @return * A string containing the list output. */ function theme_item_list($items = array(), $title = NULL, $type = 'unordered list') { $class = str_replace('-', ' ', $type);
if (isset($items)) { switch($type) { case 'unordered list': $list .= '<ul>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ul>';
case 'ordered list': $list .= '<ol>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ol>';
case 'definition list': $list .= '<dl>'; foreach ($items as $item) { $list .= '<dt>'. $item .'</dt>'; } $list .= '</dl>'; }
if (isset($title)) { $output .= '<h3>'. $title .'</h3>'; }
$output .= '<div class="'. $class .'">'. $list .'</div>';
return $output; }
Isn't something like this nicer? And, easier to use?
Steef
--- Stefan Nagtegaal Drupal-Devel@iStyledThis.nl Drupal Development Mailinglist
Thsi sounds weird to me because I learned programming with Pascal so I like long descriptive variable and constant names, but given a function that treat "ordered list" and "ol" identically, I would NEVER use "ordered list." On 10/19/05, Sergio <lsmoura@gmail.com> wrote:
May I suggest a little bit different?
/** * Return a themed list of items. * * @param $items * An array of items to be displayed in the list. * @param $title * The (optional) title of the list. * @return * A string containing the list output. */ function theme_item_list($items = array(), $title = NULL, $type = 'unordered list') { $class = ''; $output = '';
if (isset($items)) { switch($type) { case 'unordered list': case 'ul': $list .= '<ul>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ul>'; $class = 'unordered-list'; break;
case 'ordered list': case 'ol': $list .= '<ol>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ol>'; $class = 'ordered-list'; break;
case 'definition list': case 'dl': $list .= '<dl>'; foreach ($items as $item) { $list .= '<dt>'. $item .'</dt>'; } $list .= '</dl>'; $class = 'definition-list'; break; }
if (isset($title)) { $output .= '<h3>'. $title .'</h3>'; }
$output .= '<div class="'. $class .'">'. $list .'</div>';
return $output; }
Reasons: * E_ALL Compilant * break was missing * good for types 'ul'-like (like I prefer) and 'ordered list'-like (like you prefer)
btw... shouldnt there be a way to changing the class name? just a tought...
Regards, - Sergio
On 10/18/05, drupal-devel-owner@drupal.org < drupal-devel-owner@drupal.org> wrote:
Op 18-okt-2005, om 16:45 heeft drupal-devel@drupal.org het volgende geschreven:
User: dries Branch: HEAD Date: Tue, 18 Oct 2005 14:45:09 +0000
Modified files: /includes theme.inc
Log message: - Patch #32573 by Moshe: added support for ordered lists to theme_item_list().
Links: http://cvs.drupal.org/diff.php?path=drupal/includes/ theme.inc&old=1.261&new=1.262
-- [ Drupal cvs list | http://list.drupal.org/ ]
I do not, and I repeat _not_ like this approach!
A nicer solution would imo be:
/** * Return a themed list of items. * * @param $items * An array of items to be displayed in the list. * @param $title * The (optional) title of the list. * @return * A string containing the list output. */ function theme_item_list($items = array(), $title = NULL, $type = 'unordered list') { $class = str_replace('-', ' ', $type);
if (isset($items)) { switch($type) { case 'unordered list': $list .= '<ul>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ul>';
case 'ordered list': $list .= '<ol>'; foreach ($items as $item) { $list .= '<li>'. $item .'</li>'; } $list .= '</ol>';
case 'definition list': $list .= '<dl>'; foreach ($items as $item) { $list .= '<dt>'. $item .'</dt>'; } $list .= '</dl>'; }
if (isset($title)) { $output .= '<h3>'. $title .'</h3>'; }
$output .= '<div class="'. $class .'">'. $list .'</div>';
return $output; }
Isn't something like this nicer? And, easier to use?
Steef
--- Stefan Nagtegaal Drupal-Devel@iStyledThis.nl Drupal Development Mailinglist
participants (6)
-
Breyten Ernsting -
drupal-devel-owner＠drupal.org -
Earl Dunovant -
Moshe Weitzman -
Sergio -
Stefan Nagtegaal