Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support "constructor-injection" with IConfigurationElement#createExecutableExtension #8

Open
laeubi opened this issue May 11, 2022 · 8 comments

Comments

@laeubi
Copy link
Member

laeubi commented May 11, 2022

I found that one source of using static methods are extensions declared with plugin.xml

The reason for this is, that a extension must have either a default constructor, or implement IExecutableExtensionFactory where both do not allow to pass any items at construction time. Also the creator can not pass additional data when creating the extension...

Example:

IConfigurationElement config = configs[0];
teamHook = (TeamHook) config.createExecutableExtension("class"); //$NON-NLS-1$

So I'd like to propose the following:

  1. Adding a new method to IConfigurationElement with the following signature: Object createExecutableExtension(String propertyName, Object... args) throws CoreException;
  2. if args are given, let it search for a constructor accepting the provided types
  3. if no args are given or no constructor found use the default constructor

That way the example can then be enhanced to pass a workspace object for example:

IConfigurationElement config = configs[0];
teamHook = (TeamHook) config.createExecutableExtension("class", workspace); //$NON-NLS-1$

"old" implementations won't need to adapt, but newer ones could add a new constructor where they get a workspace object injected instead of calling static methods to fetch the required items.

@mickaelistria
Copy link
Contributor

This is overall a good idea, but then implementations would not be backward compatible: imagine an extension made for 4.25 that does use a constructor with args because it's supported, then this extension wouldn't run with 4.24 because it would be missing a no-arg constructor. Although it may not be a strong blocker, we need an elegant way to ensure people understand that limitation.
Isn't it already possible to pass a context and use @Inject on fields when loading an extension point? IIRC at least some extensions allow it, not sure it it's by IConfigurationElement or something else.

@vogella
Copy link
Contributor

vogella commented May 11, 2022

@mickaelistria I implemented that for the org.eclipse.ui.views extention point a few years ago. Users had to / could opt in to this via the additional Inject parameter on the extension point.

@laeubi
Copy link
Member Author

laeubi commented May 11, 2022

imagine an extension made for 4.25 that does use a constructor with args because it's supported, then this extension wouldn't run with 4.24

I think in general one can't expect such kind of compatibility (I won't call this backward compatible) because plugins written fro newer eclipse versions will generally require newer features/API already.

Isn't it already possible to pass a context and use @Inject on fields when loading an extension point

According to the implementation, no I don't see any way for that, for sure one could do so afterwards but this would always need additional techniques (e.g. E4).

@mickaelistria
Copy link
Contributor

I think in general one can't expect such kind of compatibility (I won't call this backward compatible) because plugins written fro newer eclipse versions will generally require newer features/API already.

I agree. However, for things like new API or new extension points, there are versioning guards that make them well use. As per what is allowed in extension class, it's not as easy. We'd need to document it properly on related extension point that enable it (from which version constructor params are enabled; are no-op constructors still useful...?)

@laeubi
Copy link
Member Author

laeubi commented May 11, 2022

If one want be super compatible he can create a default constructor as well (if suitable), if a default constructor is not suitable then it won't make sense of course.
But for example for the resource bundle it makes sense to often pass the workspace (what is some kind of 'context' already). So if an extension can live without the Workspace (e.g. because it get it dynamically from somewhere else) it migth be ok with a default constructor even though a Workspace is passed.

@laeubi laeubi transferred this issue from eclipse-equinox/equinox.bundles Jun 16, 2022
@github-actions
Copy link

This issue has been inactive for 180 days and is therefore labeled as stale.
If this issue became irrelevant in the meantime please close it as completed. If it is still relevant and you think it should be fixed some possibilities are listed below.
Please read https://github.com/eclipse-equinox/.github/blob/main/CONTRIBUTING.md#contributing-to-eclipse-equinox for ways to influence development.

@github-actions github-actions bot added the stale label Dec 14, 2022
@laeubi laeubi removed the stale label Dec 14, 2022
@github-actions
Copy link

This issue has been inactive for 180 days and is therefore labeled as stale.
If this issue became irrelevant in the meantime please close it as completed. If it is still relevant and you think it should be fixed some possibilities are listed below.
Please read https://github.com/eclipse-equinox/.github/blob/main/CONTRIBUTING.md#contributing-to-eclipse-equinox for ways to influence development.

@github-actions github-actions bot added the stale label Jun 13, 2023
@laeubi laeubi removed the stale label Jun 13, 2023
Copy link

This issue has been inactive for 180 days and is therefore labeled as stale.
If this issue became irrelevant in the meantime please close it as completed. If it is still relevant and you think it should be fixed some possibilities are listed below.
Please read https://github.com/eclipse-equinox/.github/blob/main/CONTRIBUTING.md#contributing-to-eclipse-equinox for ways to influence development.

@github-actions github-actions bot added the stale label Dec 11, 2023
@laeubi laeubi removed the stale label Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants