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