Remove obsolete $extra argument in theme_filter_tips()
sun - December 3, 2008 - 00:54
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | filter.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
theme_filter_tips() takes an argument $extra that is never used and can be safely removed.
I've searched through all files in core, and it's only invoked by Filter module itself.
| Attachment | Size |
|---|---|
| drupal.theme-filter-tips.patch | 2.12 KB |
| Testbed results | ||
|---|---|---|
| drupal.theme-filter-tips.patch | passed | Passed: 7474 passes, 0 fails, 0 exceptions a href=http://testing.drupal.org/pifr/file/1/drupal.theme-filter-tips.patchDetailed results/a |
#1
If this really has no use, we should remove it from the theme function declaration in hook_theme().
#2
Boy, I needed to lookup multiple functions to grasp what's actually passed there in $tips.... so I added an example.
#3
$extra = '<p>' . l(t('More information about formatting options'), 'filter/tips') . '</p>';- $tiplist = theme('filter_tips', $tips, FALSE, $extra);
+ $tiplist = theme('filter_tips', $tips, FALSE);
Note that $extra is now an unused variable. It is, after all, declared. It looks like the the "More information about formatting options" gets lost now?
#4
Good catch, but only true for filter_admin_format_form(). Fixed the appending of the more link there, which was not displayed until now, and also did not use the corresponding theme function...
Please note that we most probably want to refactor the whole processing and theming of filter tips (and the ($extra) more link) anyway, but let's discuss this in a new issue and not this one, please. Code clean-up should come first.
#5
I think this is a good patch, although the @code example tends to raise questions (e.g. what is ID for?). I'm not mandating we add more documentation, so I'll commit this after someone else has reviewed this.
#6
I agree that ID should be explained as well. Hopefully ready to go now.
#7
Fixed typo "intented".
#8
Committed it to CVS HEAD. The extra documentation pushed it over the threshold. ;-)
#9
Automatically closed -- issue fixed for two weeks with no activity.