D7 is the version in which we get "fields in core." This issue will (hopefully) lead to the of first several commits in a multi-stage process.

At the Data Architecture Design Sprint (DADS) we agreed on a basic strategy for moving fields into core. We will split CCK almost in half; roughly, the core field modules and the logic to load, input, save, and display them will go into core while the much-loved "CCK UI", content_copy, and all contrib module integration code (views, token, pathauto) will remain in contrib.

Also at DADS, we agreed that a/the primary value of putting fields into core will be the ability to bind fields to things other than nodes: certainly users, perhaps comments and taxonomy terms, and hopefully even non-local objects such as remote blog posts, Amazon products, etc., that do not exist (and are never imported) in Drupal's SQL database.

One thing that makes this task so exciting is that CCK for D6 is a fast moving target, still under heavy development. KarenS and yched have done an amazing job at refactoring CCK in a way that makes it much easier to integrate into core but some changes are inevitable. Forking the code into a core patch + what remains in contrib increases the work required for all future changes/fixes for D6 (and D5!). However, the CCK issue queue will never be empty and the D7 development cycle is moving quickly along, so we just have to get started. I/we should try to minimize or delay the fork for as long as possible (though that may not be very long).

Getting fields into core will consistent of at least the following stages, not necessarily in this order:

  • Splitting CCK into the core and contrib portions, renaming one of them (we can't have two content.modules), and committing the core portion implementing the Field API based on hook_nodeapi as CCK currently works.
  • Refactoring whichever parts of nodes make sense into fields. The obvious candidates are title and body.
  • Refactoring the Field API not to be based on hook_nodeapi and not to assume fields are attached to nodes; instead, fields will be attachable to multiple kinds of content. See the DADS final report for plans in this regard.

There will undoubtably be more to it than this and hopefully the roadmap will become more clear as we progress.

The first code will be arriving here shortly.

CommentFileSizeAuthor
#83 fic.patch619.09 KBDavid Strauss
#1 fields-265604-1.patch273.54 KBbjaspan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bjaspan’s picture

FileSize
273.54 KB

The first pass at the code for the first step of this process, splitting CCK into core/contrib and moving the nodeapi-based field functionality in core, is ready for people to look at. Here's the status:

./modules/fields contains the "Fields module." It is a subset of CCK, namely: content.module, includes/content.{crud,alter,node_form}.inc, and the core field type modules (except fieldgroup). content.alter.inc is a new file containing the very small subset of content.admin.inc that deals with altering the database tables and columns as fields are created and deleted.

I have made either no or very small changes to anything *except* includes/content.crud.inc which is substantially altered. As decided at DADS, I have split the concept of a Field from a Field Instance and I have removed the ability to change Field properties for existing fields (i.e. anything that affects the database schema) from core for now. (FYI, my medium-term plans for "field conversion" are described at http://groups.drupal.org/node/11487 (where it is called "data migration").)

I have changed the basic Field and Field Instance data structures from anonymous arrays to classes. This is not a mandatory change but I think improves the code clarity substantially (and I have some additional ideas that will lead to even more benefits later).

The split of Field/Field Instance and the switch to class-based data structures is currently confined to the Field API only. Field/Instances are converted back into their merged array form when passed to the other CCK code (e.g. content.module, field modules). I see this a temporary condition, part of delaying the fork between core and contrib CCK so as many CCK 6.x changes can go in before the fork. Eventually we'll push the new Field/Instance split through the rest of the code, but for now at least *consumers* of the Field API can use it even though field module implementors cannot.

Unit tests for field_create_field(), field_create_instance(), and field_update_instance() exist. Other field_*() methods are not tested.

This code is not nearly ready to be committed. For today, I'm mostly like feedback on includes/content.crud.inc; everything else is copied directly from CCK. Specifically:

  • The overall approach to the API. Please read the DADS final report (see http://groups.drupal.org/data-architecture-design-sprint) first.
  • The use of classes for Field and Field Instance instead of anonymous arrays.
  • The Great Renaming. Should core contain the Field module and contrib the Content module? Or should core be Content and contrib Content Admin? Or something else? Right now the module naming in the patch is inconsistent.
  • And everything else, of course. Just don't be surprised if I reply with "yes, I agree, but that task is postponed for now." :-)

Finally, besides the attached patch (which is large, but mostly just copied from CCK), this code is also available in SVN at https://jaspan.devguard.com/svn/fields/ which should be publicly readable.

KarenS’s picture

Barry, is there a corresponding issue on the CCK issue queue, or is everything in here? Also, I've been working like a banshee for the last week making fixes to the D6 version, have you picked those up in your patch?

I was planning to create a D6.2 branch in CCK in the next day or two to open up HEAD for D7 work. Just letting you know so we can coordinate all of this.

moshe weitzman’s picture

subscribe ... i like the name 'field' module for core and field_ui module for cck ... i think that some of the integration code (token, views, diff, ...) will actually flow back to the source module and out of cck.

catch’s picture

Priority: Normal » Critical
Status: Active » Needs work

Massive, massive +1 to calling the core module 'fields', except all core module names are singular, so it should probably be modules/field/field.module for file/function names.

In terms of CCK D7 contrib, I don't see that much reason to change the name since it's very well known, although presumably it could have sub-modules like field_ui.module, field_migrate.module, field_views.module once the guts are in core. And I agree with the general decision to leave the UI, views integration and data migration in contrib for now - although with a mind to moving at least the first two of those into core at a later date.

Also, less modest status.

edit: cross-posted with Moshe and Karen

sun’s picture

Priority: Critical » Normal
Status: Needs work » Active

I agree with Moshe - "Field" module is actually a better name for the functionality that "Content" module provides. IMO, we should not tackle with renaming CCK/content.module at all. Makes the overall process easier and renaming the module would only add unnecessary burden to our already hackneyed CCK maintainers.

We use singular terms for core modules, so "Fields" should be "Field", of course, also affecting function names.

sun’s picture

Priority: Normal » Critical
Status: Active » Needs work

Didn't mean to change status.

sun’s picture

Do we have a project name exclusion list? If there is one, the project name field should be added to that list to avoid contrib module name clashes.

Crell’s picture

Subscribing. Will try to have a look tonight.

webchick’s picture

Just a question. Why a module called "field"/"fields"? Is there ever a reason we'd want to remove it? Why not just make it one of the core .inc files?

chx’s picture

sun, I have http://drupal.org/project/field created a placeholder so it's impossible to create one.

bjaspan’s picture

#2: Karen: This is the only d.o issue I've created. This issue is for changes to core so I did not think an issue on the cck queue was needed. Of course, CCK will need to change to reflect what happens here, so those issues will be forthcoming, but it is too early to change CCK yet.

Also, no, I have not taken any changes to D6 CCK in the last week. However, my changes outside of includes/content.crud.inc are very minimal, so taking them will should not be too hard.

#3-#7: The interesting part of the naming issue is that once we actually decide on the name and start changing the code imported from CCK to use it, 100% of CCK patches will cease applying to the version in core. So, that is what will actually trigger the fork. That's why I left everything named "content_*" except for the Field API for now.

Another question: Do we want a single module, Field, which provides the Field API, the Field support code (what is now content.module), and all the current core fields (text, nodereference, etc)? Or do we want to keep all the core fields in their own modules? If the latter, should they all be required or optional? *Some* will be required if, for example, node.body becomes a field.

KarenS’s picture

#11 IMO we don't need separate modules for all the fields we want to support in core -- we'll need all of them to be available and won't want to be continually checking if they're enabled. I think it's similar to core hook_elements() -- we don't try to figure out which ones you might need, they're all available.

As to whether 'Field' in core should be a module or an .inc file, I think we'll want some of the goodies that come with modules, hooks and such. To me it's equivalent to the Node module which needs to be a module even though it's always there.

webchick’s picture

I never quite understood why you had to enable other modules for basic stuff like text, number, and optionwidgets. I guess this decision dates from back in the day when field modules were 50,000 K but they've gotten more slim and trim since Content module started taking care of most things internally.

But I would say that for any field types exposed by system_elements() in core, those fields/widgets should always be available.

KarenS’s picture

Oh, I know now why the fields are all modules and not includes -- it's so you can provide custom ones that will behave the same way as the core ones will, which is usually done by implementing hooks. And to provide hooks, they have to be modules. I'm hazy on the history, but I think this resulted from problems in the way that Flexinode handled things where fields were include files instead of modules.

So maybe they need to be modules, but they could all be turned on all the time. Or maybe in our new core code we will see a way to allow for custom field modules without needing the core ones to be modules.

Crell’s picture

The registry and module system refactoring that's going on should eliminate much of the overhead of "being a module" and hook implementations, so I'd favor "lots of little modules". That said, I won't really complain if text and number and trivial stuff like that is all in field.module. It should be possible to override/tweak anything via hooks, which is why it should be a module rather than an include.

(Another reason for module rather than include is that it's then somewhat easier to lazy-load, which saves parsing costs.)

Anonymous’s picture

Subscribing.

bjaspan’s picture

#15: One thing that concerns me about the registry is that APIs implemented in modules cannot be counted on to be loaded. For example, even if we declare that field.module is required, if the Field API is implemented in field.api.inc, it won't be loaded automatically. Module authors will have to write:

if (drupal_function_exists('field_create_field')) {
  // use Field API here
}

Now, actually, if I understand the registry correctly, this is not the case for classes; classes get auto-loaded based on name. So, if a module author writes:

$myfield = new Field('text');
field_create_field($myfield);

then the Field class will get auto-loaded, so if it is in field.api.inc, the API functions will get auto-loaded too. That's a nice side-benefit of using classes for Field and FieldInstance.

So probably the registry and delayed loading isn't going to be a problem, but it is something to keep in mind.

bjaspan’s picture

By the way, I'm planning to add derived Field and FieldInstance types, e.g. TextField, NodereferenceFieldInstance. So you'll actually write:

$myfield = new TextField('my_field_name');
$myfield->formatted = TRUE;
field_create_field($myfield);

These derived classes will basically do nothing but declare their additional per-field type/instance properties. e.g.:
TextField will be very simple:

class TextField extends Field {
  public $formatted = FALSE;
}

Once the API module includes PHPdoc for classes, this will go a long way towards making field modules' per-field and per-instance properties self documenting. It will also benefit IDE users and, with some additional trickery I've got planned, go a ways towards making the Field API actually (gasp!) type-safe.

This will happen as soon as we decide to throw the switch on forking CCK.

webchick’s picture

@Karen: Ahh. That makes sense.

But in that case, could we do a hook_field_info() that defines multiple types, and field_field_info() (or system_file_info(), for that matter) just defines the basic ones like text, number, date, file... that we want to be able to count on from any contributed module? Then it'd still be only one (at most) module to enable/sift through in the UI, and eliminate lots of if (module_enabled('text')) { ... } logic in other field modules.

But I'm going to refrain from further comment in this issue until I've read more of the background on DADS and the CCK APIs though, because I feel like people much smarter than me probably already thought through all of this stuff. :D

Crell’s picture

@webchick: Yes, that's sort of the end goal; however, we concluded that a declarative array syntax was going to require an underlying series of API functions anyway, so that needed to happen first.

@bjaspan: You are correct about the registry; it does favor classes in that regard, or more specifically PHP itself does. I'm still waiting for one of the projects I have planned that involves foreign data to start so that I can try coding up the models we discussed in Chicago. We should probably talk soon to make sure we're thinking along the same lines.

dropcube’s picture

subscribing...

Dries’s picture

Thanks for the great start, Barry! I've only skimmed the code, so a more detailed review is forthcoming. A couple of first remarks:

1. I think we should go with one field.module, not with multiple smaller ones. If people want to have (slightly) different fields, it is better to add these rather than to overwrite the existing ones. If there are 5 modules called "textfield.module", providing support is going to be tricky. It's better to have fields.module (that declares the regular textfield) and a textfield_advanced.module -- like that, we have a unique name for the altered textfield. Second, it actually encourages people to submit improvements to the built-in textfield.

2. In core, we don't glue words together: userreference should probably become user_reference, fieldwidget should probably become field_widget, tablename should be table_name, etc.

3. Do we need to copy over the content_update_600x update functions in the install file? I'd think that these are redundant for core.

4. The .po files don't belong in core (at this point in time).

We'll need some deep reviews here. :-)

eaton’s picture

I really regret that I don't have the bandwidth to put into this at the moment, but if we're seriously considering trying to get Fields into core for Drupal 7, we should take a VERY very sober look at the state of things.

The underlying CCK storage schema for settigns and configuration is adequate but suffers in several very specific areas (ignoring the UI for the moment):

1) There is no provision for formatter-level settings. This has led to an explosion in slight variations on formatters, and many, many dead-ends. Views faced similar poblems for quite some time and only v2 has solved it.

2) There is no provision for pluggable or configurable validation without writing a custom widget. This has led to an explosion of unnecessary field types that offer nothing more than an extra validation hook for social security numbers or phone numbers atop a plain vanilla text field.

In addition, the current CCK workflow assumes that when content types are defined and fields modified, it will always be in the context of a form -- not in the context of an automated process, or during a hook execution. Obviously, this may not be part of the patch you're proposing -- I'm still picking my way through the 270K or so ;-) but it's the first thing that comes to mind when i think about moving CCK's functionality over.

These aren't insurmountable problems, but I have very grave concerns about the impact on core if large portions of CCK are moved over without HEAVY re-factoring or complete rewrite. Karen and Yched have put tons of work into wrapping better CRUD handling around CCK's internals, and fixing a lot of user-facing issues like simple reordering of fields and multivalue fields. CCK's internals still need some specific fundamental improvements before they can be considered sufficient for core.

I'd encourage everyone reading to PLEASE keep in mind that while core means 'more eyes on the code,' it also means 'much, much longer lag between fixes.' It means a profound commitment to cast the CCK APIs in concrete, even more than they currently are. What goes into core in Drupal 7 needs to be 'good enough' to last us until 2010.

KarenS’s picture

There are a couple issues in CCK already related to this, see http://drupal.org/node/255829 and http://drupal.org/node/255825. Nedjo's idea after the code sprint was to do some work in the HEAD of CCK to split the code into the UI that would remain in contrib and a new 'Field' module that would start out in CCK but ultimately go into core. The advantage of doing that work there, instead of here in a core patch, is that you can commit partially working code and incremental changes and let people test it and work out all the details in an environment where it's easier to commit and test.

If we do it here, we have to lug this huge patch around and keep updating it, and you can't identify incremental changes from one patch to another and you can't create sub-issues for the individual parts of the code that Eaton mentions above or work on it a section at a time -- as a core patch it's all or nothing.

Plus it's really really hard to test and evaluate an API if you aren't implementing it anywhere (or only in a couple specific situations) so if we work on splitting out a CCK UI that implements the API, we have a nice way to test 'real world' implementations of the API as we develop it.

The D7 version of CCK could be focused just on getting this working right in a place where we're not limited by what you can and can't do in core, then when we think we have it working well as a new module for a field API that is implemented by the contrib Content UI, we can bring it back here as a core patch that adds whatever other changes are needed in core as well as the new Field module.

Does that make sense?

Crell’s picture

KarenS: It sounds like you're suggesting forking CCK into ck_ui and field contrib modules, developing field.module for another release, and then doing a direct-port of that into core in D8. There is logic there; I just want to make sure I'm understanding you correctly.

webchick’s picture

I read it more as "Instead of grappling with 260+K patches and forks, why not make the HEAD branch of contributions/modules/CCK the patch?" :)

