[drupal-devel] [task] New drupal forms api.

[moshe weitzman]

Issue status update for 
http://drupal.org/node/29465
Post a follow up: 
http://drupal.org/project/comments/add/29465

 Project:      Drupal
 Version:      cvs
 Component:    base system
 Category:     tasks
 Priority:     critical
 Assigned to:  adrian
 Reported by:  adrian
 Updated by:   moshe weitzman
 Status:       patch (code needs work)
 Attachment:   http://drupal.org/files/issues/form_defaults.patch (2.39 KB)

Um, snippits and macros are not a substitute for function complete.
Snippits and macros are static entities which you manually create in
the IDE and are then available as needed. If syntax changes for an
given snippit, you have to manually change it. Thats just annoying
enough to make you not use these feature at all. By contrast, function
autocomplete just works the second you open a file. If you start
working on a Contrib module you never seen before, the IDE introspects
and is immediately ready for action. So all that nice PHPDoc that
Adrian has written would be nicely used. Not so with snippits or
macros.


If you guys want to see what the fuss about IDE is, download the free
trial of Zend Studio (http://www.zend.com/store/products/zend-studio/)
or Komodo (http://www.activestate.com/Products/Komodo/ - note the small
link for the OSX alpha. I've tried the alpha and it works). 


IDEs take a little while to get configured and get comfortable ...
Create a project and start typing some common functions. See how the
params show up and all our PHPDoc is there for your use. You can also
quickly navigate your project via function names, and skip to the spot
where a given function is defined. It is a tremendous time saver, and
bug preventer. The IDEs above also offer a debugger which lets you step
through your code and set breakpoints. Want to know the current value of
a variable - just hover over it in your debugger. Once you've gotten the
hang of this, you will never debug using print statements again.


I see nothing wrong with designing Drupal so that it uses those PHP
language features that are friendly to IDEs. Namely, functions,
classes, constants, etc. Arrays are very flexible, but that flexibility
comes at a price.




moshe weitzman



Previous comments:
------------------------------------------------------------------------

Tue, 23 Aug 2005 11:34:00 +0000 : adrian

Attachment: http://drupal.org/files/issues/form.inc (20.53 KB)

This is the first check in of the new forms api code.


The system has been designed to co-exist with the current forms api,
and is contained in a new
include file (includes/form.inc).


Forms are now defined in their component arrays, similar to how menu
items are defined.


example :

<?php
 $form['body'] = array(type => 'textarea', default_value =>
$node->body, cols => 60, rows => 60);
?>


Elements can also be nested, and the $edit follows this definition. For
instance :

<?php
 $form['author'] = array(type => 'fieldset', title => t('Authoring
information'), collapsible => TRUE, collapsed => TRUE, weight => -1);
 $form['author']['name'] = array(type => 'textfield', title =>
t('Authored by'), maxlength => 60,
                                 autocomplete_path
=> 'user/autocomplete', default_value => $node->name, weight => -1);
?>




All the properties used are defined as constants, and are documented
for each of the elements, and individually.




------------------------------------------------------------------------

Tue, 23 Aug 2005 11:46:19 +0000 : adrian

A patch for node.module, blog.module and taxonomy.module that changes
them to use the new form format. This patch is very far from complete,
but I wanted to get the code out so that i'm not working alone anymore.




------------------------------------------------------------------------

Tue, 23 Aug 2005 12:08:01 +0000 : adrian

Attachment: http://drupal.org/files/issues/forms.patch (9.98 KB)

The actual patch =) 


I forgot to mention, this adds a new hook .. namely hook_elements,
which allows us to define the defaults for the elements (ie : cols and
rows for textareas) meaning they don't have to be defined for each of
the elements.




------------------------------------------------------------------------

Tue, 23 Aug 2005 12:09:11 +0000 : chx

A few notes from my conversation with adrian. valid => array('integer',
'uid') for this to work you need function valid_integer($element) and
valid_uid($element). $extra for form_select is legacy and really
needed.




------------------------------------------------------------------------

Tue, 23 Aug 2005 12:39:13 +0000 : fago

i really like this approach.


further i'd like to see the possibility to define an additional class
to a form element, which is currently not working. so we 'd have to
bring _form_get_class() and drupal_attributes() together.




------------------------------------------------------------------------

Tue, 23 Aug 2005 12:56:31 +0000 : adrian

that works already.



<?php
  $form[attributes]['class'] = 'someclass';
?>




Although I am considering just adding a class property ...


ie:

<?php
  $form[class][] = 'someclass';
?>




The fact that this is done via arrays, it means that the developer can
add classes as he or she sees fit.




------------------------------------------------------------------------

Tue, 23 Aug 2005 13:29:10 +0000 : fago

really?
i don't think so.


e.g.
 $checkbox = '<input type="checkbox" class="'.
_form_get_class('form-checkbox', $element[required],
_form_get_error($element[name])) .'" name="'. $element[name] .'" id="'.
$element[id].'" value="'. $element[return_value] .'"'. ($element[value]
? ' checked="checked"' : '') . drupal_attributes($element[attributes])
.' />'


so we will end up with two class attributes, which won't work and isn't
standard compliance.
your css property idea would be ideal imho.




------------------------------------------------------------------------

Tue, 23 Aug 2005 13:40:12 +0000 : nevets

Minor point on #5 and #6, when accessing an associated array like
$form[class][] = 'someclass';
if the key is a string it should be enclosed in quotes, i.e.
$form['class'][] = 'someclass';
(This is from the PHP documentation.)




------------------------------------------------------------------------

Tue, 23 Aug 2005 14:35:29 +0000 : moshe weitzman

Does this API affect form validation also? Thats the vague impression I
had in my head, but I don't see any validation changed in node or
taxonomy modules. perhaps that part is coming next.


There are reasons to love this patch. But one thing I don't like is the
movement toward arrays and away from functions. Modern editors and IDE's
offer function tips and function completion. These are huge time and
brain savers. They are great for newbies and for experts. It is so
helpful to just type 'form_sel', press tab, and have
form_select('title', 'name', 'value', 'options') printed for you, with
all the arguments. When you define forms in an array instead of
functions, as proposed in this patch, you lose a lot of developer
productivity and friendliness for newbies. Developers are also more
prone to mistakes this way since the editor can't guide them along.


This is the sort of advantage that means nothing to the many people who
just use a plain editor, and means everything to IDE users. Maybe
someone can think of a way to keep the flexibility without losing IDE
productivity.




------------------------------------------------------------------------

Wed, 24 Aug 2005 08:45:53 +0000 : adrian

The api has a drupal_validate_form() function, which does the following
validation :


It steps through each of the elements, and executes any of the valid
properties. An example would be valid => 'username'.
It then calls valid_username($element), which can check for errors. 


It then calls $form_id_validate() , which can check for errors between
form elements.


It then (optionally) calls $callback_validate(), which allows you to
have unique form id's , similar to how the example does the node form. 


You could create a function $type_node_form_validate(), to validate
only that form, or a theme_$type_node_form() to theme that form
differently.


An example of where this would be used is for CCK, where it will have a
single callback for all nodes created by it.


Errors are flagged using form_error($element). It's different from
form_set_error, in that it also sets the error property of the element,
which I think is more practical than using the globals.


Regarding the IDE discussion, I am on the fence about that, but
definitely leaning towards preferring the arrays over the function
calls. I think that the menu system has proven itself, and that it's
better to be consistent.


The plan for 4.7, is to leave the current form api in , so that all
contrib modules don't need to be ported, but to switch core over to the
new form system anyway, so for the time being .. the old functions will
still exist.


All this would be a non-issue if php supported named parameters, which
is essentially what we are reproducing using arrays.




------------------------------------------------------------------------

Wed, 24 Aug 2005 12:36:08 +0000 : Thomas Ilsche

I agree with moshe, and I think for day to day use the current forms api
does a good job - however on complicated constructions i consider this
to be really useful.
The problem i see is to keep the whole forms api consistent and easy to
learn, any ideas?
I'd be against deprecating the current functions.


About the keyword definitions. I think it should be consistent with for
hook_menu and all its "named parameter" friends, and to at least not
confuse it more define the keywords without the leading underscore.




------------------------------------------------------------------------

Wed, 24 Aug 2005 13:20:13 +0000 : adrian

I'd prefer to make the menu system properties be consistent with the
form system. actually.


I know the conventional logic is that all constants being uppercase,
and the first versions of the form code did stick to that, but the end
result
of the lowercase constants was far more readable code (the underscores
however are a necessity to allow for nesting.)


What i was thinking was, that we could use the conventional form api as
constructors for the form array.


ie:

<?php
$group .= form_textfield(t('File system path'), 'file_directory_path',
$directory_path, 60, 255, t('desc here'), null, etc)
?>


turns into

<?php
 $form['files']['file_directory_path'] = form_textfield(t('File system
path'), $directory_path, 60, 255, t('desc here'), null, etc);
 $form['files']['file_directory_path'][valid] = 'directory'; // any
other properties that aren't in constructors.
?>


instead of

<?php
  $form['files']['file_create_path'] = array( type => 'textfield',
title => t('File system path'), default_value => $directory_path,
maxlength => 255, valid => 'directory', description => t('desc
here')  );
?>




and form_textfield turns into

<?php
  function form_textfield($title, $value, $size, $maxlength,
$description = NULL, $attributes = NULL, $required = FALSE) {
    return array( title => $title, size => $size, maxlength =>
$maxlength, description = $description, attributes => $attributes,
required => $required);
  }
?>




Benefits :
1) easier to port
2) the ide thingy


Drawbacks :
1) more than one way to do something.
2) all forms will need to be upgraded, since the core form api will
change. ie: breaks all of contrib.
3) Constructors could become unwieldy trying to tend to most of the
parameters that can be set (weight, valid, validation_arguments, etc)




------------------------------------------------------------------------

Wed, 24 Aug 2005 13:47:17 +0000 : Dries

There should only be one way to build forms.  Simplicity and uniformity
is king.  For now, I leave it up to Adrian to decide what this "one
way" is going to look like. (I like his initial approach.  The code is
shorter which saves time too.)




------------------------------------------------------------------------

Wed, 24 Aug 2005 14:24:48 +0000 : chx

I second Dries. The old form API be gone. The IDE is going to be a
problem, yes. A possible approach: the default array have all keys
possible set to NULL or so.




------------------------------------------------------------------------

Wed, 24 Aug 2005 14:28:50 +0000 : Bèr Kessels

I prefer the One Way too. having more ways to do something normally
results in two half-witted ways, instead of one way that works As Best
As Possible. 


I like the arrays approach. I love it, in fact. I have a feeling that
the more code in drupal adopts the Array Way [tm], the more power AND
uniformity we get. Just look at the success of array based menus:
powerfull, yet simple to develop with.


But, I have a few hesitations: one is the lowercase CONSTANTS. I know,
this is good for readability, so I lean towards the side of: then just
use lowercase constants. But still, something does not feel right about
it. I think this needs some more though, or comments of others. 


Another thing I dislike is the way we use parameters to construct, IMO,
completely different widgets. We should try to not think in terms of
HTML, but in terms of usage and display. In HTML a collapsible form is
similar -or nearly- to a noncollapsible. Same goes for a multi-select
and none-multiselect select.
But where I really think we should have different APIs is for
autofills. They are IMHO not textfields, but a complete separate
widget. Thus they should get a separate API.


And last about the IDEs: allthough I do understand the problem, i
beleive it is a very bad habit to let your code/application/product be
limited, because of the tools you use. If your tools cannot handle
libraries/snippets/etc beyond some function
calls, IMO you should get anoter IDE :). But surely we should not let
us be held back by these limitations in these IDEs.