#372563: Rename drupal_set_html_head() renamed drupal_set_html_head() to drupal_add_html_head().
We still have drupal_set_header(), which is totally confusing now. One uses 'add', the other 'set'.
Let's rename drupal_set_header() equally to drupal_add_header().
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | drupal.rename-drupal-set-header.20.patch | 19.73 KB | sun |
| #16 | drupal.rename-drupal-set-header.16.patch | 19.47 KB | sun |
| #13 | 451604-rename-drupal-set-header-rev3.patch | 18.91 KB | brianV |
| #7 | rename_drupal_set_header-rev2.patch | 15.57 KB | brianV |
| #1 | rename_drupal_set_header.patch | 15.41 KB | brianV |
Comments
Comment #1
brianV commentedThis patch should do it.
Comment #2
brianV commentedComment #3
dries commentedI'm not sure it is confusing. What is confusing is that you can't tell weather it is a HTTP header or a HTML head(er). Certainly not critical.
Comment #4
brianV commented@Dries,
'set' and 'add' both work equally well for this function. 'add' was suggested to match the other function.
Perhaps 'drupal_set_http_header()' or 'drupal_add_http_header()' would be more apt?
Comment #5
dries commenteddrupal_add_http_header() works for me.
Comment #6
brianV commentedok - I will reroll a patch with drupal_add_http_header() in a bit.
Comment #7
brianV commentedNew patch
Comment #8
c960657 commentedNote that drupal_set_header() not only adds new headers, but also allows replacing and unsetting headers previously set. drupal_add_http_header() might still be a better name, though.
Comment #9
dries commentedI'd like to get @sun's feedback on the proposed name change.
Comment #11
sundrupal_add_http_header() would work for me as well.
Though - one could also argue that we have
- drupal_chmod() as replacement for PHP's chmod()
- drupal_function_exists() as replacement for PHP's function_exists()
- drupal_mail() as replacement for mail()
- and many more...
So, a different alternative would be to just rename it to drupal_header() - as replacement for PHP's header().
Please also note that in case we rename to drupal_add_http_header(), we should probably also rename drupal_get_header() accordingly.
Comment #12
sunTagging for feature freeze.
Comment #13
brianV commentedNew patch rolled, renames the following functions:
drupal_set_headertodrupal_add_http_headerdrupal_get_headertodrupal_get_http_headerComment #14
brianV commentedComment #16
sunRe-rolled. Should come back green.
Comment #17
chx commentedyay api cleanup!
Comment #20
sun/me stops counting how many RTBC patches he needs to re-roll...
Comment #21
c960657 commentedI like the addition of "_http_" to the function name.
I'm not sure that I like the change from "_set_" to "_add_", though. drupal_add_html_head() simply adds the supplied string to an unstructured variable. The added values cannot be read or changed later on. The drupal_get_header()/drupal_set_header() pair, on the other hand, reads and writes named entries in an array. I think this behaviour is closer related to cache_get()/cache_set() than to drupal_add_html_head().
However, if we decide to stick with the proposed function names, the patch looks RTBC to me.
Comment #22
sunFYI: #552478: Restrict "self-closing" tags to only void elements in drupal_pre_render_html_tag will turn drupal_add_html_head() into a structured array. :)
Reverting status.
Comment #23
dries commentedCommitted to CVS HEAD. Thanks!
Comment #25
c960657 commentedThis change should be mentioned on http://drupal.org/node/224333.
My permission to edit that page seems to have been revoked.
Comment #26
casey commentedDocumentation is critical isn't it!?
Comment #27
catch#653068: Update documentation is incomplete
Comment #28
brianV commentedDocumentation has been added to http://drupal.org/update/modules/6/7.
Comment #29
brianV commented