KarenS’s picture

Right, Nedjo's idea was to add a new module to the HEAD of CCK that would hold the code that would eventually go into core and re-factor the contrib module in the HEAD of CCK into the part that will stay in contrib -- so all of this, the new core module(s) and the contrib modules, could live in the HEAD of contributions/modules/CCK while we get it working the way we want.

I guess it's also right to say we'd be forking CCK, but CCK for D7 is going to have to change anyway if we're going to put any part of it into core, so I don't know if it's a fork or a new version :)

KarenS’s picture

Also I'm not saying it wouldn't go into core until D8, it could (and hopefully would) still go in for D7, but the idea is to get it working fairly well before we come back with a core patch.

sun’s picture

I agree with Eaton. The CRUD stuff and API is new, untested, and we don't know yet, what needs to be changed for best integration with other modules and Drupal core.

Now that CCK for D6 is in beta, wouldn't it make sense to start with these efforts already for D6? (partially outsourcing content.module into field.module that chx kindly registered)

Users already waited quite some time for CCK in D6. So I'd say, it wouldn't matter that much, if a RC/stable would be deferred another one or two months. Directing users to download a separate Field module won't hurt them either.

So, +1 to move into field.module first. Then start to think core.

webchick’s picture

Now that CCK for D6 is in beta, wouldn't it make sense to start with these efforts already for D6?

