Convert the "Field edit" form to an actual entity form

Some awesome work. Just two nits.

1. +++ b/core/modules/field_ui/src/Form/FieldConfigEditForm.php
   @@ -2,149 +2,97 @@
   +    $bundles = entity_get_bundles($this->entity->getTargetEntityTypeId());
   

   We have $this->entityManager in EntityForm we can replace this with $this->entityManager->getBundleInfo($entity_type);

2. +++ b/core/modules/field_ui/src/Form/FieldConfigEditForm.php
   @@ -2,149 +2,97 @@
   +    $ids = (object) array('entity_type' => $this->entity->getTargetEntityTypeId(), 'bundle' => $this->entity->bundle, 'entity_id' => NULL);
   

   Please change this to multi line.

Aweome, just like #2446869: Convert the "Field storage edit" form to an actual entity form, this removes a lot of weirdness.

1. +++ b/core/lib/Drupal/Core/Entity/EntityForm.php
   @@ -165,7 +165,7 @@ public function processForm($element, FormStateInterface $form_state, $form) {
        // Rebuild the entity if #after_build is being called as part of a form
        // rebuild, i.e. if we are processing input.
   -    if ($form_state->isProcessingInput()) {
   +    if ($form_state->isRebuilding() && $form_state->isProcessingInput()) {
   

   This was just added by #2448357: Config entity forms need to have access to an updated entity object at all times - but we actually need both checks ? The comment above needs to be adjusted then ?

2. +++ b/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php
   @@ -104,8 +104,16 @@ public function defaultValuesFormSubmit(array $element, array &$form, FormStateI
   -      $ids[] = $properties['target_id'];
   +      if (!empty($properties['target_id'])) {
   +        $ids[] = $properties['target_id'];
   +      }
   

   What would be a case of having a $default_values[N] that doesn't have a 'target_id' ?
   (and how is that revealed by / related to this patch ?)

3. +++ b/core/lib/Drupal/Core/Field/FormatterInterface.php
   @@ -19,7 +19,7 @@
   -   * Invoked from \Drupal\field_ui\Form\FieldEditForm to allow
   +   * Invoked from \Drupal\field_ui\Form\FieldConfigEditForm to allow
   
   +++ b/core/lib/Drupal/Core/Field/WidgetInterface.php
   @@ -25,7 +25,7 @@
   -   * Invoked from \Drupal\field_ui\Form\FieldEditForm to allow
   +   * Invoked from \Drupal\field_ui\Form\FieldConfigEditForm to allow
   

   HEAD is actually lying anyway - the settingsForm() for widgets and formatters are invoked by the Display forms, right ?

4. +++ b/core/modules/comment/src/Tests/CommentNonNodeTest.php
   @@ -392,14 +392,14 @@ function testCommentFunctionality() {
   -      'field[settings][anonymous]' => COMMENT_ANONYMOUS_MAY_CONTACT,
   +      'settings[anonymous]' => COMMENT_ANONYMOUS_MAY_CONTACT,
   

   <3, like in the other issue :-D

5. +++ b/core/modules/entity_reference/src/ConfigurableEntityReferenceItem.php
   @@ -200,13 +197,9 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state) {
      public static function fieldSettingsFormValidate(array $form, FormStateInterface $form_state) {
   -    if ($form_state->hasValue('field')) {
   -      $form_state->unsetValue(array('field', 'settings', 'handler_submit'));
   -      $form_state->get('field')->settings = $form_state->getValue(['field', 'settings']);
   -
   -      $handler = \Drupal::service('plugin.manager.entity_reference_selection')->getSelectionHandler($form_state->get('field'));
   -      $handler->validateConfigurationForm($form, $form_state);
   -    }
   +    $field = $form_state->getFormObject()->getEntity();
   +    $handler = \Drupal::service('plugin.manager.entity_reference_selection')->getSelectionHandler($field);
   +    $handler->validateConfigurationForm($form, $form_state);
      }
   

   Cool indeed !

6. +++ b/core/modules/field_ui/src/Form/FieldConfigEditForm.php
   @@ -152,55 +100,86 @@ public function buildForm(array $form, FormStateInterface $form_state, FieldConf
   +  protected function actions(array $form, FormStateInterface $form_state) {
   +    $actions = parent::actions($form, $form_state);
   +    $actions['submit']['#value'] = $this->t('Save settings');
   +
   +
   +    if (!$this->entity->isNew()) {
   +
   +      (... determine a $url ...) 
   +
   +      $actions['delete'] = array(
   +        '#type' => 'link',
   +        '#title' => $this->t('Delete'),
   +        '#url' => $url,
   +        '#access' => $this->entity->access('delete'),
   +        '#attributes' => array(
   +          'class' => array('button', 'button--danger'),
   +        ),
   +      );
   +    }
   +
   +    return $actions;
   +  }
   

   Feels weird that this is both calling the parent and re-doing all over again what it does for the 'delete' button, with just a different $url ? Can't we just override $actions['delete']['#url'], just like we simply override $actions['submit']['#value'] above ?

   (also, double empty line)

the parent actions() method will only generate the delete element if the entity has a 'delete-form' link template, and field_config doesn't have one

Fair enough. Isn't that something we'd want to add ? :-) We do add them for view_mode & form_mode config entity types.
(probably not striclty related to this patch though...)

Related - field_ui's job of "decorate the 4 core entity types (field, field_storage, view_mode, form_mode)" seems a bit randomly split between field_ui_entity_type_build() and field_ui_entity_type_alter() 200 lines lower.
The usual rule of thumb is "hook_*_build() is to add stuff that's not there, hook_*_alter() is to modify what's already there", right ? So it should all go in field_ui_entity_type_build() ?
(also not srtricly related to this patch here - followup once this one and its sister issue are in ?)

Should we wait for #2446869: Convert the "Field storage edit" form to an actual entity form?

view modes and form modes can have link templates because they have a fixed path [...] but field storage config, field config and entity displays have dynamic paths that vary by field_ui_base_route

Right, makes sense. Thanks for the explanation :-)

Opened #2459949: Remove field_ui_entity_type_alter() and move the content to field_ui_entity_type_build() - this patch here should be good to go.

Committed 95dc317 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.