Add option_definition alter to make all handlers & plugins more extensible

Do you have any pointers about how this is a standard idiom to alter views handlers options? Any other module doing this?

IMHO, the right way to do this would be to use hook_views_data_alter() to plug in your our handler, which would be a subclass of the handler with the method(s) you want to override. In this specific case, you would override the option_definition().

This requires no patch. And it's plain simple, effective, and elegant, isn't?

:-)

BTW, one thing that came to mind: what's the purpose of altering option_definition() only?

I mean, OK, you'll be able to add options here and there, and now what? How do you process them? How do you make them useful?

You'd need to alter other methods, don't you?

I still think that sub-classing a handler is the API-wise way to go.

I encountered the same issue, needing to extend every single field handler to add a new option to the base class seems like lunacy, especially considering that everything else, excluding option_definition(), can be hooked.

Patch does the job advertised perfectly.

This needs an update to views.api.php.

So I don't profess to being that great at writing docs, but I gave it a go.

If you add some @param and @return it's perfect IMO.

I've added @param, but as alter functions don't return anything I don't think @return is needed.

And maybe a more realistic example in the hook itself. Maybe something that illustrates the difference between a plugin and a handler.

Suggestions on a postcard please, the example is almost exactly what I'm using for my use-case.

One very minor thing, altered_option_definition() needs an explanation of its @return.

Updated block comment for altered_option_definition().

Waiting for DamienMcKenna's review, in the meantime adding to #2866162: Plan for Views 7.x-3.18 release so it doesn't get lost/forgotten.

A minor change to more clearly indicate what hooks are being called.

I looked around to see if this might inadvertently introduce API changes. I noticed that set_default_options() is also referenced in ViewsUiBaseViewsWizard, does that method need to made aware of the hooks too?

ViewsUiBaseViewsWizard::set_default_options() isn't calling option_definition(), so it shouldn't care about these changes I'd believe.

Also ViewsUiBaseViewsWizard doesn't extend views_object so the former's instance of set_default_options() isn't overriding the latter's.

One concern I have is that modules will need to update to support the new method because they override one of the methods being modified. Right now I only see better_exposed_filters with an overridden unpack_options(), but there could be custom code to content with. This might benefit from a changenotice.

Removing this from the 3.18 release just so we can get it out the door, but we'll look to add it to the next release.

Well of course adding support for the new method for other modules is optional, hence why altered_option_definition() was done as an addition to option_definition().

I looked at "Better Exposed Filters" overridden unpack_options() and it also isn't calling option_definition(), so not sure why it would be effected by this issue.

Also what is the policy regarding change notices, as far as I know this shouldn't hold up inclusion of the feature in a release, at least this is how change notices for "Drupal core" have been treated, see: Outstanding D8 views change notices

I've started a changenotice, please take a look and help improve it: https://www.drupal.org/node/2928637

This needs to be rerolled.

Rerolled.

Committed, thanks everyone!

Thanks.