Sweet merciful crap, NO.

CCK D6 needs to be out and stable as quickly as possible in order for the hundreds of thousands of D5 sites to be able to upgrade. The time for refactoring the D6 version (or at least the 6.x-2.x version) has LONG since passed.

bjaspan’s picture

Addressing some of the recent topics:

- Don't review the 260K patch! Everything except includes/content.crud.inc is a direct copy of CCK 6.x from a few weeks ago. As I said in #1, the only thing worth commenting on at this point is the direction in which I'm moving includes/content.crud.inc. The separation of Field and Field Instance; the switch from anonymous arrays to named data structures; the nature of the API I'm moving towards.

- Don't review the patch at all! Use SVN instead. I totally agree that changes like these cannot be made with a patch file. That's why I'm using https://jaspan.devguard.com/svn/fields/. Notice that I've already made many incremental commits just getting to this point. (Anyone that knows CVS and is not familiar with SVN can learn what they need in a few minutes reading Chapter 2 of the SVN book, http://svnbook.red-bean.com/).

- I'm perfectly happy moving this development in CCK's contrib area for now if that's the consensus; I just don't have commit access there, so I worked in SVN instead.

- We can only develop "fields in core" in CCK contrib until the changes we want to make require changes in core. That will happen when we step away from basing fields on hook_nodeapi, or when we convert title and/or body in fields, or when we start implementing the DADS' "Content Model 1 or 2." But until then, sure, let's work in contrib. We can probably get as far as the first RTBC patch for fields-in-core while still working almost entirely in contrib.

eaton’s picture

The separation of Field and Field Instance; the switch from anonymous arrays to named data structures; the nature of the API I'm moving towards.

This is where I hug you, and cry.

Dries’s picture

I think we should aim for Drupal 7, and not Drupal 8. Drupal 8 is not going to be used before 2010-2011.

I'm willing to be flexible here and to commit code that will have to evolve. Why? Because I think this is best improved incrementally -- like the rest of Drupal core.

bjaspan’s picture

Oh! Don't just review includes/content.crud.inc, also review content.test which uses/tests the API.

catch’s picture

@Eaton, there's a gsoc project for a validation API here: http://groups.drupal.org/node/11334 - I don't know what the current progress is, but it should be kept in mind.

KarenS’s picture

As soon as I figure out the best way to open up a place for D7 in CCK (http://drupal.org/node/266663) I'll create a place for the 7.x work and I'd be glad to let Barry (and Eaton if he's interested) have commit access.

bjaspan’s picture

I should point out that the work I've done already requires a very minimal core patch to each of the database.*.inc files. It's included in SVN and the patch file I attached here, but I'll show it just for clarity:

-function db_fetch_object($result) {
+function db_fetch_object($result, $class = 'stdClass') {
   if ($result) {
-    return mysql_fetch_object($result);
+    return mysql_fetch_object($result, $class);
   }
 }

The change is required (or I guess just convenient) for the nature of the Field API for which eaton gave me such a great hug. :-) There will probably be other such changes.

The point here is that working purely in contrib will not be possible for long if at all. We can do most of the CCK refactoring there, but core patches will still be necessary. Granted, not 250KB worth of patches.

KarenS’s picture

It will still help if we only put the core patches in here and leave the rest of the code in CCK. I totally missed this when looking through your code.

Crell’s picture

Barry, if I understand what you're after with the $class parameter correctly the new DB API will already include that. (This is a great use case for it. :-)) Let's plan on using that part of the new API rather than modifying the existing API. Just a note.

bjaspan’s picture

#39: Yes, definitely. But that still means that the fields in core work presupposes a modified version of core to be useable.

KarenS’s picture

The HEAD of CCK is now configured as a place for the D7 CCK to core work and Barry has commit access to it.

yched’s picture

Thanks for this Barry. I'm slowly getting up to speed after a couple of off-drupal weeks. I'll try to review asap (might still be a few days, though...)

pwolanin’s picture

bookmarking...

drewish’s picture

subscribing

bsherwood’s picture

+1 for CCK "core" in "drupal core"

subscribing...

Leeteq’s picture

Subscribing.

alex_b’s picture

Subscribing.

Alex UA’s picture

subscribing...

bjaspan’s picture

I am making another pass at this. When we last left our story, I had defined new Field API functions and structures:

class Field { ... };
class FieldInstance { ... };

function field_create_field(Field $field);
function field_delete_field($field_name);
function field_create_instance(FieldInstance $instance);
function field_update_instance(FieldInstance $new_instance);
function field_delete_instance($field_name, $type_name);

The main goal of this API change is that it defines separate semantics for Field and FieldInstance. Only FieldInstances can be updated once created. Fields must be "converted" via the mechanism I spelled out in the CCK group. Cleaning up this distinction makes the code simpler and more powerful.

However, only the Field API functions in content.crud.inc (eventually to be re-named field.api.inc or field.module or something) use the new data structures and semantics. All the rest of the CCK code, which I copied more or less unmodified, still uses the merged field/field-instance array. So, class Field has a toArray() method and class FieldInstance has a toArray() and toMergedArray() (which is field->toArray() + instance->toArray() because that happens to be how CCK currently works), and whenever the Field API functions interact with anything else in CCK, they pass these values.

In this round, my goal is to push the new Field API structures and semantics through the rest of "CCK core" (e.g. content.module, content.node_form.inc, etc.) and the field modules. Actually, I'm just going to do one field module, text, as a proof of concept; I hope to enlist the "power of the community" for the rest. :-)

I updated my working code to include all CCK changes through 6.x-2.0-RC4 (all the changes were to field modules which I hadn't modified so they applied cleanly, yay). However, changing everything to use the new Field API structures will cause 100% of future CCK 6.x-2.0 patches to fail; all changes will have to be made separately on both branches. I do not think there is any way to avoid this.

For now, I'm continuing to work in SVN. The SVN repository is https://jaspan.devguard.com/svn/fields and it should be publicly readable. Regarding the discussion about working in CCK's contrib area in CVS, I am happy to do that but want to clarify some things with Karen first. In the meantime, there is no reason everyone here cannot access SVN directly. It really is just like CVS. Start with "svn checkout https://jaspan.devguard.com/svn/fields" and go from there. :-)

Design questions are certain to come up. Watch this space if you want to participate.

bjaspan’s picture

Design question:

Fields in core is splitting CCK in half, leaving the UI and "third-party module integration" (e.g. views, token, etc) in contrib. It sounds great in theory but I've already hit a snag: hook_field_settings($op = 'views data'). This hook_field_settings() opcode lets a module return custom information for Views. In CCK 6, includes/content.views.inc defines hook_views_data() (a Views hook) which either uses a module's hook_field_settings('views data') or a default set of a views data for each field.

includes/content.views.inc can easily be left in contrib as part of CCK. However, the core field modules (text, etc.) will be in core. That means their hook_field_settings() will be in core. It seems wrong to have field modules in core implement hook_field_settings('views data') when Views is not in core; the Views support should remain in contrib. But we can't define (e.g.) text_field_settings() twice, once in core and once in contrib.

I see two solutions:

1. We abandon opcodes for hook_field_settings (and probably all Field hooks). For each former opcode, we define hook_field_settings_$op(). I believe there is a movement in this direction for D7 core already anyway. This solves the problem because then a contrib module can implement (e.g.) text_field_settings_views_data() while all the core text_field_settings_"opcodes" are defined in text.module (or field module, or whatever) in core.

2. We preserve opcodes but have hook_field_settings (and perhaps others) pass control to another hook function if they do not recognize the opcode. e.g.:

function text_field_settings($op, ...) {
  switch ($op) {
    // normal cases here

    default:
      return module_invoke_all('field_settings_extra', $op, ...);
  }
}

I think option #1 is way, way better than option #2.

Comments? Other approaches?

Crell’s picture

Death to $op.

yched’s picture

Generally, +1 for no more ops.

The current work of polishing CCK D6 and getting to know D6 Views a little better also makes me seriously consider the advantages of an OO rewrite of the notion of field module. Having field and widget 'types' be handler classes, implementing their own text::field_validate() or textarea::widget() methods would seriously clean the code of a number of not-too-nice solutions (CONTENT_CALLBACK_NONE / _DEFAULT / _CUSTOM, CONTENT_HANDLE_CORE / _MODULE) that were introduced to ensure both a) a rich set of default features and behaviors for all field modules at little cost for them, and b) enough flexibility for field modules with exotic needs to override them with the specific behavior they need.

