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

chx drupal-devel at drupal.org
Wed Sep 14 01:00:36 UTC 2005


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:   chx
 Status:       patch (code needs work)

story (and maybe others) has $form['taxonomy'] =
taxonomy_node_form('blog', $node);




chx



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.




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

Wed, 24 Aug 2005 15:31:14 +0000 : moshe weitzman

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.




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

Wed, 24 Aug 2005 16:34:41 +0000 : chx

Attachment: http://drupal.org/files/issues/forms_chx.patch (15.59 KB)

Like we have core.php for hooks we can have form.php for form API and
let that help your hand with any IDE.


Here is an updated version of Adrian's patch. User login block
reworked. I move on to other parts of user.module. I also created a
theme_password() function, so I will post form.inc as well.




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

Wed, 24 Aug 2005 16:49:05 +0000 : chx

Attachment: http://drupal.org/files/issues/form_0.inc (21.01 KB)




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

Wed, 24 Aug 2005 22:23:06 +0000 : gordon

+1 I like this patch a lot. but 2 things.



* The testarea hook is not supported. This needs to be implemented.
* the existing form_*() need to be changed to use the theme_*() so that
there is only one place that form items can be created.

I think this will be great for theme developers and it will be easier
to build forms for module developers.


Great job.




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

Fri, 26 Aug 2005 09:27:08 +0000 : adrian

Attachment: http://drupal.org/files/issues/form_1.inc (24.73 KB)

Here is an updated version of the patch.


1) admin / settings has been rewritten
2) Added form_radios and form_checkboxes and form_select and a few
others
3) Very strict validation now .. the drupal_get_form function
automatically validates any input .. no way to skip that. If it doesn't
produce errors, it calls $form_id_execute(), or the optional
$callback_execute().
4) Started the first part of the implementation of the multiple
keyword, which is when an element can have multiple values. An example
of this would be the 'files' element from upload.module , where more
files get added, and also the primary / secondary links configuration
of phptemplate.


Still needs to be done : The clean_url validation, and any other module
with a _settings hook. I haven't integrated chx' user module work at
this point.




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

Fri, 26 Aug 2005 09:48:14 +0000 : adrian

Attachment: http://drupal.org/files/issues/forms_0.patch (37.68 KB)

Here's the patch.


I'm also still busy with the filter format, which has unique
requirements and needs to be a filter_format element type (gets rid of
the in-line html)




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

Sun, 28 Aug 2005 02:52:59 +0000 : yogadex

One thing I like about the current API is that you can inject arbitrary
HTML in the middle of a form if you see fit.  It's not obvious to me
how to do that with the new API.  Is there a way? 


Could I, for example, arrange my form fields in a table with two or
three columns?




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

Sun, 28 Aug 2005 07:26:13 +0000 : naudefj

I would also like to be able to display forms in HTML tables. Here's a
great article explaining how forms can be styled using tables -
http://www.cs.tut.fi/~jkorpela/forms/tables.html




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

Sun, 28 Aug 2005 08:25:06 +0000 : adrian

EVERY form will now have a theme function, as every form now has it's
own (unique) form-id.


You can create a theme_my_form_id() function if you are a developer,
and require the form to be differently layed out (like for instance,
adding all the element outputs to a table).


You can create a mytheme_my_form_id() if you are a theme developer,
that can override the form layout. (as is shown by the current
node_form)
[?
function theme_node_form($form) {
  $output .= '';
    $output .= '';
      $output .= '';
      $output .= form_render($form['author']);
      $output .= '';


      $output .= '';
      $output .= form_render($form['options']);
      $output .= '';
    $output .= '';
    $output .= '';
    $output .= form_render($form_render);
    $output .= '';
  $output .= '';
  return $output;
}


?]
You can create a phptemplate stub to load it from a template.

<?php
function phptemplate_node_form($element) {
  return _phptemplate_default('node_form', $element);
}
?>


and the current node_form template being :
[?


      
<?php
 print form_render($form['author']) 
?>




      
<?php
 print form_render($form['options']) 
?>




    
<?php
 print form_render($form) 
?>




?]


NOTE: As a developer, you can even remix forms you didn't design.
Infact, it gives you complete themeability over everything.




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

Sun, 28 Aug 2005 08:44:27 +0000 : adrian

