This patch replaces $object with $entity, $obj_type with $entity_type and $objects with $entities. This does not change any API while it makes field API a lot more grokkable.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Hah, an heroic soul rolled a patch :-). Be forever blessed.

Big big +1, and I'd advocate committing ASAP and fixing possible remains later, because this patch will be HELL to reroll.
It would really need a bot seal, though...

Frando’s picture

Finally! Big huge +1 from me too. I wanted to write this ever since entity API was committed but never got around to do it. This will avoid some huge DX WTFs in D7.

Yes, once tests pass, let's get it in ASAP.

yched’s picture

I guess we're stuck with the API facing occurrences, though :

- {field_config_instance}.object_type schema column
- 'object keys' in hook_entity_info()
- 'object_types' in $field definitions
- 'object_type' in $instance definitions
- 'obj_type' and 'object' in hook_field_attach_view_alter()'s $context param

:-/

Status: Needs review » Needs work

The last submitted patch, call_them_entities.patch, failed testing.

chx’s picture

Status: Needs work » Needs review
FileSize
188.31 KB

Let's see this.

Edit: I rolled using:

cd modules
find . -type f -exec sed -i 's/\$objects/\$entities/g' {} \;
find . -type f -exec sed -i 's/\$object/\$entity/g' {} \;
find . -type f -exec sed -i 's/\$obj_type/\$entity_type/g' {} \;
bzr revert comment dblog trigger

Edit2: the previous patch missed /g . My bad.

yched’s picture

Status: Needs review » Reviewed & tested by the community

If the bot is happy, let's commit this before it breaks.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