KarenS’s picture

+1 from me, too on getting rid of ops. We got rid of $op in content_widget() in D6 and I seriously wondered about getting rid of it for content_field() and the settings hooks, but we already had so many huge changes that it seemed like too much to tackle.

I also have seen the advantages of the Views 2 OO code that would be nice to have in CCK, and I would love to get rid of some of our hackish solutions for making the current code work.

I see no way around ending up with a D7 CCK that will be significantly different than the D6 version, so we'll have to commit separate patches for each. Trying to make them match will hamstring the process of making the code as clean as it ought to be.

The biggest disadvantage of separate patches will be if we end up with any non-trivial changes to D6 -- new features or restructuring -- and there are issues that propose things like that, so we'll have to deal with it.

pwolanin’s picture

Barry and I had a chat about this - we also concluded that one module should be able to implement more than one field type (e.g. if the core module handles a couple basic types).

KarenS’s picture

One module can already implement more than one field type -- the Date module does that and so does the number module -- so that's not really a change. But I agree that we don't necessarily need a separate module for each basic field type.

Anonymous’s picture

Yes, core should implement the basic types but we should allow a 'custom' type that provides hook_field_type functionality. I'm thinking the functionality would check for function {mymod}_{myfield_type}_field_type to implement the details of the field type.

bjaspan’s picture

