Faciliate altering features, e.g., export field display settings separate from field data structure

I completely agree. It's something I'm working on as we speak. On my specific site I need to set the RSS display options on the site without messing with the distro field settings. I'll post more within the next couple of days when I have a patch to play with.

My plan is to have a new exportable that allows you to control/export each field within each entity that has config data. For example, you could create one feature with the "teaser" display settings for a specific entity, and another feature for a different entity, and another feature for the "rss" display settings of another entity.

That's pretty much the approach I was looking at also. I agree that the override and UI are a bit tricky, but I want to start with the basic functionality assuming that you already know which display settings you want to override and then work from there. I'm not sure if it's hook_field_update_field or hook_field_read_instance that we want to use for this.

Give me a day or so to play with this and I'll post more details. I'm under some time pressure on this. If you want to exchange more specific ideas or code use my email since I check it more often that this issue queue.

Thinking about where to draw the line between settings stored in a new Field Settings exportable. Obviously the Display settings need to go into the new exportable. But what about some of the other stuff, like the Widget?

As an example, a distro defines a specific field (an image) but a specific site wants to use a new niftier widget for that field (instead of the default image selector). This would be another case where a site-specific feature would want to override a distro feature.

But this could get carried away. Do we split the Cardinality of the field? The Translatable setting?

And do we group all of these settings together into a single exportable for each bundle type? Or split them into a finer-grained list of exportables? For example, do we really split the "teaser" and "rss" settings into two different exportable "options", or just combine them into a single option for that entity bundle type?

@mpotter I work with @rickvug and have prototyped how to separately export field display settings. I managed to create two exportables:

1. The main content type, which contains fields with $field['field_instance'] not containing $field['field_instance']['display']
2. Display settings per field, that only contains $field['field_instance']['display'] per field.

However, I believe since I am trying to split the field_instance in two different features causes trouble and the display settings are ignored. I could not find a way to let features combine data of components from multiple exportables.

Eitherway I have a vague feeling that the main problem is that the display settings are saved with the field instance in field_config_instance table.

I would be happy to share my code if you are on the same track.

Also, in reply to #5 I dont think the architectural constraints are limited to the separation of display settings from the field data structure. This is more general to features as a whole:

How do we allow sites to override anything without making that override un-manageable by future updates to the distro? (Menu links, Contexts, Views etc.)

With regard to display settings for content type fields, by exporting the field display settings into separate features, sites would be able alter display settings and use the separate feature to manage the exports, without theoretically overriding the parent feature that only provides field definitions sans their display data.

However, the questions raised are very valid in terms of granularity. I dont see a reason why we should go finer in level than providing per-field display settings. I dont see a reason why anyone would want to only export the 'rss' view mode display setting of a particular field instance separately. If they do, this means that the display settings of the main field instance itself should have been separated at first. But I may be wrong..

With all things considered, the issue mentioned in my previous comment holds: How do we combine field data from multiple exportables during features_rebuild?

@girishmuraly: I agree that this is a more generic issue to Features as a whole. In talking with some other P2 people today, I'd like to solve this in a more general way that would help with easily overriding features. I need to look at the Features Override module a bit more. I don't really like exactly how it does things, but there might be some ideas there we can use.

One idea we have is to call some sort of alter hook when exporting a feature and also when "importing"/executing a feature to allow the exported code to be changed in a way to handle overrides. For example, exporting a section of code "diffs" and then applying those diffs when creating the feature to allow overriding the base distro feature. Something like this might be more generally useful for Features and still solve this display settings issue.

@mpotter Features_override does sound like the solution in principle. However, the number of steps needed to create a moderately complex override coupled with its buggy D7 version makes it less of an immediate option. Haven't tested it in particular to this case of separating display settings though..

Is there any overlap with #975386: Automatic generation of features_component_alter() hooks

@febbraro: I don't think there is much overlap with that one.

Here is the architecture that I'll propose. In the field_features_export_render routine where it outputs the $field code, at the end of the code just before we return $fields, we will output a call like:

  features_alter_component(&$fields, 'my_bundle', 'field_default_fields');
  return $fields;