Attachment: http://drupal.org/files/issues/forms_presentation.pdf (78.63 KB)

I hate the php filter ..
here's the theme_ function ..
i don't have time to escape the template right



<?php
function theme_node_form($form) {
  $output .= '<div class="node-form">';
    $output .= '<div class="admin">';
      $output .= '<div class="authored">';
      $output .= form_render($form['author']);
      $output .= '</div>';
      $output .= '<div class="options">';
      $output .= form_render($form['options']);
      $output .= '</div>';
    $output .= '</div>';
    $output .= '<div class="standard">';
    $output .= form_render($form_render);
    $output .= '</div>';
  $output .= '</div>';
  return $output;
}
?>




I am attaching my presentation from drupalcon in portland.It has all
the examples, although stuff like the validation has changed slightly.




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

Sun, 28 Aug 2005 16:53:48 +0000 : adrian

Call for help : 


Code freeze for Drupal 4.7 is coming very quickly , and a lot of work
is still needed to get this functionality in before the code freeze
happens.
As this patch is incredibly important (even if only considered from a
security point of view), we need people to help us port all the forms
in
Drupal at the moment.


If anyone is interested in helping, could you please contact me, so we
can coordinate.




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

Sun, 28 Aug 2005 22:19:40 +0000 : killes at www.drop.org

You can assign me a module to conver as soon as my revisions patch is in
core. I think I'd like to convert profile.module.




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

Sun, 28 Aug 2005 23:41:06 +0000 : yogadex

Regarding #24:


Now I see why you keep the [printed] flag during node_render.


Should this line:
$output .= form_render($form_render);
read:
$output .= form_render($form);
??? (That is, I don't see where variable $form_render is coming from).


Regarding #25:


Must every form be converted in order to include this?  Drupal 4.7 will
still support the old forms API, yes?


I see that this was discussed early in the thread.  For those of us
with custom modules life would be a lot easier if the old form api
sticks around until say a 5.0 release.




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

Mon, 29 Aug 2005 00:02:09 +0000 : killes at www.drop.org

@yogadex: To those of us who roll out the actual security releases till
4am in the morning the disappearance of the old api with 4.7 will
provide a lot more sleep. We win, backwards compatibility sucks.




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

Mon, 29 Aug 2005 03:50:27 +0000 : adrian

The old form api will remain in core for a maximum of one release. 


another 6-9 months to convert your modules is more than enough time...
plus most of the new uber features (cck etc) are going to require you
to upgrade to the new system.


Meanwhile, new functionality like the install wizards are going to
require you to use this form api.




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

Mon, 29 Aug 2005 03:59:41 +0000 : adrian

And regarding the form_render question .. it's not a variable, it's a
small recursive function.


the theme function is passed the entire form tree. The designer can
choose to remix the form however he chooses. Since elements are nested,
and for instance the fieldsets, are named .. you can use
form_render($form['elementname']) , which will print that element, and
set the printed flag so it won't be printed again. 


Like in the example where the author and options fieldsets are
seperated out from the form and displayed seperately.
form_render($form) in turn will print anything that hasn't been printed
yet. 


if you wanted to seperate out the author field on the page, you could
use form_render($form['author']['name']) , and it would print that
element wherever, and when you then called form_render($form['author'])
or form_render($form); , it would not be printed again.




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

Mon, 29 Aug 2005 04:24:47 +0000 : yogadex

@killes: I certainly don't want you losing sleep ;)  I'm glad to hear
the new and old will coexist for at least one release.  (Unless there's
a security issue - that's another story)


@adrian: I understand the form_render() function and it's nifty.  But
there is a typo in both your attached presentation and #24 above, where
"$form_render" is written and it should be "$form", if I am reading
correctly.  Thanks for explaining.




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

Tue, 30 Aug 2005 21:14:37 +0000 : chx

Attachment: http://drupal.org/files/issues/forms_1.patch (46.21 KB)

I merged and rerolled against current HEAD.




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

Wed, 31 Aug 2005 13:59:30 +0000 : m3avrck

Minor include bug in the node.module patch:


 define('NODE_NEW_LIMIT', time() - 30 * 24 * 60 * 60);
+include_once 'includes/form.inc';


that include_once is missing the './' part.




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

Thu, 01 Sep 2005 00:25:22 +0000 : ax

that second

<?php
+include_once 'includes/form.inc';
?>


shouldn't be changed but *removed*, because it is a duplicate of 8
lines above.




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

Thu, 01 Sep 2005 00:31:52 +0000 : m3avrck

ah yes, an even better catch than mine, touche!




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

Thu, 01 Sep 2005 01:14:12 +0000 : chx

Not so. The first should be removed 'cos it is hard to patch against the
cvs id :)




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

Thu, 01 Sep 2005 01:42:27 +0000 : ax

allright, ok, of course, the first one.


what would be even better would be a working patch. the ones attached 
(last one and before) don't include form.inc, generate some duplicate
functions that result in "PHP Fatal errors:  Cannot redeclare
functions()" (system_elements(), theme_node_form()), and "PHP Warning: 
Call-time pass-by-reference has been deprecated - argument passed by
value"s. chx: are you working on this?


i would much like to help testing the new forms api and getting it into
4.7 because thats exactly what i need for a bigger project at work where
we are considering using Drupal as base framework. the one showstopper
is the extensability of Drupal forms ...


thanks for all the work done up to now, anyway.




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

Thu, 01 Sep 2005 02:38:08 +0000 : jvandyk

Attachment: http://drupal.org/files/issues/forms_2.patch (47.72 KB)

Here's an updated patch against HEAD that is mostly working. Karoly
provided the updated node.module. For me, there seems to be an issue in
form.inc in the recursive _form_builder() function where it is recursing
even when $element is not an array but is a scalar.




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

Thu, 01 Sep 2005 02:43:01 +0000 : m3avrck

Not to nitpick, but in node.module, the patch has +include_once
'includes/form.inc'; but should read +include_once
'./includes/form.inc'; for consistency and performance :)




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