Well, that was basically unanimous, so I'll be stripping $op from field.module.

re: rewriting field modules in an OO style, I am by no means opposed to that. However, Dries has made it very clear to me that he wants fields in core as soon as possible, absolutely in D7. He would rather commit a less than perfect patch and have it iterate/evolve via further patches to core than it wait indefinitely until it is "perfect" to be committed. His reasoning is that everything that might use fields in core is blocked until at least the first patch lands, and once it does the core developers will be more motivated to work on improving it than while it is still being developed in contrib.

So, I suggest we stick with our basic architecture for now until the phase 1 patch (nodeapi-based fields in core) is done.

Crell’s picture

The risk of "get it in and fix it later", of course, is that things are harder to change once they're in core. Switching from purely procedural to purely OOP after the patch lands will be difficult. I am not adamantly against that approach, just pointing out the challenges it poses.

Modules should absolutely be able to define any number of fields, widgets, and formatters. I'd even go a step farther and say that those definitions should be module-agnostic, too. In CCK D6 the theme function for a formatter is named based on the module with a required format, but you still have to handle your own theme registration. That seems disjoint to me. (Too late to change it now for D6, but something to keep in mind for a core implementation.)

catch’s picture

@Crell: I'd say that's mainly true towards the end of a release cycle or between release cycles. One advantage of it going in earlier is we'd be able to start the process of converting core fields to the new API.

