The attached patch does the following:

1) Eliminates the need for #validate_arguments by expanding the syntax of #validate. What used to be:

  '#validate' => 'max_length',
  '#validate_arguments' => 60,

is now:

  '#validate' => array('max_length' => 60),

Note that the old #validate syntax (a string or array of strings) will continue to work and multiple arguments can be specified with, for example "array(60, false)" instead of "60" above. See the comments for _form_parse_callbacks() for details.

2) Registers an error message returned by a validator.

Benefits:

  • more readable function/argument format
  • same format can also be used for '#filter' property (and others, potentially) which I will soon implement
  • simplifies validators (and increases modularity) by not requiring them to report an error, just return a message
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

adrian’s picture

I absolutely love this ..
+10

For the uninformed, Alex did some great work on the formproc module (alexreisner.com/drupal/modules/formproc), which
was developed for Drupal completely separately from the Forms API (lack of communication, d'oh).

Alex has tons of experience in writing form processing code, and I convinced him to try and get as many of his features for formproc in
before the 4.7 release.

This is a clear boon to the API, making it more readable, and lessening the amount of properties we need.

This mechanism could also be used for menu arguments.

adrian’s picture

Could you port the other callbacks too (specifically execute and process/ after_build).

chx’s picture

Title: form field validation streamlining » #validate, #execute, #process consistency
Assigned: Unassigned » chx
Category: feature » task
FileSize
8.2 KB

As happened with hook_node_info so should happen with these: we allow only array('functionname' => array(arg1, arg2, arg3)) syntax. Note that this adds #process_arguments functionaility, makes form.inc execute less code and makes form.inc shorter.

adrian’s picture

I'm not fond of standardising on one format, as having an => array() always hanging around when a vast vast minority of callbacks ever use parameters (but enough to not just drop the arguments entirely). This reminds me of all the null, null, null, $variable stuff in the old form api.

eaton’s picture

In those normal cases, though, the array() is a default and the user doesn't need to set it. Transparent from the perspective of someone making a simple form. Am I misunderstanding the code?

chx’s picture

If it's simple, all is dandy. Look at filter_form, there you see what Adrian's problem is. However, I think consistency and speed worths an extra => on _very_ rare occasions.

chx’s picture

I meant: an extra => array()on _very_ rare occasions.

Alex Reisner’s picture

Given the wide variety of situations in which callbacks can be used I don't think it's wise to generalize about the frequency of argument-passing. Especially if this is about establishing a global syntax, I vote for the format that results in the most readable code as long as it doesn't impose a tremendous speed penalty. In my judgment my proposed solution is this, but I accept that it's a judgment call and defer to those more familiar with Drupal core than I.

chx’s picture

Here is the discussion that lead to hook_node_info as of now:

[Sat Aug 27 2005] [19:06:15] <chx> but... do you want to always write return array('name' => t('something'), 'type' => 'example_module', 'base' => 'example_module'); instead of return t('something') ???
[Sat Aug 27 2005] [19:06:53] <Dries_> chx: yes, i always want to write that
[Sat Aug 27 2005] [19:07:09] <Dries_> chx: maybe a dozen people have to write that
once :)

And here is the sentence that lead to this patch:

[Mon Nov 28 2005] [17:46:27] <Dries_> chx: programmers have to write less, but in the mean time, we have to write more code, and every single page request (thousand of these per second) have to execute your is_array() checks :P

Same train of thought IMO.

chx’s picture

Title: #validate, #execute, #process consistency » #validate, #submit, #process consistency
FileSize
7.53 KB

Reroll to keep up with head

webchick’s picture

Ok, tested this patch:

Create account/login: both are working fine
Node submit: tried both a page (basic) and a poll (more advanced), and both are working fine.
Checkboxes/Radios: Futzed with the themes in admin/theme and settings saved fine.
Date element: this did NOT work, but a clean HEAD does the same thing: I entered a profile field of "birthdate" and put in a date (Apr 7, 1987 -- and no that's not my birthdate :P) and it got reset to "01/01/1".

So overall, it doesn't seem to break anything that wasn't already broken, and does create consistency among the various form-related things. +1.

chx’s picture

Status: Needs review » Reviewed & tested by the community
Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks.

Richard Archer’s picture

Priority: Normal » Critical
Status: Fixed » Needs review
FileSize
716 bytes

This patch backed out part of Patch #39605 and filters no longer work.

Here's a patch that fixes it.

chx’s picture

Jaza’s picture

Status: Needs review » Reviewed & tested by the community

Before I applied this patch, the 'input formats' fieldset on the node add/edit form shows up empty. After applying it, the fieldset shows up with all options displaying, and when the node is saved, the filtering system functions correctly depending on the selected format, and the selected format is remembered and re-populated on editing.

So in short.. this 1-line patch is ready for core, and should go in ASAP.

Jaza’s picture

Title: #validate, #submit, #process consistency » Input format selection broken on node edit form
Dries’s picture

I committed 39778. This is fixed now? Please mark this issue 'fixed' after testing. Thanks.

Richard Archer’s picture

Status: Reviewed & tested by the community » Fixed

Yes, this has been fixed.

Anonymous’s picture

Status: Fixed » Closed (fixed)