Thu, 01 Sep 2005 03:24:16 +0000 : webchick

I don't know if this is just the Drupal newbie in me talking, but I am
getting quite a few errors with this patch (Drupal HEAD, up-to-date as
of about 5 minutes ago). I've uploaded the form.inc from #19 and
applied the forms_2.patch from #38, and I'm getting errors like the
following:


*admin/settings:*
warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 511.


*admin/comment/configure, admin/settings/user*:
warning: Missing argument 2 for system_settings_form() in
$DRUPAL_ROOT/modules/system.module on line 733.


*node/add/[anything]*:
warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 406.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 403.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 411.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 406.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 403.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 411.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 406.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 403.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 411.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 406.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 403.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 411.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 406.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 403.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 411.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 435.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 448.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 435.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 448.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 435.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 448.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 435.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 448.


warning: Invalid argument supplied for foreach() in
$DRUPAL_ROOT/includes/form.inc on line 435.


warning: Cannot use a scalar value as an array in
$DRUPAL_ROOT/includes/form.inc on line 448.


---


Any ideas?




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

Thu, 01 Sep 2005 03:35:44 +0000 : jvandyk

That's why I said "mostly working." :)


m3avrck, sorry, I diffed against the copy where I hadn't corrected the
./ thing.




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

Thu, 01 Sep 2005 09:47:05 +0000 : adrian

Good morning guys.
I am looking at those errors now, and i will upload an updated
forms.inc in a short while.




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

Thu, 01 Sep 2005 22:46:02 +0000 : adrian

Attachment: http://drupal.org/files/issues/form_2.inc (26.76 KB)

Here is an updated patch and includes/form.inc.


Changes :
1 ) Introduce a global $form_variables array, which works exactly like
$_POST['edit'] used to, except the values in it are filtered through
the form system and only values that have form elements will be in it.
Each form being processed on a page resets this array, so they won't
interfere with each other.
2) introduce a form_id hidden that gets submitted with the page. This
means it can intelligently decide which form to process.
3) Fix the bug above. I wrapped the offending lines in if is_array() ,
HOWEVER .. at the point I call that code I am fairly certain I am only
working on arrays, hence I need to look it over more
4) fixed the user login form on /user. The user login block still
doesn't work
5) Added a default css class (and some temporary css in drupal.css) for
everything rendered with the form api. All other forms will have a thick
red border if they have not been ported.
6) fixed a bug with element_info where it was setting the wrong
defaults and then messing up the first form element because of it.
7) finished the filter module 'filter_format' element. Filter forms are
printing now.
8) Added the 'weight' element. Could be cool to theme this with
something ajaxy, right ?
9) Fixed some bugs with admin/system
10) Updated the following modules : Book, Page, Story, Blog, Node,
Taxonomy and a fixed user form.
11) Moved the inclusion of form.inc to it's rightful place in
drupal_bootstrap_full.
12) Added a bunch of comments to places. We will still have to go
through all the comments and make sure they all 'parse' =)