bsherwood’s picture

With all the talk of getting CCK "core" into drupal core, is the field module going to also replace profile modules fields as well?

I think that is one of the big issues with user profiles in general in drupal. Nodes offer more flexibility then generic profiles, and one of the main reasons why we have modules like Bio and Node Profile.

I personally don't really care if user profiles are nodes or whatever they are called now, I think people just want a flexible and extendable way to add fields to hold user data. Other wise we have profile fields and CCK fields and just seems like a duplication of code, IMHO.

I can understand that this might be more to ask at this time. I know the majority of the development is just trying to get it in for D7, but maybe it is something that can happen after field.module has been created, then roll the changes into that.

Anonymous’s picture

With all the talk of getting CCK "core" into drupal core, is the field module going to also replace profile modules fields as well?

Once this is a core functionality for general use then I think revamping the profile module to use core fields is a natural outcome. The date field logic in the profile module needs a date field; i.e. I loathe the three column serialized array used now. Though, I haven't found the time to suggest a patch for it.

bjaspan’s picture

#60: The medium-term goal for Fields in Core is that fields will be attachable to multiple kinds of content objects, including nodes, users, or anything else. Read the DADS report (see Description of this issue) for details.

bjaspan’s picture

As promised, I have pushed the new Field API structures and semantics through the rest of "CCK core" (e.g. content.module, content.node_form.inc, etc.) and the text field module. I'd appreciate some reviews/comments on the direction I'm going (Karen and Yves, this means you particularly!). The code is not in CVS or a patch; it's in SVN. Start with "svn checkout https://jaspan.devguard.com/svn/fields" and go from there. :-) If this is a problem for anyone, let me know.

