API page: http://api.drupal.org/api/drupal/includes!common.inc/function/drupal_get...

Enter a descriptive title (above) relating to drupal_get_destination, then describe the problem you have found:

There's no @return.

Also, this is not quite true on D7:

> Prepares a 'destination' URL query parameter

What is returned is a query array *containing* a destination, rather than a portion of a query parameter.

See this comment for details: http://api.drupal.org/api/drupal/includes!common.inc/function/drupal_get...

Comments

edb’s picture

Status: Active » Needs review
StatusFileSize
new630 bytes

Attached a patch that adds the return statement documentation. Thoughts on the wording?

jhodgdon’s picture

Status: Needs review » Needs work

Thanks for the patch!

The list needs a bit of formatting work -- see
http://drupal.org/node/1354#lists

Looking at the code, I also don't think this documentation is accurate. What is returned in the array seems to be either $_GET['destination'] or current_path() with a query string appended if there is one.

albert volkman’s picture

Status: Needs work » Needs review
StatusFileSize
new611 bytes
new649 bytes

Cleaned up formatting errors and altered the return doc a bit.

edb’s picture

StatusFileSize
new651 bytes

Yep that's much clearer documentation, thanks Albert. However, I think 'querystring' should be two words.

jhodgdon’s picture

Status: Needs review » Needs work

"desination" is misspelled. :) Other than that, I think it's OK... maybe could use a comma in there before the "or"?

albert volkman’s picture

Status: Needs work » Needs review
StatusFileSize
new652 bytes

Ooops. Fixed :)

Status: Needs review » Needs work

The last submitted patch, drupal_get_destination_docs-1722244-6.patch, failed testing.

jhodgdon’s picture

The test failure above is not related to this patch...

But I'm still not sure about this documentation:

+ * - destination: The path provided via the destination query string or the
+ * current path complete with query string if available.

This doesn't tell me which one (destination query string or current path) takes precedence, and I also think that it could use a comma before "or", and I'm not sure what "if available" really means -- if what is available?

albert volkman’s picture

If the path includes a query string, it'll be included. I suppose that doesn't need to be explicitly stated, so perhaps it should just be "The path provided via the destination query string or, if not available, the current path." Not sure about the structure of that sentence... is it clear that the query string is the subject of "if not available"?

jhodgdon’s picture

yes, that seems very clear to me. Thanks!

albert volkman’s picture

StatusFileSize
new631 bytes

You're welcome!

albert volkman’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks good! I'll get it committed shortly (probably the patch will work for 7.x also).

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x and 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

David_Rothstein’s picture

Version: 8.x-dev » 7.x-dev
Status: Closed (fixed) » Reviewed & tested by the community

This was committed to Drupal 7, but for some reason was reverted later: http://drupalcode.org/project/drupal.git/commitdiff/549ced20094cc361f790...

I assume that was by mistake... but it's pretty weird.

I could fix it myself, but I'll leave it to another core committer to confirm that I'm not going crazy :)

jhodgdon’s picture

You are not going crazy (at least on this issue; I cannot vouch for your overall sanity. :) )...

The patch is still in 8.x, but there is no @return in 7.x and the patch needs to be reapplied. I suspect there was a merge issue or some other git mistake. I'll get it re-committed shortly.

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Committed (again) to 7.x.

Automatically closed -- issue fixed for 2 weeks with no activity.