The node form is not actually working or validated at present, and if
someone could take a look at that , I'd be much obliged. It's important
to get used to the new (strict) workflow for forms. It generally means
having to shift some code around, and it makes it harder to write 'bad
code'.




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

Thu, 01 Sep 2005 23:05:56 +0000 : adrian

Attachment: http://drupal.org/files/issues/forms_3.patch (54.13 KB)

here's the patch




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

Fri, 02 Sep 2005 05:48:44 +0000 : Dries

/introduce a form_id hidden that gets submitted with the page. This
means it can intelligently decide which form to process./


Does this mean people could change the ID before they submit the form? 
I'm not sure that is a good idea.




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

Fri, 02 Sep 2005 10:04:21 +0000 : adrian

The access check is tied to the page, not to the form.


If the page has 2 forms on the page, this check is to see which of the
two he submitted.


Changing the form_id in the hidden will do nothing but cause the form
not to execute, at all.




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

Fri, 02 Sep 2005 14:04:45 +0000 : m3avrck

Running this patch I'm getting this error Fatal error:
_drupal_bootstrap_full() [function.require]: Failed opening required
'./includes/form.inc'


Didn't see the file anywhere in the patch, unless I missed something...




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

Fri, 02 Sep 2005 14:44:22 +0000 : adrian

drupal.org renames the files to avoid conflicts.


it's attached as form_2.inc




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

Sat, 03 Sep 2005 03:06:21 +0000 : asimmonds

Should there be "return_value" and "delta" property constants defined in
form.inc?




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

Sat, 03 Sep 2005 06:32:57 +0000 : adrian

Attachment: http://drupal.org/files/issues/form_3.inc (27.33 KB)

Well spotted. 


Added those defines.




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

Sat, 03 Sep 2005 11:53:53 +0000 : nicopepe

Attachment: http://drupal.org/files/issues/modif_Form_Inc.php (2.52 KB)

Hello
I am a newsbe and i am vey concern about the form project. I want first
to congratulaed for all this nice work. I propose this code in the
attached file. Tell me what you think, and i'll finished it.




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

Sun, 04 Sep 2005 14:57:30 +0000 : nicopepe

Hello again,
I see no reaction to my proposition. Maybe not interresting code ? My
aim with that code is :


- Open all the possibilities of attribute in an input balise.
- Not multiply the number of constant (The key of an element is an
attribute name, define by the programmer)
- Check within drupal_attributes function if these attributes are
correct in an INPUT Balise.
- Include a javascript code insertion for form client validation.


In fact i would like to create later on,  a module with generic
javascript for client validation (Date checking, required fields, ...).
I have this in mind when i proposed this code.


Any answer this time ?




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

Sun, 04 Sep 2005 15:11:41 +0000 : clydefrog

nicopepe, it is difficult to tell what your changes are because you have
not submitted a patch. Take a look at http://drupal.org/patch. Your code
might get a better response that way.




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

Sun, 04 Sep 2005 18:25:23 +0000 : Dries

As an aside, it might be worth looking into the XForms [1].  They have a
short introduction for HTML authors [2] in case you don't feel like
reading the entire specification. A friend of mine is a member of the
XForms working group and told me someone was working on a JavaScript
implementation of XForms.  


More importantly, XForms have (client-side) validation build-in. For
example, to specify a valid range you use:


<range ref="volume" start="1" end="10" step="0.5"/>


Similarly, XForms have the ability to state that a value must be
supplied before the form is submitted, have a notion of typed values
(eg. you can specify that you expect an URL, integer or date to be
entered).  XForms also include stuff to introduce wizard-like behavior
of filling in a form in stages, include functionality to dynamically
add fields (eg. like when you run out of textfields for poll
questions), etc.  A lot of the stuff we have been talking about. :-)


