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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Version: 7.x-dev » 8.x-dev
jn2’s picture

Is 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?

sun’s picture

Issue tags: +DrupalWTF

yes, 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 ;)

sun’s picture

Title: Remove underscores from Form API properties and rename hook_forms() to hook_form_info() » Consistent array key syntax in $form_state (underscores vs. spaces)
jn2’s picture

Status: Active » Needs review
FileSize
3.91 KB

Here's a patch for D8 for the complete_form key.

Status: Needs review » Needs work

The last submitted patch, complete_form_underscore-597108-5.patch, failed testing.

jn2’s picture

Status: Needs work » Needs review
FileSize
4.56 KB

Hehe. 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.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Coolio :)

catch’s picture

Issue tags: +API change
xjm’s picture

Tagging issues not yet using summary template.

xjm’s picture

Summary added.

xjm’s picture

Issue summary: View changes

Updated issue summary.

xjm’s picture

Issue summary: View changes

Added note to update docs.

Dries’s picture

Version: 8.x-dev » 7.x-dev

Committed to 8.x. Moving to 7.x for Angie to consider.

webchick’s picture

Version: 7.x-dev » 8.x-dev
Status: Reviewed & tested by the community » Fixed

It'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.

webchick’s picture

Status: Fixed » Needs work
Issue tags: +Needs change record

Oops. This needs a quick http://drupal.org/node/add/changenotice node.

jn2’s picture

Status: Needs work » Fixed
Issue tags: -Needs change record

Change notice done.

jhodgdon’s picture

Status: Fixed » Needs work

The 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?

sun’s picture

The 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.

jhodgdon’s picture

Status: Needs work » Fixed

OK. I've edited the change notice to add this other issue to it, and remove the notice about doxygen. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -DrupalWTF, -API change, -API clean-up

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

Anonymous’s picture

Issue summary: View changes

Added link to docs and ref. to other issue.