where "my_bundle" is taken from the specific entity/bundle identifier.

The new "Field Override" exportable would generate the code for this alter hook. For example, to change the teaser display settings for a specific field "my_field", it might output the code:

function field_override_my_bundle_field_default_fields(&$data) {
  $data['node-my_field']['field_instance']['display']['teaser'] =>array(
          'label' => 'hidden',
          'settings' => array(),
          'type' => 'hidden',
          'weight' => 0,
        ),
}

the master "field_alter_components" routine would loop through the various components looking for the specific field_override_* alter hook to call and call drupal_alter to make the change to the passed $fields data.

This architecture would allow us to handle any kind of code diff for overrides. A UI for selecting which stuff to override could be improved later. For now I'll just select a field and output the overridden display settings. But you could imagine overriding other stuff later, such as widgets, etc.

@mpotter, I don't see where $fields are returned in field_features_export_render, but I agree with your suggestion although I think a drupal_alter will be sufficient:

function field_features_export_render($module, $data, $export = NULL) {
  $translatables = $code = array();

  $code[] = '  $fields = array();';
  $code[] = '';
  foreach ($data as $identifier) {
    if ($field = features_field_load($identifier)) {
      unset($field['field_config']['columns']);
      unset($field['field_config']['locked']);
      unset($field['field_config']['storage']);

      // Alter the $field; this is where the display settings can be modified in 
      // $field['field_instance']['display'], for example
      drupal_alter('features_field_load', $field);

However, that means we do need to impose similar alter hooks in field_features_rebuild so that the defaults are also correctly built reading overrides.

No, you are missing the cool part of this. We are not calling drupal_alter directly in render. We don't want to change the rendered code. We want Field to still output the same rendered code that it does now.

We are calling the alter routine within the actual code that is generated by render. So to be more specific:

function field_features_export_render($module, $data, $export = NULL) {
  ...
  if (!empty($translatables)) {
    $code[] = features_translatables_export($translatables, '  ');
  }

  $code[] = "  features_alter_component(&\$fields, {$bundle}, {$hook});";

  $code[] = '  return $fields;';
  $code = implode("\n", $code);
  return array($hook => $code);
}

and

function features_alter_component(&$data, $bundle, $hook) {
  drupal_alter('feature_override_'.$bundle.'_'.$hook, $data);
}

So the $field data is modified at the time the feature code is executed. Our override feature just needs to export the code for the specific feature_override_bundle_hook routine (as shown above) to modify $fields.

@mporter, now that I understand what you meant, it does sound like an exciting idea. I'd be interested to see how it comes out if you get to try it..

Obvious todos:
* Features need to also create code that would create specific 'features_override_$bundle_$hook'
* Would we try to amalgamate features_override & features for this or simply patch features if the changes are lightweight?

Current plan is to submit a smallish patch for Features that adds the various changes to exported code to call features_override_$bundle_$hook. Then have a separate module (Features Field Override?) that will use this new functionality to override the field display settings.

Then in the future a more general module (Features Alter?) can be written to handle the more generic case of exporting Overrides to other features, with a nice UI to select which code diff to export, etc.

Here is a patch for Features that adds the two pieces of functionality needed for overrides. Currently the patch only adds the override alter hook to the Field exportable (features.field.inc). Here are the two changes:

1) The code generated in features.field.inc by field_features_export_render includes a call to a new "alter" function: features_alter_component. This function takes 2 arguments: The data array to be modified/overridden, the name of the component (field), and the specific hook or id being overridden (for Field, this is the Bundle name). The features_alter_component is in features.module and just calls drupal_alter. The hook called by drupal_alter will exist in a new Override Feature created by a separate Feature Exportable (see below).

2) To support drupal_alter, the function generated for exportables need to accept an argument. To support this, a new property is added to the hook_features_api called "render_args" which simple provides a string value written as the argument list for the exportable function.

Sample Override Module
I have created a sandbox (http://drupal.org/sandbox/mpotter/1280900) to demonstrate the new override alter hooks. This sandbox module creates a new Features exportable that overrides the display settings of specified entity bundles.

There are still several issues with this code that need to be resolved. For example, if you create a "Distro Feature" that contains the base content type and fields, and then create a "Override Feature" that overrides the field settings of a bundle, you can disable the Override feature and see the original display settings restored. However, if you then enable the Override feature, the override doesn't take effect until the parent Distro feature is reverted. Since the Distro feature is not marked as "overridden", it cannot be reverted except via the "drush fr --force distro_feature" command.

Dealing with detecting changing overrides and handling the revert requires a lot more thought. For example, you don't necessarily want to force the parent feature to be reverted when the override is enabled (or disabled). Any comments, suggestions, or help on this would be appreciated. Feel free to download the patch and the sandbox module and play with it.

For the record, I think ideally in d8 field display settings should be taken out of $instance definitions and centralized in separate, standalone $display objects, containing settings for all fields + non-fields in a view mode.

I still need to open that core issue...

@mpotter, I have tried your module with the patch and posted my comments separately here #1281454: Initial impressions and testing outcomes

New patch for overriding features. This patch updates Features to work with the Beta 1 version of my sandbox module (http://drupal.org/sandbox/mpotter/1280900)

The combined patch and sandbox module implements the Field Settings override feature, which allows display settings to be separately exported to their own feature. The sandbox module illustrates how a general override mechanism might be used in Features.

If feedback on this is positive, then similar hooks will be added to other core Features exportables. So please test and advise.

Change Note: Changes to core features with this patch are:

1) The code generated in features.field.inc by field_features_export_render includes a call to a new "alter" function: features_alter_component. This function takes 2 arguments: The data array to be modified/overridden, the name of the component (field), and the specific hook or id being overridden (for Field, this is the Bundle name). The features_alter_component is in features.module and just calls drupal_alter. The hook called by drupal_alter will exist in a new Override Feature created by a separate Feature Exportable (sandbox module linked above).

2) To support drupal_alter, the function generated for exportables need to accept an argument. To support this, the features_export_render_defaults routine in features.export.inc has been changed to add a &$data argument if the function name ends in "_alter". This is a change from the previous patch.

3) An additional hook name is defined in the hook_features_api routine. The property "normal_hook" is similar to the existing "default_hook" but can be used to specify a different function used to return the normal exportable data rather than the default hook. The routine features_get_default_hooks is changed to accept an argument to determine which hook property to return. The new features_get_normal_hooks function calls this to return this new property. The features_get_normal_hooks function is called in features_get_normal.