It might be useful to check if our developer API would map gracefully
on XForms.  It would impose that we are XForms-ready (future proof). 
It's especially interesting because Mozilla is working on a XForms
implementation [3].  (XForms 1.0 support will be included in Firefox
1.1.  You can test it using the nightly Firefox builds.)
[1] http://www.w3.org/MarkUp/Forms/
[2] http://www.w3.org/MarkUp/Forms/2003/xforms-for-html-authors
[3] http://www.mozilla.org/projects/xforms/




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

Mon, 05 Sep 2005 11:15:16 +0000 : nicopepe

As a newsbe, i have not install any patch module Yet. So i will do it.
For Dries, i will have a look on this Xforms projet, but i just wonder
if it is MS compatible (80% of market) ?


Maybe before adding this code I should have write my ideas. I think
this new form API  should consider having an easy way to perform
database update  as well as new records adding. So in my view, 2
specials “events” are needed :


-	call an user defined procedure BEFORE the Form is Loaded (non
existing yet ?) to read database and set value to the form.
-	call an user defined procedure AFTER the Form is validate
(drupal_execute_form)to write to the database update or new record.


I agree with Adrian to use arrays. They allowed flexibility. I think
using $form and $element  arrays with others keys than theses defined
with property constants is a good way. Property constants are used for
regular Drupal core functionality, and user defined keys values used
for extra functionalities. This means that the $form array should have
a well documented structure to be able to extend it.


 I see here three examples:


- Use all JS events (as my code suggest) for client validation.
examples :
$form['onSubmit'] = "js function name or code...";
$element['onkeydown'] = "js function name or code...";


- Create an $element[‘NonValidationMessage’] Key used in
drupal_validate_form user function to return to the user when the
validation of an element is not OK (This message could also be used in
JS in client validation).


- Possibility for a programmer to input in the $element array the name
of the database field and database table to store the input value in
case of correct validation. Exemple :
$form['tablename'] = "mytable";
$form['idValue'] = "value";
$form['idName'] = "myid";
$element['Databasefieldname'] = "myfieldname";
Then create an function to create automatic  SELECT STATMENT to read
the value before display the form ( When we need to modify value stored
in a database) or INPUT SQL statement after validation,


 I hope i am clear with my ideas. Maybe some of them could be used
already in the core.




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

Mon, 05 Sep 2005 11:47:08 +0000 : Dries

nicopepe: I'm not saying we should implement XForms at this point.  I
meant to say that (i) XForms looks like an emerging standard and that
(ii) the XForms API tries to solve the same problems we are trying to
solve with the new form API.  It is worth investigating XForms.  Being
able to generate XForms-compliant code with minimal changes is a nice
extra, but certainly not a must at this point.  Hope that clarifies my
comment.




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

Mon, 05 Sep 2005 12:01:28 +0000 : Junyor

Dries: It'd probably be more interesting to look at Web Forms 2:
http://www.whatwg.org/specs/web-forms/current-work/.  It's geared
toward the Web, whereas XForms isn't.  Web Forms 2 is also more likely
to see native implementations.




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

Mon, 05 Sep 2005 12:33:38 +0000 : killes at www.drop.org

Dries: In my book we are trying to solve a securoty issue woth this
patch: the case of inserting bogus fields into our html forms. Client
side validation is noce from a usability pov but we are trying to do
server side.




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

Mon, 05 Sep 2005 14:02:58 +0000 : Dries

Gerhard: it's obvious that client-side validation is no replacement for
server-side validation. :-)


Anyway, instead of looking years ahead, let's focus on the patch at
hand.




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

Mon, 05 Sep 2005 15:06:27 +0000 : nicopepe

Client side validation will never replace server side validation for
security reason !


I do understand that client side is not yet a priority, but please let
doors open for module developper if they want to implement Client side
validation in their module.




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

Mon, 12 Sep 2005 17:55:57 +0000 : adrian

Attachment: http://drupal.org/files/issues/forms.tar.bz2 (0 bytes)

Here is an updated, and more complete version of this patch. 


I had a bit of a SNAFU with the multiples code, and needed to refactor
it to work properly.


Changes in this version :
 1) Introduced a 'process' property. This is used by the radios and
checkboxes elements, to call 'expand_radios' and 'expand_checkboxes'
functions which turn the single element into a set of elements.
 2) admin/themes screen done.
 3) deprecated fix_checkboxes, since it was no longer needed.