Do not blindly review the whole thing. I'm specifically interested in feedback on these items:

* The Field API from a field-using developer's point of view. Probably the best way to see this is to look at content.test and see how I use the field_*() functions (granted they are buried among a bunch of asserts). If your module wanted to create a new field on a content type, how would you feel writing:

$field = new TextField('fieldname');
$field->required = TRUE;
// etc
field_create_field($field);

$instance = new TextFieldInstance('fieldname', 'content_type name', 'text_textarea');
$instance->widget->rows = 17;
field_create_instance($instance);

* Everything in includes/content.crud.inc. Note that any functions named content_* in this file pre-date my changes but will be renamed to field_* and be part of the Field API.

* The Field API from a field-module developer's point of view. Look at text.module and how it defines TextField, TextWidgetSettings, and TextFieldInstance. NOTE: ONLY TEXT.MODULE IS CONVERTED TO THE NEW API SO FAR. The other field modules will not work!

* The Field API from a CCK developer's point of view. Now that I've pushed the Field and FieldInstance distinction through CCK core you can see how it will affect CCK internals. Mostly, it just replaces $field['foo'] with $field->foo or $instance->foo. In some cases I had to de-multiplex some functions (e.g.: content_callback) that used to take a $entity argument for "field" or "widget" because now the functions use argument types and cannot take any kind of argument.

