Problem/Motivation
$form_state['complete form']
uses a space between words. At present, all other form properties use an underscore between words, e.g.:
$form_state['always_process']
.
(Note: The original post also suggests renaming hook_forms()
. This suggestion has been moved to #597280: Introduce form registry to fix various problems and increase security.)
Proposed resolution
Rename the complete form
parameter to complete_form
for consistency.
Remaining tasks
Update FAPI documentation when patch is accepted. There is one reference to complete form
in the FAPI reference. An additional reference is in the patch in #859970: Cleanup form_api $form_state docs - keys in 2 places, missing some.
User interface changes
None.
API changes
FAPI property renamed from:
$form['complete form']
$form_state['complete form']
to:
$form['complete_form']
$form_state['complete_form']
Original report by @sun
Spin-off from #571086: Allow to specify a 'wrapper callback' before executing a form builder function:
Some properties in $form and $form_state use a underscore, some others use a space. For example:
- $form_state['always_process'] - http://api.drupal.org/api/function/drupal_build_form/7
- $form_state['complete form'] - http://api.drupal.org/api/function/form_builder/7
- $forms[$form_id]['callback arguments'] - http://api.drupal.org/api/function/hook_forms/7
Proposal:
1) All properties in $form should use underscores.
2) All properties in $form_state and hook_forms() should use spaces.
3) hook_forms() is a half-baked run-time form registry. We should rename that to hook_form_info() for consistency.
Comment | File | Size | Author |
---|---|---|---|
#7 | complete_form_underscore-597108-7.patch | 4.56 KB | jn2 |
#5 | complete_form_underscore-597108-5.patch | 3.91 KB | jn2 |
Comments
Comment #1
sunComment #2
jn2 CreditAttribution: jn2 commentedIs this issue still alive? If so, I'd like to help. Also:
See #859970: Cleanup form_api $form_state docs - keys in 2 places, missing some. If you look through the complete list of $form_state keys in that patch, you'll see that 'complete form' is the only key without underscores. I counted 10 $form_state keys with underscores. Is there a reason it would be preferable for the $form_state keys not to use underscores?
Comment #3
sunyes, this is still an issue. Of course, the proposal is open for discussion. If there's really only one remaining key in $form_state that does not use underscores, then it probably makes sense to change only that one instead of all others ;)
Comment #4
sunNote that 3) of the proposal is a different issue: #597280: Introduce form registry to fix various problems and increase security
Comment #5
jn2 CreditAttribution: jn2 commentedHere's a patch for D8 for the complete_form key.
Comment #7
jn2 CreditAttribution: jn2 commentedHehe. All that grepping, and I still missed one. At least it was easy to see from the test results where I needed to look. This one should have caught them all.
Comment #8
sunCoolio :)
Comment #9
catchComment #10
xjmTagging issues not yet using summary template.
Comment #11
xjmSummary added.
Comment #11.0
xjmUpdated issue summary.
Comment #11.1
xjmAdded note to update docs.
Comment #12
Dries CreditAttribution: Dries commentedCommitted to 8.x. Moving to 7.x for Angie to consider.
Comment #13
webchickIt's possible for contrib modules to be using the old key name in hook_form_alter()/form wizard implementations, so I think we need to keep this api clean up D8 only.
Comment #14
webchickOops. This needs a quick http://drupal.org/node/add/changenotice node.
Comment #15
jn2 CreditAttribution: jn2 commentedChange notice done.
Comment #16
jhodgdonThe API change notice states that some doxygen doc needs to be cleaned up for this patch. Shouldn't that have been done here on this issue? If not, why not -- I thought patches were supposed to take care of their documentation?
And which doxygen doc still needs to be cleaned up?
The change nodes are supposed to indicate which online docs and other modules need to be updated, not which follow-up patches need to be done in core, right?
Comment #17
sunThe Doxygen things shouldn't have been mentioned in the change notice. The Doxygen was supposed to be changed as part of #859970: Cleanup form_api $form_state docs - keys in 2 places, missing some, which actually should have landed first. :(
So indeed, after aforementioned issue landed, we need a follow-up patch here to fix the phpDoc.
Comment #18
jhodgdonOK. I've edited the change notice to add this other issue to it, and remove the notice about doxygen. Thanks!
Comment #19.0
(not verified) CreditAttribution: commentedAdded link to docs and ref. to other issue.