It broke. :(

Great patch, though.

chx’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
188.31 KB

The bot can jank back to CNW if it broke. I reran the script.

chx’s picture

FileSize
188.01 KB

Opsie that was the old patch.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, entity_vars.patch, failed testing.

chx’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
188.01 KB

Sniff, sniff. The bzr mirror is behind by about an hour.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, entity_vars.patch, failed testing.

chx’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
193.84 KB
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome, thanks.

Committed to HEAD.

yched’s picture

Status: Fixed » Needs review
FileSize
124.5 KB

W00t.

And here's the remainder - mostly comments and PHPdocs. That one was manual, folks :-p. Never. Again.

Pasqualle’s picture

Status: Needs review » Reviewed & tested by the community

nice patch

totally off topic: I noticed that some simpletest messages are translated and some messages are not. Can someone tell me the issue number? I guess we do not need to translated those, same as we do not translate schema descriptions. It is just an extra work for translators without rational benefits.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

My goodness! You poor thing. :(

Committed to HEAD, while it's still fresh. :)

Pasqualle’s picture

Status: Fixed » Needs review
FileSize
1.04 KB

it's still not complete

found one more which can be fixed without breaking the API:
http://api.drupal.org/api/function/field_ui_existing_field_options/7

***************************************************
fixing these would break the API:
(search obj_type)
http://api.drupal.org/api/function/hook_field_attach_view_alter/7
http://api.drupal.org/api/function/field_view_field/7
http://api.drupal.org/api/function/hook_field_attach_preprocess_alter/7
http://api.drupal.org/api/function/field_attach_view/7
http://api.drupal.org/api/function/field_attach_preprocess/7
http://api.drupal.org/api/function/field_create_instance/7
http://api.drupal.org/api/function/locale_field_attach_view_alter/7
http://api.drupal.org/api/function/locale_field_fallback_view/7

(search object_type)
http://api.drupal.org/api/function/field_format/7
http://api.drupal.org/api/function/hook_field_widget_form/7
http://api.drupal.org/api/function/_field_invoke/7
http://api.drupal.org/api/function/field_attach_load/7
http://api.drupal.org/api/function/field_attach_view/7
http://api.drupal.org/api/function/field_attach_rename_bundle/7
field.crud.inc
http://api.drupal.org/api/function/field_default_view/7
http://api.drupal.org/api/function/field_default_form/7
http://api.drupal.org/api/function/field_multiple_value_form/7
http://api.drupal.org/api/function/_field_info_collate_fields/7
http://api.drupal.org/api/function/_field_info_prepare_instance/7
field_sql_storage.module
field_sql_storage.test
field.test
http://api.drupal.org/api/function/field_ui_inactive_instances/7
... (and many more)
***************************************************

these are 3 different cases:
context['obj_type'] : function parameters stored in array
#object_type : render element
instance['object_type'] : db column field_config_instance.object_type

so it is still confusing..

Pasqualle’s picture

As I see we already broke the field API since alpha1 with #641670: Move $form['#field'] meta information into $form_state so we might do it again to fix this issue once and for all..

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community
yched’s picture

re #18 : yup, that's the remaining ocurrences I pointed in #3. Part of the public API, so changing these are API changes.

Agreed on RTBC for the fix in the patch.

chx’s picture

Pasqualle and all -- yes I am (painfully) aware of those. We can't fix everything in D7 IMO -- but let's see what the core committers are going to say.

Dries’s picture

Committed the patch in #18.

Personally, I don't think it makes sense to leave things in an intermediate state -- it feels like that would only add to the confusion.

chx’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
57.63 KB

As Dries asked I ran more sed, this time nuking all occurences of obj_type and object_type and changed 'object' => 'entity' and fixed the new presave functions by hand.

Status: Needs review » Needs work

The last submitted patch, sed_more.patch, failed testing.

KarenS’s picture

Still badly needed. It is totally confusing to see all the remaining mixed use of 'object_type' and 'object' vs 'entity_type' and 'entity' throughout the code.

aspilicious’s picture

Status: Needs work » Needs review

#24: sed_more.patch queued for re-testing.

aspilicious’s picture

damn didn't wonna retest..
Well... Ok then...

Status: Needs review » Needs work

The last submitted patch, sed_more.patch, failed testing.

yched’s picture

Status: Needs work » Needs review
FileSize
57.7 KB

Rerolled. You don't scare me, bot.

yched’s picture

Rather try this one - additionally replaces 'object keys' -> 'entity keys' in hook_entity_info().

yched’s picture

Status: Needs review » Reviewed & tested by the community

W00t !.

This will break anytime. If we still want to do this, now looks like a good time.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Hm. I guess Dries says in #23 that the API breaking train is okay, in favour of consistency in this part of the code.

Committed to HEAD, but are we about done here? It's awfully late for any more of these kinds of changes.

KarenS’s picture

Yeah but this one was a huge WTF for anyone trying to port their code (which is where I ran into it). Do we have objects or entities? Oh no, in this place it is an object, in that place it is an entity. We needed to get this in.

yched’s picture

Thks Angie.
Not really related to the 'object' / 'entity' stuff, but I think the last naming WTF is the 'cacheable' property in hook_entity_info(), whose meaning is ambiguous since the entity API landed. Been discussed for months but no-one set to it so far. Couldn't find an existing issue, created #754686: Rename 'cacheable' property in hook_entity_info()

Should the changes that got committed here be mentioned on http://drupal.org/update/modules/6/7 ? Or somewhere else ?

KarenS’s picture

Are we doing updates from prior versions now? If so, we have a db table change that needs an update. The 'field_config_instance' table changed the column name from 'object_type' to 'entity_type'. I think that is the only db change, it's the only one I've run into.

yched’s picture

As a summary of the API changes:
- db table field_config_instance:
'object_type' column renamed to 'entity_type'
- hook_entity_info():
'object keys' entry renamed to 'entity keys'
- $field definitions:
$field['object_types'] entry renamed to $field['entity_types']
- $instance definitions:
$instance['object_type'] entry renamed to $instance['entity_type']
- hook_field_attach_view_alter(), $context parameter:
$context['obj_type'] and $context['object'] entries renamed to $context['entity_type'] and $context['entity']

yched’s picture

Status: Fixed » Reviewed & tested by the community
FileSize
853 bytes

Followup - one occurrence slipped through.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD, however, this suggests that our tests aren't 100% yet.

yched’s picture

re Dries #39 : Well, this specific test happens to be a little conservative, and the second clause usually catches the case at hand, so this omission had no actual effect.

JohnAlbin’s picture

For those people getting PHP notices/failures because of the DB change introduced with this patch (what Karen mentions in #36 above), you can use the following query to fix it in MySQL:

ALTER TABLE field_config_instance CHANGE COLUMN object_type entity_type varchar(32) NOT NULL DEFAULT '';

Also, see #763636: Notice: Undefined index: view modes in _field_info_prepare_instance()

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

TR’s picture

Title: Confusing arguments all around » Call them entities instead of objects
Status: Closed (fixed) » Needs review
FileSize
1.63 KB

I found two places where "object" is still used instead of "entity" in user-facing exception messages. Patch doesn't affect the API, just the content of the message strings.

yched’s picture

Status: Needs review » Reviewed & tested by the community

Right.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.