Get rid of form_set_error()

Yeah, would have been nice. D8 material now.

If we would pass the form by reference all across validate then we could set the error on the form itself too. Needs to be done in form_state too so we have a cumulative list without iterating the form to display the errors themselves but if we do it on the form elements then they are self-contained to be erroneus and much easier to theme as such.

subs

I agree with @chx, ideally we store it in both $form and $form_state, whereas $form_state should contain the complete list of errors, and the element in $form should only hold declarative properties.

To think a bit ahead of time: In #742344: Allow forms to set custom validation error messages on required fields, I already attempted to introduce a $element['#required_error'] property (holding a custom required error message for #required elements). So ideally, we'll try to think a bit of other changes in the future, especially with regard to the $form element properties; e.g. an #error* element property namespace, so we don't end up with a shitload of different #has_error, #errormessage, ... properties. (btw, the internal form validation properties could use a similar name(space) overhaul)

Also, we need to retain the currently existing edge-case functionality of

1. passing '' (empty string) as parents, essentially filing an error against the entire form.
2. passing custom][parents, allowing to mark an entire form section as erroneous.

API changes:

• New $form_state['errors']
• New $form_state['limit_validation_errors']
• New $element['#errors']
• form_set_error(&$form_state, $name = NULL, $message = '')
• form_error(&$element, &$form_state, $message = '')
• form_get_errors(&$form_state)
• form_get_error($element, &$form_state)

Needs work:

• form_set_error() in:
  • file_save_upload()
  • openid_begin()
  • SearchQuery::executeFirstPass()
  • image_gd_settings()
• form_get_errors() in:
  • comment_preview()
  • node_preview()
  • openid_authentication()
  • system_settings_form()
  • theme_taxonomy_overview_terms()
• ??

Thanks! Looks pretty good already. Can you roll further patches with -p to include more context info in the diff, please?

Huh I am sorry I butt in after a 100kb+ patch but there was no previous patch so I couldn't before

if (!drupal_valid_token($form_state['values']['form_token'], $form['#token'])) {
       // Setting this error will cause the form to fail validation.
      form_set_error($form_state, 'form_token',

That's hideous. Can't we do a form_error($form['form_token'] instead? The very issue title is to get rid of this function completely.

Update:

• Replaced form_set_error() with form_error() where possible.

Issues:

• There are 6 calls to form_set_error() left. These are all called in places where $form and $form_state are unavailable.
• There are 6 calls to form_get_errors() where $form_state is unavailable.
• ??

Arr forgot local commit

Update:

• Re-added form_get_errors($form_state) instead of calling empty($form_state['errors']) to retain a healthy API
  

#1180662: Don't call form_set_error(), form_get_errors() and alikes in functions that aren't necessarily form related

+++ b/includes/form.inc
@@ -1521,31 +1517,20 @@ function form_set_error($name = NULL, $message = '', $limit_validation_errors =
+      $form_state['errors'][$name] = $message;

Rather than call what is now a 3-param function, wouldn't it be easier to do this bit in validation handlers, and then have the Form API look at these after all the validation handlers have been called?

0 days to next Drupal core point release.

chx pointed me to this issue while I was working on #2131851: Form errors must be specific to a form and not a global, the code base has changed drastically, but the $elements['#errors'] approach really really helped.

They are duplicates of each other, but I don't really know if it's okay to close this much older one...

I think I've sorted it in #2131851: Form errors must be specific to a form and not a global