New patch. Fixes issue with override exportable not being included before alter hooks are called. Also added "override" property to api to specify which existing exportable this override applies to. This makes some code for handling includes easier.

Another patch. This was tested with the newest sandbox module linked above. Fixed issues with having the Override feature in the same module as the base Distro feature.

Greatly simplified the Features Field Override module resulted in fewer patches needed to Features itself. Here is the new patch. It replaces all previous patches in this thread. The only changes needed to core Features are now:

1) The code generated in features.field.inc by field_features_export_render includes a call to a new "alter" function: features_alter_component. This function takes TWO arguments: The data array to be modified/overridden and the name of the component (field). The features_alter_component is in features.module and just calls features_include to be sure the alter routines are loaded, and then calls drupal_alter. The hook called by drupal_alter will exist in a new Override Feature created by a separate Feature Exportable (sandbox module linked above).

2) To support drupal_alter, the function generated for exportables need to accept an argument. To support this, the features_export_render_defaults routine in features.export.inc has been changed to add a &$data argument if the function name ends in "_alter".

The above patch does not cleanly apply (using features 7.x-1.0-beta4)

[girishn@girishn-dev features]$ git apply -v features-overrides-1277854-23.patch
features-overrides-1277854-23.patch:39: trailing whitespace.
/**
features-overrides-1277854-23.patch:56: trailing whitespace.

Checking patch features.export.inc...
error: while searching for:
 * Return a code string representing an implementation of a defaults module hook.
 */
