The value returned from preg_match(), which then returned from drupal_match_path(), is not (strictly speaking) a boolean. The documentation is then not correct when it says that the possible values returned from the function are TRUE, and FALSE.

Comments

jhodgdon’s picture

Title: Documentation problem with drupal_match_path() » drupal_match_path() return value doc incorrect
Issue tags: +Novice

Good catch! The doc is correct in Drupal 7 (there is a cast to Boolean), but not Drupal 6 (where it will return 0 or 1).
http://api.drupal.org/api/function/drupal_match_path

JuliaKM’s picture

Status: Active » Needs review
StatusFileSize
new691 bytes

Here is a patch to update the Drupal 6 documentation for drupal_match_patch to reflect that the returned value is 0 or 1 and not a boolean.

Please note that the returned value in Drupal 7 is boolean.

This patch was created at Triangle NC Hack Night. One computer may have been harmed in the creation of this patch :(

jhodgdon’s picture

Status: Needs review » Needs work

This is sort of OK as-is, but normally we don't say "The return value is" in the @return section. When this is displayed on api.drupal.org, @return becomes "Return value", so normally we would just start the @return section describing what the return value is, rather than with "The return value is".

One other issue: There can be several patterns passed into the $patterns arg, so really it's 1 if any of the patterns matches. Your patch says "the pattern", which is misleading.

avpaderno’s picture

It would be better to say that the return value is 1 if there is a match, and 0 if there isn't a match. At least it would not make assumptions about the number of patterns passed.

JuliaKM’s picture

Status: Needs work » Needs review
StatusFileSize
new657 bytes

Thanks jhodgdon and kiamlaluno. Here's a re-rolled patch. The new text says:
"1 if there is a match, 0 if there is not a match."

jhodgdon’s picture

StatusFileSize
new656 bytes

Looks good to me, except I think there is an extra space at the end of the line you added. Patches have been held up for as little as that before. :)

Here's a new patch, with that fixed.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community
gábor hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed.

Status: Fixed » Closed (fixed)
Issue tags: -Novice

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