Checkboxes and radios work perfectly now ( they were broken
previously).
 4) The way to access the form value is through a global $form_values
array, that is set up to the structure of the form. It's no longer
flattened, and it works perfectly.
 5) Introduced an array_walk_recursive function (copied from PHPCompat
in PEAR) that allows you to step through the values and do something
with each key. (admin/settings uses this property).
 6) Moved the old form api into a legacy.inc
 7) Did a bunch more forms. 


In this tarball you will find the 2 new files (legacy.inc and form.inc)
and the current patch.




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

Mon, 12 Sep 2005 17:57:30 +0000 : adrian

Attachment: http://drupal.org/files/issues/Forms.rtf (14.02 KB)

Here's the status document of all the forms (rtf export from
omnioutliner)


It's incomplete, as I haven't finished cataloging all the forms yet.




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

Mon, 12 Sep 2005 18:01:25 +0000 : m3avrck

No patch in that zip :)




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

Mon, 12 Sep 2005 18:54:08 +0000 : m3avrck

Also, adrian, is there a way to add events or set variables in
javascript, if the submit button on a form has been clicked? Need this
functionality for a patch about to (hopefully!) make its way into HEAD
soon.




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

Mon, 12 Sep 2005 20:51:26 +0000 : adrian

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

I'm sure it's possible, but I think we prefer attaching events and the
like onto forms using the id attribute.


here's the patch file.




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

Mon, 12 Sep 2005 23:07:59 +0000 : adrian

Attachment: http://drupal.org/files/issues/Forms_0.rtf (22.47 KB)

Here's a complete list of the forms in drupal that still need changing.
Everything that has already been done is not on this list (ie: all the
node modules, and the like)


Some of the forms mix normal output, and form output in the same page,
and switch out depending on certain requirments (the
aggregator_list_page was an example of this), making it a fairly
intensive process to seperate the two pieces of functionality into
clean code. 


If something is rated simple, it means it only involves changing the
functions to arrays, and either returning the array, or returning the
final form using drupal_get_form('form_id', $form); 


If we want any chance of getting this into core before 4.7, I am going
to need some volunteers to handle tackling these forms for me. Once
again, I am looking for a few good men (where men == female OR men ==
male .. obviously. =) ).


I am going to be concentrating on the really complex forms, and the
theme('confirm') form, which is required by a fair amount of things.




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

Tue, 13 Sep 2005 03:38:20 +0000 : Jeremy at kerneltrap.org

I thought I'd give this patch a quick try, but ran into a couple
problems.  First is very minor, it makes some changes to settings.php
that it shouldn't -- something to clean up before the final version.


Second, it adds includes for form.inc and legacy.inc, but the files
themselves are not actually included in the patch.




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

Tue, 13 Sep 2005 11:53:05 +0000 : adrian

They are in the tarball i uploaded earlier.




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

Tue, 13 Sep 2005 12:07:31 +0000 : Jeremy at kerneltrap.org

I tried that last night too, but the earlier tarball was 0 bytes and
contained no files:


   Attachment:	forms.tar.bz2 (0 bytes)


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

Tue, 13 Sep 2005 12:22:52 +0000 : adrian

Attachment: http://drupal.org/files/issues/forms.tar_0.bz2 (27.82 KB)

That is the weirdest thing .. 


locally :
Benton:~/dev/forms adrian$ ls -l forms.tar.bz2
-rw-r--r--   1 adrian  adrian  28489 Sep 12 15:30 forms.tar.bz2




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

Tue, 13 Sep 2005 15:03:17 +0000 : adrian

Attachment: http://drupal.org/files/issues/Forms.html (10.02 KB)

Html version of the status list.




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

Tue, 13 Sep 2005 15:12:22 +0000 : Dries

Guys, please take a look at the status page.  If we want the new form
API in Drupal 4.7, we'll have to work together pretty much non-stop to
complete it!  Care to help?  Pick an item, migrate to the new form API
and upload a patch.




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

Tue, 13 Sep 2005 15:26:09 +0000 : Bèr Kessels

Can we not exclude this from the code freeze? I mean that we now (last
days) focus on all these outstanding patches for 4.7. and then have 
acollaborative effort on this forms patch. That will at least give us
the benefit that we can aim on a stable target. Instead of junting on a
still changing core. From what I see a lot of people are focussing very
hard at their babies in the queue, to try to push them into 4.7 (that
counts for me anyway) So they will not spend any time on the form api
yet.




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

