Move simple field types into Core/Field

Will need re-roll after #2198917: Use the string field type for the node title field

Rebased against latest HEAD, and fixed one set of fails.

Not sure what to do about Drupal\field\Tests\reEnableModuleFieldTest - this test seems like it needs further work outside this issue, as disabling and re-enabling modules is no longer supported, and this test doesn't really do what it claims any more.

Created #2204153: Remove or repurpose reEnableModuleFieldTest

The meta now move duplicated field types to Core

email and number modules are moved to Core #2218199: Move email and number field types to \Drupal\Core\Field, remove modules

Because that issue was/is not about simple/not simple, it's about *duplicate* field types, we had a email type in Core and then email.module extended it and we have integer/float field types in Core\Field and number. Also, we wanted to get that in as a first step. The meta is still open.

Moving more to Core\Field is fine with me.

Repurposing, then. Moving some field types to field.module would make little sense now that Core/Field has most of them.

So datetime, link and telephone could be moved to Core/Field
options and text - still needs some polish
image and file - is not simple

telephone field, in FieldImportDeleteUninstallUiTest moduke is replaced with options

Fix broken test, now test able to use field_test module.
Converted a link module

Datetime module is tricky, it has collision with #2226493: Apply formatters and widgets to Node base fields but probably datetime_form_node_form_alter() could be removed here or in context of #2150511: [meta] Deduplicate the set of available field types
Also filed #2331783: hook_ENTITY_TYPE_prepare_form() is not documented

Fix broken test and outdated @todo (once touched)

Very much +1, but won't be able to review before DC Amsterdam, so I hope someone else can.

Also, there are two patches in the RTBC queue that, combined, remove almost all code from the datetime.module file (sorry, browsing issues & pasting links from my phone is super tedious :-/)

[edit: sorry, one of them, that moves helper #validate callbacks into the widget classes, is not RTBC yet actually]

Postponed #2331961: Move datetime module into Core/Field on both issues.
This is a separate issue because api changes:
1) removal of datetime_date_default_time()
2) conversion of DATETIME_* constz

1. +++ b/core/modules/block_content/src/Tests/BlockContentFieldTest.php
   @@ -11,8 +11,6 @@
     * @group block_content
   - * @todo Consider removing this test when https://drupal.org/node/1822000 is
   - * fixed.
   

   Not sure what this @todo was about exactly and how that issue was related, but it looks like a useful test to me.

2. +++ b/core/modules/field/src/Tests/Link/LinkItemTest.php
   @@ -24,7 +24,7 @@ class LinkItemTest extends FieldUnitTestBase {
       *
       * @var array
       */
   -  public static $modules = array('link');
   +  public static $modules = array();
   

   No need to keep an empty array around.

3. +++ b/core/modules/field/src/Tests/Telephone/TelephoneItemTest.php
   @@ -23,7 +23,7 @@ class TelephoneItemTest extends FieldUnitTestBase {
       * @var array
       */
   -  public static $modules = array('telephone');
   +  public static $modules = array();
   

   Same here.

4. +++ b/core/modules/field/src/Tests/reEnableModuleFieldTest.php
   @@ -2,7 +2,7 @@
   - * Contains \Drupal\field\reEnableModuleFieldTest.
   + * Contains \Drupal\field\Tests\reEnableModuleFieldTest.
   

   Woah, weird class name.

5. +++ b/core/modules/field/src/Tests/reEnableModuleFieldTest.php
   @@ -24,9 +24,8 @@ class reEnableModuleFieldTest extends WebTestBase {
        'field',
        'node',
   -    // We use telephone module instead of test_field because test_field is
   -    // hidden and does not display on the admin/modules page.
   -    'telephone'
   +    // Using field_test module since it displays on the admin/modules page.
   +    'field_test',
      );
   

   I don't think we need to keep that comment around then or at least not the admin/modules part. Interesting side effect of the removal of hidden for test modules, though...

6. +++ /dev/null
   @@ -1,74 +0,0 @@
   -function link_help($route_name, RouteMatchInterface $route_match) {
   

   I don't remember what we did for other help texts, like email_help(). Did we jus just remove them too? Maybe part of it should be moved to field_help() ? There seems to be some useful explanation here, unlike email had.

   I will ask @jhodgdon to comment on this.

1) That's why I remove the @todo because this is only test that tests fieldability of content blocks, otoh BlockContentCreationTest uses body field on block
2) fixed
3) fixed
5) changed comment to point that we are able to use test module for the test, seems a time ago test modules are not displayed at module's page. So while test modules are visible...
6) Looking at #2218199: Move email and number field types to \Drupal\Core\Field, remove modules we simply remove this, but it makes sense to file follow-up to return all of them somehow but not in field module...

