Not sure if we necessarily should do this, but in reviewing the D7 vs. D8 version of Pants module, this struck me as something kind of silly.

In Drupal 7, you have a settings form done in a single function. You name the function, chuck some form API stuff into it, and return system_settings_form() around the result.

In Drupal 8, you have two functions (buildForm() and submitForm()), as well as this getFormID() method you didn't have to do before (because it would be smart and just get the form ID from the function name).

The buildForm/submitForm thing I don't actually take much issue with (even though system_settings_form() is useful magic, it was magic nonetheless and was hard to explain to people). But having to declare a method in order to return a form ID seems like annoying busy-work to me.

It would be great if instead FormBase's getFormId() function could auto-generate this for you, and you only override it in the case you want to do something funky. We have to be a bit careful though, because class names are not unique across modules due to namespaces.

But if we were to take something like http://drupalcode.org/project/pants.git/blob_plain/refs/heads/8.x-prague... and FormBase turned it into:

"$module_class_name_in_lowercase_with_underscores"

like:

"pants_pants_settings_form"

...I think that'd be loads easier. Apparently we have a "camelize and underscore" method in Container which could maybe move into the Drupal class and work for this. (per msonnabaum)

The disadvantage of doing this is that it makes it harder to find the form ID since it's not sitting there in the class definition, per aspilicious. But IMO it'd be much better if we standardized on a "learn a pattern once and it applies everywhere" kind of thing, like we did in D7 (form definition function name == form ID).

What say you?

Comments

tim.plunkett’s picture

I'm hugely -1 on this, because it would make it very very tricky to figure out where a form actually comes from.

I always use the HTML to figure this out.

<form id="system-performance-settings">
That tells me that it's probably a function called system_performance_settings(), so that's what I grep for.

In D8, that same grep will then point me to \Drupal\system\Form\PerformanceForm, since that has

  public function getFormID() {
    return 'system_performance_settings';
  }

If that was automatically generated, I have no idea how I'd find it.

We'd also have the slightly less-important side effect of needlessly changing all of the form IDs from D7 to D8.

----

The only reason to auto-generate this is to prevent clashes. Since before they were PHP functions, that was handled for us.
But if we do so, we need something in drush or devel to list out all form classes and their generated form IDs, to be able to find the code responsible for a form.

webchick’s picture

Thought exercise: Can we make it an annotation then? (oh god, did I just say that?)

mcrittenden’s picture

Title: Have FormBase auto-generate form IDs, so you don't need to implement getFormId() on all forms » Have FormBase auto-generate form IDs, so you don't need to implement getFormID() on all forms

tim.plunkett, if it's a standard pattern for generating them then it seems like it'd still be pretty easy to find, no? E.g., "pants_pants_settings_form" must be the Pants module, in the class PantsSettingsForm, so grep for that.

We'd also have the slightly less-important side effect of needlessly changing all of the form IDs from D7 to D8.

That seems more important to me since it means a million hook_form_alter's will be targeting nonexistent form ID's when upgrading and everyone will have to track down the new ID's. Since webchick mentioned overriding, if this is strictly necessary then core forms could all override so that they keep the same ID's.

The only reason to auto-generate this is to prevent clashes.

Reducing boilerplate is a huge plus as well.

tim.plunkett’s picture

Status: Active » Closed (duplicate)

Marking as a dupe of #2089593: Have FormBase auto-generate form IDs, so you don't need to implement getFormID() on all forms, despite this being much older, because that issue has a patch.

webchick’s picture

That was a recursive link. :) Looks like you meant #2261533: Provide default getFormId().