Tue, 13 Sep 2005 15:41:45 +0000 : webchick

For what it's worth, I agree with Bèr. I've been picking away at
comment.module to make it compatible with the forms API, but it's very
difficult since the patch needed to enact all these changes differs
from day to day (today I applied the .tar'ed version and still am
getting errors with a few hunks which I need to troubleshoot first in
order to make sure my changes aren't causing errors).


Once things are more "settled" with the rest of core, it seems like it
would be much easier to make a more "stable" patch as a starting point,
which would thus ease efforts on the part of the rest of us to get the
changes made. This patch represents a HUGE change, and I worry about
the idea of trying cramming it in at the last minute.




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

Tue, 13 Sep 2005 15:54:29 +0000 : Souvent22

I totally agree with the above comments. With so many chagnes happening
so fast, we wake up to a new core each morning, and must make changes
accordingly. I don't know what the core will get 'stable' enough before
the freeze to really hack through what needs to be done in an efficient
manner. We all seem to agree this *needs* to happen, but perhaps a
better plan of *'how'* it should happen since it is such a big change
would make things easier, more stable, and not make it seem like we're
trying to cram a square block through a round hole.




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

Tue, 13 Sep 2005 17:00:23 +0000 : m3avrck

I agree with Ber and Souvent. I think we should all focus on our pet
patches for the Drupal 4.7 freeze (which I think should be Friday give
us that one day ;). After that, we should have a "sub-freeze" where
*only* FORM related API patches go in, set this for 3-4 days after the
4.7 freeze. Then, once that is complete, *then* get back into the
regular bug/performance/usability freeze over all.




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

Tue, 13 Sep 2005 18:58:32 +0000 : Dries

Let's do it the other way around.  If you help with the form API, you
get a 3-day extension for your other patches.  The form API is far more
important than so it needs to hit core first.  I already granted a 2
week extension, and unless this is near completion, I won't extend the
current development cycle.  Sorry.




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

Tue, 13 Sep 2005 19:49:54 +0000 : chx

Dries, we have a deal. Contacted Adrian, will start working on filter
module 90 minutes from now.




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

Tue, 13 Sep 2005 20:27:18 +0000 : Dries

I volunteer to tackle the statistics.module as a starter.




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

Tue, 13 Sep 2005 20:34:58 +0000 : mathias

Put a mark next to my name for path module.




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

Tue, 13 Sep 2005 20:39:46 +0000 : adrian

I am tackling the system module, and (god forbid) node_admin_node (which
is the single most complex form i have come accross).


I am desperately looking for someone who uses localization to help port
locale.module and locale.inc




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

Tue, 13 Sep 2005 20:44:15 +0000 : Bèr Kessels

Attachment: http://drupal.org/files/issues/forms_system.module.patch (29.24 KB)

quick rerolled system module patch. 


which rasies the question: how to we work on this? send in patches per
file, i assume?




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

Tue, 13 Sep 2005 20:45:36 +0000 : Bèr Kessels

my hand goes up for blog.module. to get warmed up.




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

Tue, 13 Sep 2005 21:03:23 +0000 : Tobias Maier

i want to start with watchdog as my first.




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

Tue, 13 Sep 2005 21:08:23 +0000 : Bèr Kessels

hmm; make that block.module. my mistake. I am on bloCK.module now.




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

Tue, 13 Sep 2005 21:20:45 +0000 : m3avrck

hold on the filter.module ... really nice patch about to make it's way
in to redo that area: http://drupal.org/node/27364 ... talked to Dries,
he said he'll commit this filter patch before the FORMs API rewrite team
swoops in so we don't waste our times doubling coding effort :)




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

Tue, 13 Sep 2005 21:27:13 +0000 : adrian

Attachment: http://drupal.org/files/issues/forms.patch_0.txt (105.93 KB)

quick update of the patch.
Also, the filter module is not being worked on right now, as we are
going to be redoing most of it before we port it.




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

Tue, 13 Sep 2005 22:35:42 +0000 : Tobias Maier

Attachment: http://drupal.org/files/issues/issue29465_watchdog.module.patch (1.8 KB)

hello,


I know I'm doing the simplest form of all - the watchdog.module
but i'm running in some errors.


I hope you can help me


here are the important code snipets which are bugging me:

<?php
function watchdog_overview() {
[...]
  if (empty($_SESSION['watchdog_overview_filter'])) {
    $_SESSION['watchdog_overview_filter'] = 'all';
  }
  $form['filter'] = array(
    type => 'select',
    title => t('Filter by message type'),
    options => $names,
    default_value => $_SESSION['watchdog_overview_filter']
  );
  $form['submit'] = array(type => 'submit', value =>t('Filter'));
  $header = array(
[...]
  return drupal_get_form('watchdog_form_overview', $form) . theme('table', $header, $rows);
}
[...]
function theme_watchdog_form_overview($form) {
  return '<div class="container-inline">'. form_render($form) .'</div>';
}
function watchdog_form_overview_execute($form_id, $form) {
  global $form_values;
  if ($_POST['op'] == t('Filter') && isset($form_values['filter'])) {
    $_SESSION['watchdog_overview_filter'] = $form_values['filter'];
  }
}
?>


the value of $_SESSION['watchdog_overview_filter'] and
$form_values['filter'] remain 'all' the whole time...
why?
I tried a lot.


I found out, that "..._execute" will run when drupal_get_form is
called.
So all places where $_SESSION['watchdog_overview_filter'] were called
before wont get the current value...


Interesting is <select name="edit[edit[filter]]"
id="edit-filter"><option value="all" selected="selected">all
messages</option><opt..., too
is this edit[edit[filter]] really wanted?


I dont know how to solve this...
please give me some hints.
Thanks


the attached patch is not working at the moment!!




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

Tue, 13 Sep 2005 22:41:27 +0000 : Bèr Kessels

I give up on block.module. I have been up and coding for nearly 48 hours
now, it is just too much. I am even thinking in arrays ATM :)
array('bed' => array('top'=>'sheets')) and so on :)