IMHO, it makes sense in field.module, because it is documentation for site builders on how those field types can be used as configurable fields. That is still field.module specific.

Filed follow-up #2332687: Lost help for field types from Core/Field

re-roll

Regarding the hook_help(), we do definitely need to have help describing the available field types. Rather than just removing the hook_help() entirely, probably this should be added to the Field module help. Some of the field modules' hook_help() had specific help on configuration options, which would be a shame to lose.

@jhodgdon is there any idea where to put the removed help?
field module help better left in separate issue

Previous to this effort of moving field types into Core/Field, if you went to admin/help/field, you would get a list of enabled modules that provide fields/widgets (for some reason not including formatters??), with a link to their module help pages, so you could click through to get more help:

This list is generated in function field_help() in field.module.

So what I think we need to do is:

a) Rewrite this help text so that it mentions that some field types, formatters, and widgets are provided by Drupal Core without even enabling a module. Right now it doesn't even say this.

b) If any of the previous patches on this effort removed useful information that was previously in the module help for these fields/widgets, add it to the Field module help. Same for any future field modules that are being removed in this effort, such as Telephone, which definitely does have specific information that shouldn't be lost.

re-roll, well discus the way to move hook_help() tommorow

Implemented a part of #40a - added a list of core field types.
There's no place to put a removed hook_help parts but there's a bug in field_help() - no closing DL

I think we should properly fix that in #2332687: Lost help for field types from Core/Field

Thanks @andypost!

I took a look at your patch, mostly looks great!

A few nitpicks:

a) core/config/schema/core.entity.schema.yml has newlines between the new schema items you added, but not one before the first schema item you added:

      sequence:
         - type: string
+entity_view_display.field.telephone_link:
+  type: entity_field_view_display_base
...
+          label: 'Title to replace basic numeric telephone number display'
+
+entity_form_display.field.telephone_default:
+  type: entity_field_form_display_base

Be consistent?

b) field_help():

...the field types and input widgets themselves are provided by additional modules and core..."

I'm not sure I'm happy with the "... additional modules and core" wording. ... I think it would be fine if it said "Drupal core" instead of just "core". I just don't think that readers of this help know what "... and core" really means.

c) field_help()

Some of the modules are required;

Is this still true?

d) field_help()

+      $output .= t('Field types provided by core:');
+      $item_list = array(
+        '#theme' => 'item_list',
+        '#items' => $core_field_types,
+      );
+      $output .= drupal_render($item_list);

I believe that item lists can have headers. It would be better to put the "Field types provided by core" in as a header, because I think there are some accessibility concerns? I guess this should be done with the field modules list as well?

e) If we are going to display the descriptions of fields in this list we need to make sure that they have descriptions that are readable and appropriate for help. Things like "An entity field containing a boolean value" are not OK. I guess that can be filed as a separate issue, but the more things we put off to separate issues, the less likely they are to be fixed at all. Most of these descriptions look like developer-speak and not end-user-speak... so either we need to fix them so they are aimed at end users (and document this somewhere in the Field annotation class too!), or if they are aimed at developers we should not be displaying them in hook_help().

f) core/modules/field/src/Tests/FieldImportDeleteUninstallTest.php

The changes to this file are very confusing...

     // Stage uninstall of the Telephone module.
     $core_extension = \Drupal::config('core.extension')->get();
-    unset($core_extension['module']['telephone']);
+    unset($core_extension['module']['text']);
     $staging->write('core.extension', $core_extension);

Comment needs updating?

g) same file

-  public static $modules = array('telephone');
+  public static $modules = array('filter');
 