function features_export_render_defaults($module, $hook, $code) {
  $output = array();
  $output[] = "/**";
  $output[] = " * Implementation of hook_{$hook}().";
  $output[] = " */";
  $output[] = "function {$module}_{$hook}() {";
  $output[] = $code;
  $output[] = "}";
  return implode("\n", $output);

error: patch failed: features.export.inc:325
error: features.export.inc: patch does not apply
Checking patch features.module...
Hunk #1 succeeded at 345 (offset 24 lines).
features-overrides-1277854-23.patch:34: new blank line at EOF.
+
Hunk #2 succeeded at 863 (offset 101 lines).
Checking patch includes/features.field.inc...
Hunk #1 succeeded at 123 (offset 19 lines).
Hunk #2 succeeded at 159 (offset 25 lines).

Using -p0:

[girishn@girishn-dev features]$ git apply -p0 -v features-overrides-1277854-23.patch
features-overrides-1277854-23.patch:39: trailing whitespace.
/**
features-overrides-1277854-23.patch:56: trailing whitespace.

Checking patch a/features.export.inc => b/features.export.inc...
error: a/features.export.inc: No such file or directory
Checking patch a/features.module => b/features.module...
error: a/features.module: No such file or directory
Checking patch a/includes/features.field.inc => b/includes/features.field.inc...
error: a/includes/features.field.inc: No such file or directory

Did you checkout the 1.x version of Features?

git clone --branch 7.x-1.x http://git.drupal.org/project/features.git

That's what the patch was applied against here.

Hmm, not sure what happened. Maybe I had an older dev version here. Here is a new patch that I just did for today's 1.x-dev version of Features:

Looks like we posted at the same time. Try the patch in comment #28

Yes, patch at comment #28 applies on the latest 7.x-dev

I'm impressed by the working of the features_field_override module after applying the patch in #28. Associated testing was carried out at #1281454: Initial impressions and testing outcomes

Leaving this ticket open to further test the changes to features module. Wonder if automated tests are not run for patches here?

Good catch. Here is an update that is more compliant.

I've also applied for full module status for the Features Field Override module. Should be able to get the patch into Features soon.

Exporting features overrides is very interesting.

I don't see though why it is necessary to export this strange "features_alter_component()" calls - shouldn't it make use of regular drupal_alter() hooks instead? Every exportable should have one or in case of faux exportables I guess features should implement one like it does for the main default hook.

I could see features adding first-class support for overrides in .info files, e.g. having syntax like following would be nice:

features-overrides[component][] = "name"

Then features could just go and export it using an alter hook.

The features_alter_components does call drupal_alter. However you will also notice in the code that it needs to call features_include_defaults($component) to ensure that the *_alter routines from the override components have been properly included. Calling a secondary routine such as features_alter_components allows changes to be made to this in the future without breaking all existing features.

You are correct that every exportable should have a call like this. And yes, this method can certainly be extended to a more generic override mechanism in the future. Once the issues have been worked out with the Features Field Override you'll see additional progress on this. One step at a time.

ad #38:
Yep, makes sense. I miss-understood parts of the patch, got it now!

Once the issues have been worked out with the Features Field Override you'll see additional progress on this. One step at a time.

Sounds great.

Anyone else care to give this a test and RTBC it?

I found a problem with this patch. The call to "features_include_defaults" in the new "features_alter_components" was including the "field" component include file, but not the needed "field_settings" include file. So when reverting a feature, the *alter hook in the override component had not been loaded yet and wasn't being called.

This gets a bit tricky: how to determine what include files need to be loaded. And not just when calling get_defaults, but also when calling get_normal.

I looked at the various existing ways Features allows an exportable to include a code file. If you set the 'file' field in the API hook, that file gets included, but it is a "global" file in the sense that it cannot be located within the exportable feature module directory.

To fix this, I needed to add a new property to the API info called 'include_file'. If set to TRUE, the feature/component-specific *.inc file will be included once. It looks for the {module_name}.features.{component}.inc file within the {module_name} directory where {module_name} is the name of the exportable feature module. Then I added the new routine "features_include_overrides" which is called from features_include to include any of these types of files.

A bit confusing, and if somebody has a better way to handle this, let me know.

Here is the new version of the patch that fixes the problem with reverting an override not working.

Probably needs somebody to re-review this latest change.

While looking for a more general way to handle the alter-hook, rather than modifying each exportable render routine, I moved the drupal_alter calls up into the main routines of Features. Both features_get_normal and features_get_default need to call drupal_alter on the data returned by the exportable.

Turns out the features_get_default *already* had a drupal_alter call! It had a slightly different naming convention, but rather than changing it I added the corresponding call to the features_get_normal routine.

This makes the patch to Features even simpler! I will post the new patch here, but I am still pursuing simplification of this even further. Mainly I'd like to come up with a better way to handle the includes so we don't need the new features_include_overrides() routine.

Hmm, not so easy. Using the existing alter hook call creates naming collisions between some existing features and the Override features. So I added an additional drupal_alter call that uses "override_hookname" instead of just "hookname" for drupal_alter. New version of patch is here.

subscribing

Adding a tag for a release blocker

Here is the latest update to this patch. A modification to my sandbox shows that I can move the alter hooks out of the exportables and into the Override module itself. This eliminates the need for the include_overrides.

The new "universal" alter hook is called from both features_get_defaults and features_get_normal. It uses the array in the first argument to look for two different possible alter hooks:

1) mymodule_override_features_alter(&$data)
2) mymodule_override_default_hook_alter(&$data)

where "default_hook" is the component-specific hook, such as field_default_fields (e.g. mymodule_override_field_default_fields_alter(&$data)).

Other than this universal alter hook, the only other changes now in this patch are a bug fix in features_include_defaults to use array_keys, a bug fix in field_features_rebuild to clear local cache, and the check for outputting an alter hook when exporting a feature. Even though alter hooks within the exported code are no longer needed, I kept this feature for future possibilities.

See the sandbox (http://drupal.org/sandbox/mpotter/1280900) for the latest Features Override Field module that goes with this universal alter hook.

Thanks Mike for digging into this key set of questions.

I agree that a universal hook is needed. It seems like a parallel case with form altering. Sometimes you know what form you want to alter, and so can implement a hook specific to that form, but other times you want to alter all or a large subset of forms, and so need a universal hook.

Questions/comments:

1. I'm not clear on why we need designated "override" alter calls. Isn't all altering overriding in this sense? The two calls drupal_alter($default_hook, ...) and drupal_alter('override_' . $default_hook, ...) seem pretty much identical. Is there a use case for having both? What would one do differently in hook_{$default_hook}_alter() vs. hook hook_override_{$default_hook}_alter()? Is there a case in which a single module would implement both? Unless there's a clear need for drupal_alter('override_' . $default_hook, ...), I'd say skip it and stick with adding just a universal alter hook.

2. For the universal hook, I'd suggest, drupal_alter('features_default') rather than drupal_alter('override_features').

3. You're right that our alters need to affect not only default but also normal components. This point addresses a bug in Features. Currently, changes made to default components at the drupal_alter() stage often show up as overrides. They should not: the default component should be the version post-altering and the normal component should be the version that's actually used, whether overridden or default. To get this working, I assume, we need to invoke not only the new universal alter hook but also the existing $default_hook one.

   But - and it's a big but - simply running these alters after we've fetched the normal version of a component isn't going to get us what we need here. Alters run on default hooks are, by definitions, alters of the defaults--not the normal (potentially overridden) components. If we run alters on overridden data, we could well be reverting it to its pre-override state, and so losing changes. So adding alter calls to features_get_normal() looks like the wrong approach. Instead, we need to ensure that when we load normal components what we get is fully altered.

   To really solve this problem, we need to understand better just why we're sometimes getting false overrides showing up as a result of default hook alters--and I'm not convinced we yet fully understand that. I suspect we have a range of related but distinct cases, and that solving them may require several different fixes.

   One case is adding or removing components via an alter. In this case, those added or removed components will show up as differences. Here, the problem is that the info data fed to hook_features_render_defaults() in features_get_normal()is unaltered. To fix this, we would need to ensure (a) that any module removing or adding components via a default hook alter also implements hook_system_info_alter() and makes corresponding changes there. And, for this to work, we need to ensure that hook_system_info_alter() is invoked on data loaded via features_get_features, which, on my testing, doesn't look to be the case.

   The other case is changing existing components. Here, we expect features_get_normal() to load either (a) the default component, in the case that that component has not been overridden, or (b) the overridden component, in the case that it has been overridden. If the component is still in its default state (which is equal to the default code + any alters done), no overrides should show up, but in practice the components do at least sometimes show up as overridden. Why? Is it only pseudo exportables that have this bug? One source of the current bug seems to be that, if a component is not overridden, features_get_normal() loads it from the default implementation but doesn't invoke altering, so we get an incomplete version. So, yes, we need to ensure that altering happens when we load normal components, before we pass them to rendering.

   In solving this issue, we need to keep in mind that, with our proposed universal alter hook, we're introducing an alter that modules implementing features hooks don't yet know about. If Views is loading up a default view, it doesn't yet know that it'll need to run the view through our universal alter hook as well as hook_views_default_views_alter(). Probably, invoking this universal hook when loading a default component becomes part of what's required for Features support/compliance.

   In sum: while just running normal components through some alters after we've loaded them will in some cases get rid of false overridden messages, it will also in some cases incorrectly revert overrides. More digging is needed.

4. Are there issues open for the two other bug fixes addressed here? Could you explain what the current bugs are, and why the solutions you are suggesting are needed? Are any code comments needed to explain?

5. Unless we have a solid need for hooks within the exported code, I'd drop it.

6. Need to patch features.api.php to document any new hooks.

#1: The difference is the the existing hook is only called from features_get_defaults, whereas the new hook is called from both features_get_defaults and features_get_normal. I didn't want to add the old hook name to features_get_normal since that would break existing code using the old hook. So I just created a new hook name.

#2 I personally think that features_defaults is too confusing since the alter hook is also called from features_get_normal.

#3: You actually need to try the current method and play around with it. I agree that on the surface it seems counter-intuitive, but it works in practice. Install my sandbox module for Field Settings and play with it to see how it works in practice.

I'm still thinking about the Add/Delete cases as per rewriting the Features Override module. It's possible that this universal alter doesn't handle those cases. But if you want an alter hook that doesn't get called by features_get_normal, than just use the existing (old) alter hook. Another reason why it makes sense to keep the new alter hook named differently than the old one.

Since I have limited time at the moment if you (or anybody else) can look into this more, I'd appreciate it. All I can say is that I'm running the current hooks on a complex production site without any problems and without the alter hook in get_normal it doesn't work. I've never lost any state data.

Also, introduction of these new hooks doesn't break anything, so I see little reason to hold up putting these into a Features release.

Perhaps if it makes people feel more comfortable, the alter call in features_get_normal could be renamed to be different than the call in features_get_defaults. I'd implement both hooks in Features Field Override. We could decide later what Features Override needs to do.

Not sure I understand your argument regarding Views. Views doesn't need to do anything with the new hooks. The new hooks would only be used by a new Override module.

#4: I have not seen existing issue reports for the bugs. The keys() bug I only found when playing with the code as it only happens when calling the routine with an empty argument and that is not something any existing code does. The issue with the cache in field_rebuild also came up during testing as I ran into cases where the previously cached field was getting used to rebuild, causing the feature to not get rebuilt. There are several issues about features sometimes not being rebuilt and this might be part of a solution. But it only applies to Field exportables.

#5: I'd counter that. No reason for Features to not support alter hooks in the exported code. It was certainly useful to me in early Override work. Why remove a potentially useful feature?

#6: Yes, as soon as it's committed we can document it.

The key question here is: is there conceptually a difference between an "override" and an "alter", such that the two should be handled in radically different ways? I don't see that there is. The aim of the original issue here is to be able to capture in code an override of a default field, altering the default field. This seems like a straightforward case of implementing an alter.

Beyond this, we need to keep in mind the key purpose of features_get_default() and features_get_normal(). These functions fetch representations of the default and normal states of components so that they can be compared. They run alters because, when a component is loaded, it will be be passed to its default alter hook. But when we actually load an exportable like a view on a site, features_get_default() is not called, for the simple reason that Views + Ctools has its own method for loading a view and doesn't need Features. So any altering we add to features_get_default() won't affect views (or other true exportables). For faux exportables like fields, where there's a need to hack in a way to create instances from code representations, features_get_default() is reused for the purpose of generating the items to be saved--but this isn't its primary purpose.

It looks like we can introduce a universal hook only if we require it for features compatibility, or allow modules providing component types to return a key indicating they support the universal alter hook.

Let's take the "philosophy" over to the Features Override module. Let's try to get this thread back on track with the original intent of this issue. A simple patch has been proposed which allows the override of Field Settings as desired in the original post. Whether or not this is "universal" or can also handle other exportables such as "views" can be discussed later and additional patches can be submitted as needed. But I would like to have the current patch reviewed so we can get it committed to Features for the people who just need Field Settings.

While it's motivated by an aim to facilitate field display overrides, nothing in the patch is specific to that use case. It's instead several proposed API changes and bug fixes.

A lot of this patch is an attempt to address an existing bug in Features. Rather than introducing a new alter hook, I think we need instead to fix the existing one. So I've moved this identified bug to a new issue, copying over information from this issue, and sketched in tests to detect the bug: #1345174: Alter hook implementations lead to false overridden status for features components.

I also think the proposed universal hook deserves its own issue, so reopened and updated #1317054: Provide support for exporting of altering of components. My feeling is this feature request would best be postponed until we have fixes to those issues, but I'll leave that call to someone else.

I've sketched in new tests for feature component altering in #1345174: Alter hook implementations lead to false overridden status for features components. They don't show any existing bugs, beyond a need for documentation as explained in that issue.

Mike, can you have a look at that issue? Are there bugs you've found that aren't covered by those tests? In what specific case is there a need to pass features_get_normal() results through an alter call to avoid false overrides? Or is there a different reason for introducing alters in features_get_normal()?

I'll take a look at your tests but haven't done much with module tests myself yet.

The reason for introducing the alter in features_get_normal was because I could never get Field Settings to override properly without it. Maybe this issue is specific to Field settings?

But saying "nothing in the patch is specific to that use case" is incorrect since it's this exact alter hook that was needed to make any progress on the original issue of this thread. The "proposed API change" is what is needed for this issue.

Turns out I misunderstood how Features worked with real exportables (vs faux-exportables like Field). The alter hooks in this patch are only useful for exportables like Fields. They are not useful for other Features like Views/ctools.

I'll post a new patch soon without the extra hooks once I see what's really needed to change in Features itself. If the drupal_alter is called at the end of the actual default_hook exportable, then additional changes to Features code might not be needed.

So you are right on track in #51. Sorry for my confusion!

I have a brand new Sandbox for handling Overrides. It uses the same method of placing the drupal_alter hook within the actual Features exportable. However, it now works with *all* Features types, including CTools features such as Views. It requires the patch to Features #1317054: Provide support for exporting of altering of components. It provides the functionality of the Features Override module, but is much easier to use, has a simple code exportable syntax, and provides several new features such as line-by-line override control.

Eventually I will either make this into the new 2.x version of Features Override or I will move it into core Features itself.

This issue is fixed by the patch in #1317054: Provide support for exporting of altering of components and the new sandbox linked above, so closing this as a dup.

Just waiting for more community testing. But we added it to the D7 Stable Release Blocker list so it will definitely be in the official release (or something very similar to it).