Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center./**
964 * Send the HTTP response headers previously set using drupal_add_http_header().
965 * Add default headers, unless they have been replaced or unset using
966 * drupal_add_http_header().
967 *
968 * @param $default_headers
969 * An array of headers as name/value pairs.
970 * @param $single
971 * If TRUE and headers have already be sent, send only the specified header.
972 */
973 function drupal_send_headers($default_headers = array(), $only_default = FALSE) {
974 $headers_sent = &drupal_static(__FUNCTION__, FALSE);
975 $headers = drupal_get_http_header();
976 if ($only_default && $headers_sent) {
977 $headers = array();
978 }
979 $headers_sent = TRUE;
As the title, there may be a typo in the bootstrap.inc file. We believe that the second param should be unified with the real param.
So i suggest they are both "$default_single".
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | drupal-typos_in_docs_for_drupal_8-1853050-12.patch | 1.26 KB | Jerenus |
| #10 | drupal-typos_in_docs_for_drupal_8-1853050-10.patch | 1.17 KB | Jerenus |
| #9 | drupal-typos_in_docs_for_drupal_8-1853050-9.patch | 1.01 KB | Jerenus |
| #7 | drupal-typos_in_docs_for_drupal_8-1853050-7.patch | 1023 bytes | Jerenus |
| #4 | drupal-fix_typo-1853050-4.patch | 842 bytes | Jerenus |
Comments
Comment #1
Jerenus CreditAttribution: Jerenus commentedHere is my patch.
Comment #2
Jerenus CreditAttribution: Jerenus commentedComment #3
Jerenus CreditAttribution: Jerenus commentedSorry about my mistake, here is the new one.
Comment #4
Jerenus CreditAttribution: Jerenus commentedMay be you like "default", new one ;)
Comment #6
jhodgdonThat patch in #4 looks fine... but could we also fix the typos in the next line down? "headers have already *been* sent... the specified header*s*". That will most likely also cause the line to need wrapping to 80-character maximum. Thanks!
Also, we need to fix this in Drupal 8.x first, then backport the fix to 7.x.
Comment #7
Jerenus CreditAttribution: Jerenus commentedjhodgdon,
Here is my new patch for Drupal 8.x, please check.
Thanks!
Jerenus
Comment #8
Lars Toomre CreditAttribution: Lars Toomre commentedBoth of the function parameters have optional values. Hence, each of the description lines should start with '(optional)' and include a statement of what the default value is. Each @param directive also could use a variable type hint.
I am not at a development machine right now, but if this is not re-rolled before late tomorrow, I will roll a patch with the above changes.
Comment #9
Jerenus CreditAttribution: Jerenus commentedjhodgdon,
Sorry for missing your last piece of advice. Here is new one.
Thanks!!
Jerenus
Comment #10
Jerenus CreditAttribution: Jerenus commentedLars Toomre,
Here is additional one for your advice(#8).
Thanks,
Jerenus
Comment #11
jhodgdonRE #8 - Starting with (optional) - Yes. Including information on the default values - No, unless it is not obvious from the function signature.
So on the patch in #10 - you can take out the "Defaults to FALSE". Also, if you're going to put the type "bool" into one parameter, maybe put the type "array" in for the other one? Thanks!
Comment #12
Jerenus CreditAttribution: Jerenus commentedjhodgdon,
Here is the new one, please check.
Thanks!
Jerenus
Comment #13
jhodgdonThis is much better, thanks!
But... the parameter names for this function are quite confusing, since they have "default" in the name, which doesn't make any sense. Now that we have clear documentation about what the parameters *do*, it is quite obvious that the names of the parameters are bad. Maybe we should change the names of the parameters to $additional_headers and $only_additional, or something like that? As they are only parameter names and local variables, that is considered "documentation" and can be done in this patch...
I also think that the previous paragraph of documentation is confusing... well I can't figure out what this is trying to say, given the code and the rest of the documentation:
" Default headers are not set if they have been replaced or unset using drupal_add_http_header()."
Any guesses? If you think these additional changes are out of scope for this issue, then we can commit the docs fix (which is good!) and work on those other problems in a different issue. Thoughts?
Comment #14
jhodgdonOK... I went ahead and committed the patch in #12 to both 8.x and 7.x. Thanks for your work on this patch!
I'd like to leave this issue open though. What I'd like to see happen:
- Make sure the whole documentation for this function makes sense
- Make sure the parameter names make sense
In my opinion, neither of these is true at this time (see comment #13).
Comment #15
Jerenus CreditAttribution: Jerenus commentedjhodgdon,
Sorry about our jet lag. $only_additional is good. If you want to improve this patch, i'd like to do. But, can you change the author name of the patchs to my name? Thank you very much!
Jerenus
Comment #16
jhodgdonWhat are you talking about on the "author name of the patches"? Both of the commits have your user name in the commit message (which is how we attribute patches in Drupal Core):
http://drupalcode.org/project/drupal.git/commit/11fafe6
http://drupalcode.org/project/drupal.git/commit/2b6e2c1
Comment #17
skyredwang@jhodgdon
he was saying that: his patch contains author meta data, but after you committed it, the author was changed to you. Please see the commit msg, both the author and the committer was you. You probably didn't simply apply his patch, which was the correct work flow. You might have just edited the file and committed; this way, git doesn't credit the people.
For more info: http://www.mcdruid.co.uk/content/git-commit-author-give-credit-where-cre...
Comment #18
jhodgdon@skyredwang: In the Drupal Core project, we do not use author attribution for git patches, as a general rule.
To quote @webchick, replying to another user by email a few days ago:
I believe [this user] might be referring to giving credit via the --author flag, e.g.
git commit --author=[username]@[uid].no-reply.drupal.org
This makes people show up here: http://drupal.org/node/3060/committers as well as in Git log.
To date, however, we do not employ this method of commit credit in Drupal core, however, because in nearly all cases [...], a patch is the cumulative effort of several people: someone to do the initial patch, one or more people to provide feedback on it, maybe more people doing re-rolls and adding tests, etc. Unfortunately, Git's "author" flag only supports single values, not multiple values. :\ The people who are listed on that page are generally people who've done major "forklift" operations into core for major initiatives that were merged in from an external sandbox.
So Jennifer is correct that in Drupal core, we do this via comma-separated names in the commit message itself, and parse these out using various tools, such as stats.marvil07.net/drupal-core/. This is more fair overall to everyone.
Comment #19
Jerenus CreditAttribution: Jerenus commentedjhodgdon，
Thanks for your patience and good attitude. Anyway, i will continue to dedicate my efforts to the community. Any new idea about the #13? Or i just try to patch as you said above? Or you want to keep the present?
Thanks!
Jerenus
Comment #20
jhodgdonThe patch in #12 has been committed, and I would like to see a change as noted in #13/14... Do you have a different idea? I'm definitely open to suggestions. It would be good if this function's code actually made sense. :)
Comment #33
quietone CreditAttribution: quietone as a volunteer commenteddrupal_send_headers was deprecated in Drupal 8.x-dev, see #1984766: Change notice: Start relying on Request/Response objects for cache handling and removed in Oct 2014 in #2362123: drupal_page_header() and drupal_send_headers() are dead code: already deprecated, zero uses remain: remove. There is no need to update the documentation.
I think closing as outdated is the best choice here.