...
-    unset($core_extension['module']['telephone']);
+    unset($core_extension['module']['text']);
...
-    $this->assertFalse(\Drupal::moduleHandler()->moduleExists('telephone'));
+    $this->assertFalse(\Drupal::moduleHandler()->moduleExists('text'));

If the first change is using the filter module for this test, shouldn't the others be? And I thought the text fields were being moved to core and that module was going away?

h)core/modules/field/src/Tests/reEnableModuleFieldTest.php

-    // We use telephone module instead of test_field because test_field is
-    // hidden and does not display on the admin/modules page.
-    'telephone'
+    // Using field_test module while it displays on the admin/modules page.
+    'field_test',

Huh? Confusing comment.

e) If we are going to display the descriptions of fields in this list we need to make sure that they have descriptions that are readable and appropriate for help. Things like "An entity field containing a boolean value" are not OK. I guess that can be filed as a separate issue, but the more things we put off to separate issues, the less likely they are to be fixed at all.

Totaly agreed with the need to come up with good, non-dev-lingo descriptions for those, but I don't think we can reasonably put that in the scope of that issue, or make it a pre-requisite. That's an issue of its own, and we should move forward with moving the field types to Core before that.

Right now there is no viable strategy for retaining the help that we have now for these modules, which includes a coherent, user-friendly description of the fields and in some cases, some necessary help on how to use them.

I fundamentally disagree with proceeding on this patch without also moving the *existing* documentation to someplace where users can find the information. We already have written the text (it is in the hook_help()). It needs to be moved someplace where it can still be accessed through hook_help(), not discarded. We should not regress on the existing help just because it seems nicer or cleaner to have these fields in core instead of in modules, where as far as I know, they are working fine and not causing loads of trouble.

But that will be a decision for another core maintainer. I've stated my objections...

@jhodgdon: #44 was not about losing existing help, it was about proceeding here without having to polish the "descriptions" of all existing core field types as a preliminary.

But we are losing existing help. The short descriptions we had previously, when you clicked the links to the previously-existing field modules, have vanished, and they've been replaced in this patch by incomprehensible developer-oriented descriptions.

We now have a strategy for replacing this help. See the issue summary in #2332687: Lost help for field types from Core/Field.

Basically, on that issue I'm going to add a patch to field_help() to add a section to put this help in. For now, we decided the only information that got lost was from the Number module help, so I'll put that in the section. I'm going to write a patch right now for that.

On this issue, what we'll need to do is: as fields are removed, if they have details in their hook_help() that are important for UI users to know, add notes to that same help section.

And on comment #27 on that other issue, here's an overview the information that was identified that we wouldn't really like to lose as we lose the modules:

* Link module - There's quite a bit of useful information there about formatting them.

* Options module - Has quite a bit of useful information about what they mean and about setting them up.

* Telephone module - The information about displaying telephone numbers as links is actually relevant and useful, and is important for documenting accessibility (accessibility features need to be documented to satisfy ACAG guidelines).

* Text module - Has quite a bit of relevant and useful information about choosing between the text field types, and their settings and formatters, as well as editors and text formats.

As #2332687: Lost help for field types from Core/Field got commited we can move here, but now removal of telephone module is a question

Right, if any of the 4 modules listed in #49 is being removed, the information listed in #49 should be added to the field_help() function, similar to what was done for Number... It looks like the current patch is removing Link and Telephone modules. I'm not sure about the others?

Rerolled on 8.0.x

patch looks strange because it is twice bigger

Trying again

It seems core has changed significantly, I don't have the knowledge to fix up the patch here.

I think the patch is just that big because you haven't set up git diff properly to deal with moving files. See https://www.drupal.org/documentation/git/configure, the part about rename.

Once you upload a new patch, then we should see what changed in those classes, maybe you dropped some changes by keeping old moved files.

Thanks for your help Berdir, I checked my .gitconfig and it conforms to all of those rules. (I am using Aquia dev desktop).

Is this still something to be done? If so, we should slot this under #3118154: [meta] Deprecate dependencies, libraries, modules, and themes that will be removed from Drupal 10 core by 9.4.0-beta1 at least for modules to be deprecated / removed.