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.In #1315992: No standard for documenting menu callbacks, we decided on a new standard for menu callbacks (cf. http://drupal.org/node/1354#menu-callback ), including
After a blank line, tell what path or paths it pertains to. If there is more than one, format it as a list. If a function is used for many paths that can't be enumerated, you use the router path wildcard %: admin/help/% or node/%node.
and
At the end, include an @see line that points to the hook_menu() implementation where this callback is registered. If multiple hook implementations use the callback, include one @see line for each.
Now, there is the pervasive pattern of using drupal_get_form as a menu callback and passing the form id as a parameter. This means that the respective form constructor basically functions as the menu callback.
Suggestion: Adopt the two requirements mentioned above for form constructor functions that are referenced in hook_menu() declarations (which is most of them).
That is, document form constructor functions that are referenced in hook_menu() implementations as follows:
/**
* Form constructor for the foo bar form.
*
* Path: admin/foo/bar
*
* [possible parameter documentation]
*
* @see foo_menu()
Rationale: The nice thing about the new standard is that it is very easy to grep for a certain path and find the function that is responsible for it. Except if the page is displaying the form, in which case this documentation is missing---the drupal_get_form() pattern is general enough to warrant a special doc standard.
Comments
Comment #1
xjmI think this pattern makes sense. You're right and it's not actually a page callback. Some confirmation forms are exceptions, but form constructors generally it's probably redundant and not entirely correct (since
drupal_get_form()is actually the page callback).This would mean rerolling the poor
node.modulepatch again, though. :)Comment #2
jhodgdonThe pattern I think we've been using in other patches is:
Page callback: Form constructor for the xyz.
Yes, I realize that technically the page callback is drupal_get_form(), but that gives you one more bit of information in the one-line description...
Comment #3
xjmHmm, I disagree, because the return value will be different from other page callback return values, so it doesn't fit the pattern. Page callbacks return markup, but form constructors return FAPI arrays. It's a minor point either way. I do think having the path is a good idea.
Comment #4
jhodgdonOK... can we think of another notation to put in the first line that would indicate that not only is this a form callback, but that it's a special form callback that's used as a page in hook_menu(), or do you think that is not necessary, since I guess many/most (??) forms are?
Comment #5
jhodgdonComment #6
sven.lauer CreditAttribution: sven.lauer commentedAs I mentioned in that other issue, I agree that it is factually incorrect to call these "page callbacks" (well, except that we can, of course, redefine the term ...).
And I also think that it is sort of unnecessary, since, really, most (almost all?) form constructors actually act as "pseudo page-callbacks". So adding the "Path: ..." would be enough to distinguish those that are (have a path) from those that do not (don't have a path).
Plus, "Page callback: Form constructor for the x form" just takes a LOT out of the 80 character space that could be filled with more useful information.
Comment #7
xjmYeah, I agree that just having the path is probably sufficient.
Comment #8
jhodgdonOK. So our standard would be just to add the Path: line?
Comment #9
sven.lauer CreditAttribution: sven.lauer commentedBoth the path and the @see to the hook_menu implementation, I'd say.
Comment #10
xjmYeah, #9 sounds good to me.
Comment #11
jhodgdonOK. I think this is consistent with our other standards, so it probably doesn't merit more discussion. I've added this to the hook_menu and form sections of node/1354 and we can consider this adopted.