So , release the 'lock' on block.module, hereby.




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

Tue, 13 Sep 2005 22:49:20 +0000 : Tobias Maier

I have to give up for today too.
Maybe I have time again tomorrow afternoon


cya




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

Wed, 14 Sep 2005 00:09:25 +0000 : adrian

Attachment: http://drupal.org/files/issues/forms.patch_1.txt (112.89 KB)

Tobias : Thank you kindly, I have integrated your code, and you found a
bug in the select element, and a 1 line fix later .. it was working. 


It's been integrated. Thanks. 


Attached is the latest patch , which also includes the new version of
the theme('confirm') dialog, which has been rewritten and called
confirm_form()


An example for porting any of the confirm screens :



<?php
function _forum_confirm_delete($tid) {
  $term = taxonomy_get_term($tid);
  $extra = form_hidden('tid', $tid);
  $output = theme('confirm',
                  t('Are you sure you want to delete the forum %name?', array('%name' => theme('placeholder', $term->name))),
                  'admin/forums',
                  t('Deleting a forum or container will delete all sub-forums as well. This action cannot be undone.'),
                  t('Delete'),
                  t('Cancel'),
                  $extra);
  return $output;
}
?>


Turns into :

<?php
function _forum_confirm_delete($tid) {
  $term = taxonomy_get_term($tid);
  $form['tid'] = array(type => 'hidden', value => $tid);
  return confirm_form('forum_confirm_delete', $form,
                  t('Are you sure you want to delete the forum %name?', array('%name' => theme('placeholder', $term->name))),
                  'admin/forums', t('Deleting a forum or container will delete all sub-forums as well. This action cannot be undone.'),
                  t('Delete'), t('Cancel'));
}
?>


I changed the parameter order to be inline with the rest of the form
system, ie: a form_id, and then a $form array (which has the same use
as the $extra parameter had before).


To have something happen, just create a $form_id_execute($form_id,
$form) function, provided it passes the validation.


The function has also been moved to system.module (since it has nothing
to do with themes anymore).




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

Wed, 14 Sep 2005 00:11:15 +0000 : adrian

Attachment: http://drupal.org/files/issues/Forms_1.rtf (22.05 KB)

Attached is an updated document with what the progress is.




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

Wed, 14 Sep 2005 00:56:29 +0000 : chx

form.inc , theme_select has an extra edit[ ...]







More information about the drupal-devel mailing list