Regarding the $op question, I removed the one case that was causing a problem but not a bunch of others, yet.

moshe weitzman’s picture

I took a look at those bullet points in particular. Nothing jumps out at me as problematic. I don't especially grok what below is doing. Maybe a comment would help.


class TextFieldInstance extends FieldInstance {
  protected $widget_class = 'TextWidgetSettings';
}


bjaspan’s picture

Heh. I wrote comments for that but apparently I forgot to commit them. Fixed. :-)

yched’s picture

Not forgetting this, but it will be difficult for me to look at the code before a couple days

cburschka’s picture

Subscribing.

Edit: The SVN repository should probably be brought up to date with CVS.

oadaeh’s picture

Adding another subscriber.

RobLoach’s picture

With all the talk of getting CCK "core" into drupal core, is the field module going to also replace profile modules fields as well?

The inclusion of Views/Field into Drupal core could potentially replace a lot of Drupal core (blog module, profile module, forums module, etc).

bsherwood’s picture

Is there an effort into getting Views into core? I thought Views was getting postponed till D8.

I would really love to see Views/CCK get into core and replace the older (IMHO deprecated) modules like: blog, forum and profile. Since these are really nothing more than views; At least with the presentation portion of blog and forum. Profile module is somewhere in between; its not a node but it is more than a view?

webchick’s picture

I'm not aware of anyone spear-heading efforts for Views in core for D7, but that doesn't mean it couldn't happen if someone picked up the torch!

aaron’s picture

time for me to jump into this issue. i want fields for file objects... is the patch at #1 the only patch right now? i'll read through things and see what i can do to help.

Xano’s picture

I suggest (re)writing all validators in such a way that they can easily be used by any random field. Error messages should therefore be in the form of "!field_name may not contain spaces" and if an element's validation function checks if its value is both numeric and doesn't contain the same number twice, for instance, both checks should have their own validation function for maximum flexibility.

alexanderpas’s picture

+1 for CCK core into Drupal core

I don't know about title but body should be a field!

subscribing

yched’s picture

CCK HEAD now contains Barry's work from his SVN repository, synced with current CCK D6, and updated to core HEAD.

moshe weitzman’s picture

Wow, thanks to whomever did that big chunk of work. I guess it was yched. Note that these files are still deleted according to http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/cck/?hideat...

I will be at the Sprint. Since I live in Boston, is easy for me.

yched’s picture

Moshe : everything now lives in the /fields and /field_ui folders (er, with actual modules still named content*), with only a 'WARNING D7 ONLY' text file at the root. Unless I'm missing something, current state of the CVS seems fine.

D7 update : I actually did not check all the items on Converting 6.x modules to 7.x for now, only the ones that were clearly needed to make the thing seem to work.

markus_petrux’s picture

Oh, it's so great to know this is going to get "in".

PS: subscribing

tstoeckler’s picture

I just wanted to know what the actual status is by know, and what exactly is to be achieved at the sprint. Thanks

scor’s picture

How about RDF mappings into fields? If this is done in a generic fashion, it would not only be useful for RDFa in core, but also for other contrib modules using the RDF API.

aaron’s picture

How about RDF mappings into fields?

A good idea, but I think it's kitten killing feature creep for this issue. Probably needs its own request.

bjaspan’s picture

Until further notice, Fields in Core discussion is taking place at http://groups.drupal.org/fields-core.

David Strauss’s picture

FileSize
619.09 KB

I'll be posting weekly patches here so people can review our work and post progress. If you'd like to participate before this lands in core, using Bazaar is a better idea than applying these patches and working from there. We've added a few modules for development purposes to sites/all/modules which will *not* be going into core.

Anyone can generate a patch versus HEAD:

bzr branch bzr://vcs.fourkitchens.com/drupal/7-fic
cd 7-fic
bzr merge bzr://vcs.fourkitchens.com/drupal/7
bzr commit -m "Merge in CVS HEAD."
bzr diff --old=bzr://vcs.fourkitchens.com/drupal/7 > fic.patch

You'll also want to remove the changes to .bzrignore, because that doesn't exist for core.

catch’s picture

Component: base system » field system
bjaspan’s picture

Status: Needs work » Closed (duplicate)

This issue has been obsoleted by http://drupal.org/node/361683.